New issue
Advanced search Search tips

Issue 868948 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 6
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Passwords are not filled on sign-in form on kohls.com with new form parsing

Project Member Reported by dvadym@chromium.org, Jul 30

Issue description

Chrome Version: 69.*
OS: all

What steps will reproduce the problem?
(1) Have saved credentials on www.kohls.com
(2) Turn on the flag #new-password-form-parsing and restart chrome
(3) Go to sign-in form https://www.kohls.com/myaccount/kohls_login.jsp

What is the expected result?
Username/password are autofilled

What happens instead?
Username is autofilled, password is not autofilled.



Please use labels and text to provide additional information.

If this is a regression (i.e., worked before), please consider using the
bisect tool (https://www.chromium.org/developers/bisect-builds-py) to help
us identify the root cause and more rapidly triage the issue.

For graphics-related bugs, please copy/paste the contents of the about:gpu
page at the end of this report.


 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 1

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

commit f0b656aa075ce3bcd25869e84a97353ab515e23a
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Wed Aug 01 12:34:10 2018

Fill multiples time with NewPasswordFormManager.

PasswordFormManager fills multiple times on different events from the
Renderer. That's not optimal from performance point of view, but it adds
robustness. This CL implements the same behaviour in
NewPasswordFormManager as in PasswordFormManager: to fill till 5 times.

This is quick fix, that will be merged in M-69. The better more
robust filling is more complicated and will be implemented later.

Bug:  868948 , 831123
Change-Id: I44c93c5358b0e6707c545001f5f1a1fb7b60eec3
Reviewed-on: https://chromium-review.googlesource.com/1155116
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579777}
[modify] https://crrev.com/f0b656aa075ce3bcd25869e84a97353ab515e23a/components/password_manager/core/browser/new_password_form_manager.cc
[modify] https://crrev.com/f0b656aa075ce3bcd25869e84a97353ab515e23a/components/password_manager/core/browser/new_password_form_manager.h
[modify] https://crrev.com/f0b656aa075ce3bcd25869e84a97353ab515e23a/components/password_manager/core/browser/new_password_form_manager_unittest.cc
[modify] https://crrev.com/f0b656aa075ce3bcd25869e84a97353ab515e23a/components/password_manager/core/browser/password_manager.cc

Labels: Merge-Request-69
Project Member

Comment 3 by sheriffbot@chromium.org, Aug 3

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
It was implemented a new more advance password form filling mechanism, which we're going to launch in M-69. This CL is small bug fix, basically it imitates some behaviour of the old filling mechanism and in order to fix regressions in M-69.
Is the change baked/verified in canary and looks safe to merge?
Yes, this change is safe to merge. I've tested it in Canary and everything works as expected. As I wrote in #4 it doesn't change any behaviour and it's pretty simple, so regressions are unlikely (actually it fixes regressions).
Labels: -Merge-Review-69 Merge-Approved-69
Approving merge to M69 branch 3497 based on comment #6. Please merge ASAP. Thank you.
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 6

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b9c28441e117ce10fd7ec74170d642eb482532a1

commit b9c28441e117ce10fd7ec74170d642eb482532a1
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Mon Aug 06 15:29:38 2018

Fill multiples time with NewPasswordFormManager.

PasswordFormManager fills multiple times on different events from the
Renderer. That's not optimal from performance point of view, but it adds
robustness. This CL implements the same behaviour in
NewPasswordFormManager as in PasswordFormManager: to fill till 5 times.

This is quick fix, that will be merged in M-69. The better more
robust filling is more complicated and will be implemented later.

TBR=dvadym@chromium.org
TBR=vasilii@chromium.org

(cherry picked from commit f0b656aa075ce3bcd25869e84a97353ab515e23a)

Bug:  868948 , 831123
Change-Id: I44c93c5358b0e6707c545001f5f1a1fb7b60eec3
Reviewed-on: https://chromium-review.googlesource.com/1155116
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#579777}
Reviewed-on: https://chromium-review.googlesource.com/1163709
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#417}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/b9c28441e117ce10fd7ec74170d642eb482532a1/components/password_manager/core/browser/new_password_form_manager.cc
[modify] https://crrev.com/b9c28441e117ce10fd7ec74170d642eb482532a1/components/password_manager/core/browser/new_password_form_manager.h
[modify] https://crrev.com/b9c28441e117ce10fd7ec74170d642eb482532a1/components/password_manager/core/browser/new_password_form_manager_unittest.cc
[modify] https://crrev.com/b9c28441e117ce10fd7ec74170d642eb482532a1/components/password_manager/core/browser/password_manager.cc

Status: Fixed (was: Assigned)

Sign in to add a comment