New issue
Advanced search Search tips

Issue 918111 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 10
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocking:
issue 842643



Sign in to add a comment

Detection of when form may be submitted is not correct.

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

Issue description

In order to prevent regressions because of site isolation, the logic of
detection that navigation might be a form submission was implemented
in the Browser process (bug 842643). 

Namely the navigation may be a form submission if
1.it is renderer initiated
2.it is not link transition. 

It is the same as it is in the Renderer process (it is in function FormTracker::DidStartProvisionalLoad). 

It turned out, that browser-side enum ui::PageTransition doesn't allow to detect link transition, so some navigation that are not link transitions are skipped.

As result, there is no save prompt on accounts.google.com with the new parser.
 
Blocking: 842643
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 28

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

commit 03cad4f76faeae0a2e9cf2089ea109e99b4c16c5
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Fri Dec 28 11:10:53 2018

Fix detection when form may be submitted.

In order to prevent regressions because of site isolation, the logic of
detection that navigation might be a form submission was implemented
in the browser process. Namely the navigation may be a form submission
if it is renderer initiated and is not link transition. It is the same
as it was in the renderer process. It turned out, that now enum
ui::PageTransition doesn't allow to detect link transition. This CL
implements the similar logic, namely:
  1.Renderer initiated navigations
  2.Form submission or navigations without user gesture (nowadays a lot
of forms are submitted with some JavaScript).

Bug:  918111 , 831123, 842643

Change-Id: I246e5a5f0f92ec6482053fe77b53fab45445242e
Reviewed-on: https://chromium-review.googlesource.com/c/1391675
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619143}
[modify] https://crrev.com/03cad4f76faeae0a2e9cf2089ea109e99b4c16c5/components/password_manager/content/browser/content_password_manager_driver.cc
[modify] https://crrev.com/03cad4f76faeae0a2e9cf2089ea109e99b4c16c5/components/password_manager/content/browser/form_submission_tracker_util.cc
[modify] https://crrev.com/03cad4f76faeae0a2e9cf2089ea109e99b4c16c5/components/password_manager/content/browser/form_submission_tracker_util.h
[modify] https://crrev.com/03cad4f76faeae0a2e9cf2089ea109e99b4c16c5/components/password_manager/content/browser/form_submission_tracker_util_unittest.cc

Labels: Merge-Request-72 OS-Chrome OS-Linux OS-Mac OS-Windows
This is the bug for the feature which is going to be launched in M-72. This fix is simple and not-risky.
Project Member

Comment 4 by sheriffbot@chromium.org, Jan 3

Labels: -Merge-Request-72 Merge-Review-72 Hotlist-Merge-Review
This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review
Please contact the 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
Labels: -Merge-Review-72 Merge-Approved-72
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 7

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

commit 81c3334c4a621885ff8efb51b157f3dcac0444e1
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Mon Jan 07 18:43:57 2019

Fix detection when form may be submitted.

In order to prevent regressions because of site isolation, the logic of
detection that navigation might be a form submission was implemented
in the browser process. Namely the navigation may be a form submission
if it is renderer initiated and is not link transition. It is the same
as it was in the renderer process. It turned out, that now enum
ui::PageTransition doesn't allow to detect link transition. This CL
implements the similar logic, namely:
  1.Renderer initiated navigations
  2.Form submission or navigations without user gesture (nowadays a lot
of forms are submitted with some JavaScript).

Bug:  918111 , 831123, 842643

TBR=dvadym@chromium.org

(cherry picked from commit 03cad4f76faeae0a2e9cf2089ea109e99b4c16c5)

Change-Id: I246e5a5f0f92ec6482053fe77b53fab45445242e
Reviewed-on: https://chromium-review.googlesource.com/c/1391675
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#619143}
Reviewed-on: https://chromium-review.googlesource.com/c/1398445
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#594}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/81c3334c4a621885ff8efb51b157f3dcac0444e1/components/password_manager/content/browser/content_password_manager_driver.cc
[modify] https://crrev.com/81c3334c4a621885ff8efb51b157f3dcac0444e1/components/password_manager/content/browser/form_submission_tracker_util.cc
[modify] https://crrev.com/81c3334c4a621885ff8efb51b157f3dcac0444e1/components/password_manager/content/browser/form_submission_tracker_util.h
[modify] https://crrev.com/81c3334c4a621885ff8efb51b157f3dcac0444e1/components/password_manager/content/browser/form_submission_tracker_util_unittest.cc

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

Commit: 81c3334c4a621885ff8efb51b157f3dcac0444e1
Author: dvadym@chromium.org
Commiter: dvadym@chromium.org
Date: 2019-01-07 18:43:57 +0000 UTC

Fix detection when form may be submitted.

In order to prevent regressions because of site isolation, the logic of
detection that navigation might be a form submission was implemented
in the browser process. Namely the navigation may be a form submission
if it is renderer initiated and is not link transition. It is the same
as it was in the renderer process. It turned out, that now enum
ui::PageTransition doesn't allow to detect link transition. This CL
implements the similar logic, namely:
  1.Renderer initiated navigations
  2.Form submission or navigations without user gesture (nowadays a lot
of forms are submitted with some JavaScript).

Bug:  918111 , 831123, 842643

TBR=dvadym@chromium.org

(cherry picked from commit 03cad4f76faeae0a2e9cf2089ea109e99b4c16c5)

Change-Id: I246e5a5f0f92ec6482053fe77b53fab45445242e
Reviewed-on: https://chromium-review.googlesource.com/c/1391675
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#619143}
Reviewed-on: https://chromium-review.googlesource.com/c/1398445
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#594}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
Status: Fixed (was: Started)

Sign in to add a comment