New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 906584 link

Starred by 0 users
Status: Fixed
Owner:
█ vabr@chromium.org
hobby only
Closed: Nov 22
Cc:
dvadym@chromium.org
battre@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Regression

Blocking:
issue 831123



Sign in to add a comment

New FormData -> PasswordForm parser needs a fallback

Project Member Reported by vabr@chromium.org, Nov 19 
Chrome Version       : 72.0.3616.0
With --enable-features=new-password-form-parsing,new-password-form-parsing-for-saving

What steps will reproduce the problem?
1. Stage the attached file on a web server
2. Type some credentials in
3. Force saving those and reload the page

After step 2, Chrome should offer a key icon in the omnibox to allow manual saving. This does not happen. One has to re-run Chrome with the above features disabled or remove the autocomplete attribute from the password field to enable saving that field.
Then, after step 3, Chrome should fill the form on load. Instead, it does not fill it even if the user clicks into the password field (no suggestions).

This is because the password field has a (fake) credit-card-related autocomplete attribute, so the new parser ignores it. Unlike the old parser, the new one does not have a fallback mode, so it just gives up.

This tracks adding the fallback to the new parser.

The design doc https://docs.google.com/document/d/1KxWnt3-Pykz4ut4P1IHxNhn6Ef64v3VMyGobUWyLnDg/edit will be updated.
ignored-fields.html
423 bytes View Download
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 20 

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

commit 2d5e7421c87413595049be624f257560e885a0d7
Author: Vaclav Brozek <vabr@chromium.org>
Date: Tue Nov 20 15:20:17 2018

Fix FormParserTest.CVC

The test was meant to check what happens if a password field has a
CVC-like name. Instead, it tested what happens if a text field has
this name.

This CL fixes that mistake. The test thankfully still passes.

This is a yak-shaving for adding the fallback pass, hence the below
bug association.

Bug:  906584 
Change-Id: I9228c0252c97397fb5486e8a62c4cfb6a3f14cf6
Reviewed-on: https://chromium-review.googlesource.com/c/1343269
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609710}
[modify] https://crrev.com/2d5e7421c87413595049be624f257560e885a0d7/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 22 

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

commit 7a90eaac8f6dd6538dbcfddd2d1b01bd1d1d5b51
Author: Vaclav Brozek <vabr@chromium.org>
Date: Thu Nov 22 10:27:05 2018

Add fallback pass to new password form parser

The new FormData->PasswordForm parser lacks a fall-back pass for local
heuristics, if all passwords end up disqualified.

This CL implements that according to the design in
https://goo.gl/ERvoEN

Bug:  906584 
Change-Id: I3d0fa466699d2472ad687a14b920943443e06d0f
Reviewed-on: https://chromium-review.googlesource.com/c/1344431
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610364}
[modify] https://crrev.com/7a90eaac8f6dd6538dbcfddd2d1b01bd1d1d5b51/components/password_manager/core/browser/form_parsing/form_parser.cc
[modify] https://crrev.com/7a90eaac8f6dd6538dbcfddd2d1b01bd1d1d5b51/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc

Comment 3 by vabr@chromium.org, Nov 22

Status: Fixed (was: Started)
►

Sign in to add a comment