Ignore element names in password blacklisting |
|||
Issue descriptionCurrently, username_element and password_element are used for matching a saved blacklist entry to an observed password form. As a result, on pages which use different element names each time a login page is displayed, the user cannot blacklist a credential. We should ignore element names in the matching of blacklisted forms.
,
Apr 11 2016
Keep backwards compatibility in mind...
,
Apr 12 2016
You mean use-cases where the users would have two similar forms on the same page, and used blacklisting to only save passwords on one of them? For this to happen, those forms would need to share the action URL and origin (including path), which makes that situation rather unlikely.
,
Apr 12 2016
I just mean that users may have blacklisted forms in the past and these should stay blacklisted.
,
Apr 12 2016
Yes, this change increases the set of blacklisted forms, what has been blacklisted will stay blacklisted.
,
Apr 12 2016
(Just for completeness, my answer in #3 was not correct, because blacklisting credentials do not go through scoring, they are applied separately. The only remaining matching done is then the same origin requirement, nothing about actions. But the conclusion is still that it is unlikely that this would suddenly blacklist a form which the user wanted not to have blacklisted. And #5 is still correct in saying that whatever was blacklisted before will stay blacklisted.)
,
Apr 12 2016
If we ignore every aspect of the form itself, we will essentially be putting the entire site in the blacklist. As vabr@ pointed out, if we falsely identify a form as a password form, the user will likely blacklist it and unknowingly blacklist the entire site. Perhaps it would make sense, instead of removing these conditions, to add a rule that a form also matches the blacklist if it has the same action. The idea is that the recipient of the form is less likely to be a moving target as well.
,
Apr 12 2016
#7 raises a good point; the "same origin" check mentioned in #6 is indeed without path, so it would cause blacklisting of the whole domain. That might break the user experience as described in the second sentence of #7. I just checked a random site with this issue, and it encodes the session ID in the action path, though, so we might need to think of another way to distinguish the forms to blacklist.
,
Apr 12 2016
(The site I checked was financepilot-pe.mlp.de.)
,
Apr 13 2016
vasilii@ mentioned that this issue does not occur when the "smart bubble" is used. See bug 564045 for that feature and its design doc. It might be a good exercise to understand why the smart bubble solves this. To force the smart bubble behaviour, one can use a command-line flag, e.g., --force-fieldtrial-params="PasswordSmartBubble.3-Times:dismissal_count/3" Depending on the findings, we might either get inspiration for the current blacklisting mechanism, or decide to give up and wait until the "smart bubble" is fully launched.
,
Apr 13 2016
I was able to verify that smart bubble will eventually blacklist the form. The reason for this is that it works with the general page and doesn't try to identify the specific form in the page. We can combine this with the existing conditions: a form is blacklisted if it matches a specific form or there are already multiple blacklisted forms in this page.
,
Apr 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a4a7c52f6f98adf2aa9fc752c0b6e579a54476b0 commit a4a7c52f6f98adf2aa9fc752c0b6e579a54476b0 Author: cfroussios <cfroussios@chromium.org> Date: Thu Apr 14 14:02:52 2016 Blacklisting applied to the form and the page A form is blacklisted if: It has the same elements as a blacklisted form from the same site OR It appears on the same page as a blacklisted form from the same site BUG= 602195 Review URL: https://codereview.chromium.org/1886283002 Cr-Commit-Position: refs/heads/master@{#387303} [modify] https://crrev.com/a4a7c52f6f98adf2aa9fc752c0b6e579a54476b0/components/password_manager/core/browser/password_form_manager.cc [modify] https://crrev.com/a4a7c52f6f98adf2aa9fc752c0b6e579a54476b0/components/password_manager/core/browser/password_form_manager_unittest.cc
,
Apr 14 2016
,
Jun 23 2016
,
Jul 7 2016
,
Nov 29
|
|||
►
Sign in to add a comment |
|||
Comment 1 by vabr@chromium.org
, Apr 11 2016Status: Assigned (was: Available)