New issue
Advanced search Search tips

Issue 912134 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 28
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug

Blocking:
issue 831123



Sign in to add a comment

Back button is considered as form submission indicator

Project Member Reported by dvadym@chromium.org, Dec 5

Issue description

When the new form parser is on for saving (flag #new-password-form-parsing-for-saving), the back button is considered as submission indicator. As result
if the user typed in a password field on a page, clicking back button leads to 
showing save prompt.
 
Blocking: 831123
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4f009f4c944662c1eab47d5a7584f24eec1ddf6e

commit 4f009f4c944662c1eab47d5a7584f24eec1ddf6e
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Sat Dec 08 08:41:15 2018

Propagate information about navigation to DidMainFrameNavigate.

Navigations that are renderer initiated and not link clicked initiated
are considered as maybe form submission. And PasswordManager checks
whether it was successful submission (by checking that a form in which
the user typed is absent after navigation finished).

This CL implements propagating that navigation was renderer initiated
to PasswordManager. Along the way form_submission_tracker_util.h was
NotifyOnStartNavigation was renamed to NotifyDidNavigateMainFrame
and PasswordManager::OnStartNavigation is removed since
DidNavigateMainFrame is used submission detection.

Bug:  912134 , 831123, 842643
Change-Id: I3c6404ac2fbe5f8489541ab551f6a338b650f21c
Reviewed-on: https://chromium-review.googlesource.com/c/1363193
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614962}
[modify] https://crrev.com/4f009f4c944662c1eab47d5a7584f24eec1ddf6e/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/4f009f4c944662c1eab47d5a7584f24eec1ddf6e/chrome/browser/password_manager/password_manager_browsertest.cc
[modify] https://crrev.com/4f009f4c944662c1eab47d5a7584f24eec1ddf6e/components/password_manager/content/browser/content_password_manager_driver.cc
[modify] https://crrev.com/4f009f4c944662c1eab47d5a7584f24eec1ddf6e/components/password_manager/content/browser/form_submission_tracker_util.cc
[modify] https://crrev.com/4f009f4c944662c1eab47d5a7584f24eec1ddf6e/components/password_manager/content/browser/form_submission_tracker_util.h
[modify] https://crrev.com/4f009f4c944662c1eab47d5a7584f24eec1ddf6e/components/password_manager/content/browser/form_submission_tracker_util_unittest.cc
[modify] https://crrev.com/4f009f4c944662c1eab47d5a7584f24eec1ddf6e/components/password_manager/core/browser/form_submission_observer.h
[modify] https://crrev.com/4f009f4c944662c1eab47d5a7584f24eec1ddf6e/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/4f009f4c944662c1eab47d5a7584f24eec1ddf6e/components/password_manager/core/browser/password_manager.h
[modify] https://crrev.com/4f009f4c944662c1eab47d5a7584f24eec1ddf6e/components/password_manager/core/browser/password_manager_unittest.cc
[modify] https://crrev.com/4f009f4c944662c1eab47d5a7584f24eec1ddf6e/ios/chrome/browser/passwords/password_controller.mm
[modify] https://crrev.com/4f009f4c944662c1eab47d5a7584f24eec1ddf6e/ios/web_view/internal/passwords/cwv_password_controller.mm

Labels: Merge-Request-72
Please confirm why this is absolutely critical for M72, and please tell how safe this merge is. 
Labels: -Merge-Request-72
Labels: Merge-Request-72
I've done thorough testing in Canary today and checked metrics. It works as expected. This CL fixes regression in M-72.
Project Member

Comment 7 by sheriffbot@chromium.org, Dec 12

Labels: -Merge-Request-72 Hotlist-Merge-Approved Merge-Approved-72
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 12

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b10d17c23b26f133a42d8757d67e39fcdbffb6ce

commit b10d17c23b26f133a42d8757d67e39fcdbffb6ce
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Wed Dec 12 18:18:26 2018

Propagate information about navigation to DidMainFrameNavigate.

