New issue
Advanced search Search tips

Issue 803215 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 770175



Sign in to add a comment

doesn't autofill password for prioritypass.com

Project Member Reported by mbw@google.com, Jan 17 2018

Issue description

At https://www.prioritypass.com/en/login, with a saved password, clicking in the username box does nothing.
 
I /think/ it may help to open chrome://password-manager-internals, then visit that site and expose the password field, then add the content from the chrome://password-manager-internals tab to this bug.

Comment 2 by mbw@google.com, Jan 17 2018

Labels: Restrict-View-Google

Comment 3 by mbw@google.com, Jan 17 2018

Comment 4 by battre@chromium.org, Jan 19 2018

Blocking: 770175
Status: Available (was: Untriaged)
Thank you, I could reproduce this and noticed that even fill on account select is not available.

Comment 5 by battre@chromium.org, Jan 19 2018

One hypothesis of a potential root cause:
The form contains two fields with the same id...

[DOM] Found 2 elements with non-unique id #Password_FormField: (More info: https://goo.gl/9p2vKq)
<input class=​"get-value" id=​"Password_FormField" name=​"Password" placeholder=​"Password" type=​"password" value>​
<input class=​"get-value" id=​"Password_FormField" name=​"Password" placeholder=​"Password" type=​"password" value>​

Comment 6 by battre@chromium.org, Jan 19 2018

Fill on account select does work, but only if you click on the password field, not on the username field. I got that wrong.

Comment 7 by battre@chromium.org, Jan 24 2018

I have analyzed this further and the situation is as follows:

The page has the following structure:

<collapsible div>
  <input type="text" name="Username"/>
  <input type="password" name="Password"/>
</collapsible div>
<collapsible div>
  <input type="text" name="MembershipNumber"/>
  <input type="password" name="Password"/>
</collapsible div>

As none of the input elements are wrapped by a <form>, the password manager sticks them into the artificial global <form>. This causes the issue that we have a form with two password fields, which is interpreted by the password manager as a password-change form and which is therefore not autofilled.

I have no idea how to fix this.

Comment 8 by battre@chromium.org, Jan 24 2018

Labels: -Restrict-View-Google

Comment 9 by battre@chromium.org, Jan 24 2018

Labels: allpublic
Opening this as I am reaching out to the admins. I have deleted the password manager internals log, even though it should not contain any sensitive information in the first place.
Password Manager correctly determines that this form is 2 forms (it thinks that this is SignUp+SignIn forms, but it's not so important). So Password Manager can correctly fills the first form (and it's probably ok). 
The problem is that Password Manager is confused with 2 fields with the same name (there are 2 fields with name Password). It can be easily fix in this case, by filling just the first of field with the same name. But I think that the best solution is to introduce better field identifiers.

Here is CL for simple fix https://chromium-review.googlesource.com/c/chromium/src/+/886707
Owner: dvadym@chromium.org
Status: Started (was: Available)
Project Member

Comment 12 by bugdroid1@chromium.org, Jan 31 2018

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

commit 9c5823d6eabc57e8d9c95e4f08d2270ce853ca4f
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Wed Jan 31 14:38:45 2018

Do not skip password forms with multiple inputs with the same name.

Atm in case when Password Manager tries to fill a password form
and encounters inputs with the same names it stops processing such form.
This CL implements that the first field will be filled.

Bug: 803215
Change-Id: Ia1ec0f1de8390c59308e8c70cbaebe285ae7431d
Reviewed-on: https://chromium-review.googlesource.com/886707
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533271}
[modify] https://crrev.com/9c5823d6eabc57e8d9c95e4f08d2270ce853ca4f/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/9c5823d6eabc57e8d9c95e4f08d2270ce853ca4f/components/autofill/content/renderer/password_autofill_agent.cc

Components: UI>Browser>Autofill
Project Member

Comment 14 by bugdroid1@chromium.org, May 8 2018

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

commit 017745bc613504f1823d5cb1a0d5591a79e53f67
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Tue May 08 14:20:45 2018

Revert "Do not skip password forms with multiple inputs with the same name."

This reverts of https://chromium-review.googlesource.com/c/chromium/src/+/886707.

The reason: it breaks Password Manager on almost any sites where
there are simultaneously fields with ids and without ids. Fixing this properly is
too difficult and risky for merging. And anyway processing of fields with empty
ids will be much improved in https://crbug.com/831123

Bug:  839376 , 803215

Change-Id: I91b535161335844266b0559b0ac21ecaf27faddc
Reviewed-on: https://chromium-review.googlesource.com/1049926
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556791}
[modify] https://crrev.com/017745bc613504f1823d5cb1a0d5591a79e53f67/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/017745bc613504f1823d5cb1a0d5591a79e53f67/components/autofill/content/renderer/password_autofill_agent.cc

Project Member

Comment 15 by bugdroid1@chromium.org, May 11 2018

Labels: merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/edf80dc6f3de6f21d541a43347ef9cb1a2ee53bf

commit edf80dc6f3de6f21d541a43347ef9cb1a2ee53bf
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Fri May 11 08:42:00 2018

Revert "Do not skip password forms with multiple inputs with the same name."

This reverts of https://chromium-review.googlesource.com/c/chromium/src/+/886707.

The reason: it breaks Password Manager on almost any sites where
there are simultaneously fields with ids and without ids. Fixing this properly is
too difficult and risky for merging. And anyway processing of fields with empty
ids will be much improved in https://crbug.com/831123

Bug:  839376 , 803215

TBR=dvadym@chromium.org

(cherry picked from commit 017745bc613504f1823d5cb1a0d5591a79e53f67)

Change-Id: I91b535161335844266b0559b0ac21ecaf27faddc
Reviewed-on: https://chromium-review.googlesource.com/1049926
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#556791}
Reviewed-on: https://chromium-review.googlesource.com/1055387
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#565}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/edf80dc6f3de6f21d541a43347ef9cb1a2ee53bf/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
[modify] https://crrev.com/edf80dc6f3de6f21d541a43347ef9cb1a2ee53bf/components/autofill/content/renderer/password_autofill_agent.cc

Sign in to add a comment