Issue metadata
Sign in to add a comment
|
Manual password saving in password dialog broken |
||||||||||||||||||||
Issue descriptionGo to https://telematici.agenziaentrate.gov.it/Main/login.jsp Enter the following data into the form: Username: Username Password: Password PIN: 1234 Click on the key icon, observe that you are offered to save Username / 1234. Change this to Username / Password. Click save. Observe that the password form is changed to now contain the content Username: Username Password: Password PIN: Password
,
Nov 27
https://ebank.esunbank.com.tw/index.jsp is another example. There the password is later filled into a text field. Raising priority to P1.
,
Nov 28
The problems in both cases are in server predictions. In the first case the server responded that PIN is password, in the second is that the first text field is password and the second text field is username. I'll implement in the new parser that a text field will be no chosen as password even if the server responses that. We need to have a look why server responded incorrectly and to make overrides for these sites.
,
Nov 28
,
Nov 28
In both cases, the server predicts the not-so-password field because that's the field users actually save as password. https://plx.corp.google.com/scripts2/script_5b._bd2d1e_0000_2633_8f70_001a114a7e5c?p=FORM_SIGNATURE%3A4638058470103394044 https://plx.corp.google.com/scripts2/script_5b._bd2d1e_0000_2633_8f70_001a114a7e5c?p=FORM_SIGNATURE%3A5794880210646308347 Interestingly, users *correct* our fill-on-pageload choices, by overwhelmingly moving their password into the PIN field. With that in mind, I think the explanation is that 1. We save the PIN as password during singup 2. We fill the PIN into the password, because of manual overrides 3. The users move the PIN to the PIN field. 4. The pipeline sees that the PIN field is the place to put saved passwords. I can't verify the signup flows of these websites, and I don't know how to verify that we have manual overrides in place, therefore I'm guessing for 1 and 2.
,
Nov 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/31885f533a09743f89c2d90c8a73faa59f82071d commit 31885f533a09743f89c2d90c8a73faa59f82071d Author: Vadym Doroshenko <dvadym@chromium.org> Date: Thu Nov 29 10:37:06 2018 Ignore password related predictions for non-password fields. The server might make mistake and say that <input type=text is password field. This CL teaches the new parser to ignore such predictions. Bug: 908869, 831123 Change-Id: Ib4cfa715e0746b828be3c2356e8ae5bc60717a69 Reviewed-on: https://chromium-review.googlesource.com/c/1354000 Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> Reviewed-by: Vaclav Brozek <vabr@chromium.org> Cr-Commit-Position: refs/heads/master@{#612134} [modify] https://crrev.com/31885f533a09743f89c2d90c8a73faa59f82071d/components/password_manager/core/browser/form_parsing/form_parser.cc [modify] https://crrev.com/31885f533a09743f89c2d90c8a73faa59f82071d/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc
,
Nov 29
vabr going hobby only -> reducing involvement. Please contact me directly in urgent matters. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by battre@chromium.org
, Nov 27