Navigations that are renderer initiated and not link clicked initiated
are considered as maybe form submission. And PasswordManager checks
whether it was successful submission (by checking that a form in which
the user typed is absent after navigation finished).

This CL implements propagating that navigation was renderer initiated
to PasswordManager. Along the way form_submission_tracker_util.h was
NotifyOnStartNavigation was renamed to NotifyDidNavigateMainFrame
and PasswordManager::OnStartNavigation is removed since
DidNavigateMainFrame is used submission detection.

TBR=dvadym@chromium.org

(cherry picked from commit 4f009f4c944662c1eab47d5a7584f24eec1ddf6e)

Bug:  912134 , 831123, 842643
Change-Id: I3c6404ac2fbe5f8489541ab551f6a338b650f21c
Reviewed-on: https://chromium-review.googlesource.com/c/1363193
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614962}
Reviewed-on: https://chromium-review.googlesource.com/c/1374351
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#291}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/b10d17c23b26f133a42d8757d67e39fcdbffb6ce/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/b10d17c23b26f133a42d8757d67e39fcdbffb6ce/chrome/browser/password_manager/password_manager_browsertest.cc
[modify] https://crrev.com/b10d17c23b26f133a42d8757d67e39fcdbffb6ce/components/password_manager/content/browser/content_password_manager_driver.cc
[modify] https://crrev.com/b10d17c23b26f133a42d8757d67e39fcdbffb6ce/components/password_manager/content/browser/form_submission_tracker_util.cc
[modify] https://crrev.com/b10d17c23b26f133a42d8757d67e39fcdbffb6ce/components/password_manager/content/browser/form_submission_tracker_util.h
[modify] https://crrev.com/b10d17c23b26f133a42d8757d67e39fcdbffb6ce/components/password_manager/content/browser/form_submission_tracker_util_unittest.cc
[modify] https://crrev.com/b10d17c23b26f133a42d8757d67e39fcdbffb6ce/components/password_manager/core/browser/form_submission_observer.h
[modify] https://crrev.com/b10d17c23b26f133a42d8757d67e39fcdbffb6ce/components/password_manager/core/browser/password_manager.cc
[modify] https://crrev.com/b10d17c23b26f133a42d8757d67e39fcdbffb6ce/components/password_manager/core/browser/password_manager.h
[modify] https://crrev.com/b10d17c23b26f133a42d8757d67e39fcdbffb6ce/components/password_manager/core/browser/password_manager_unittest.cc
[modify] https://crrev.com/b10d17c23b26f133a42d8757d67e39fcdbffb6ce/ios/chrome/browser/passwords/password_controller.mm
[modify] https://crrev.com/b10d17c23b26f133a42d8757d67e39fcdbffb6ce/ios/web_view/internal/passwords/cwv_password_controller.mm

Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/b10d17c23b26f133a42d8757d67e39fcdbffb6ce

Commit: b10d17c23b26f133a42d8757d67e39fcdbffb6ce
Author: dvadym@chromium.org
Commiter: dvadym@chromium.org
Date: 2018-12-12 18:18:26 +0000 UTC

Propagate information about navigation to DidMainFrameNavigate.

Navigations that are renderer initiated and not link clicked initiated
are considered as maybe form submission. And PasswordManager checks
whether it was successful submission (by checking that a form in which
the user typed is absent after navigation finished).

This CL implements propagating that navigation was renderer initiated
to PasswordManager. Along the way form_submission_tracker_util.h was
NotifyOnStartNavigation was renamed to NotifyDidNavigateMainFrame
and PasswordManager::OnStartNavigation is removed since
DidNavigateMainFrame is used submission detection.

TBR=dvadym@chromium.org

(cherry picked from commit 4f009f4c944662c1eab47d5a7584f24eec1ddf6e)

Bug:  912134 , 831123, 842643
Change-Id: I3c6404ac2fbe5f8489541ab551f6a338b650f21c
Reviewed-on: https://chromium-review.googlesource.com/c/1363193
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614962}
Reviewed-on: https://chromium-review.googlesource.com/c/1374351
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#291}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
Status: Fixed (was: Started)

Sign in to add a comment