New form parser should ignore values that are definetely not username |
||||||
Issue descriptionThe old parser ignores that values which are 1 or 2 digits numbers from consideration for username. The old parser should do the same.
,
Dec 7
I was wondering whether we have similar heuristics for passwords: For example if a site has two filled password fields and one contains 3 or 4 characters and the other 10, then the 10 characters may be more likely to be the password.
,
Dec 7
No we don't have now. We can implement it pretty easily now. We need to investigate this. This bug just makes status quo with the old parser.
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/013b4399c4e89a9034a2a8c35849e2bacb937bd0 commit 013b4399c4e89a9034a2a8c35849e2bacb937bd0 Author: Vadym Doroshenko <dvadym@chromium.org> Date: Fri Dec 07 16:20:17 2018 Do not take 1, 2 digits numbers as usernames. This CL implements checking on IsProbablyNotUsername for candidates in username values as it is in the old parser (IsProbablyNotUsername in password_form_manager.cc) Bug: 831123, 912918 Change-Id: I54fe88d5b87b26c5b5829c3db1f7cf5d3513170c Reviewed-on: https://chromium-review.googlesource.com/c/1367726 Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Cr-Commit-Position: refs/heads/master@{#614715} [modify] https://crrev.com/013b4399c4e89a9034a2a8c35849e2bacb937bd0/components/password_manager/core/browser/form_parsing/form_parser.cc [modify] https://crrev.com/013b4399c4e89a9034a2a8c35849e2bacb937bd0/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc
,
Dec 7
Ack regarding comment #3. We probably don't want to offer saving passwords of 1 character either.
,
Dec 7
I've added to feature sync agenda about not saving passwords with 1,2 characters.
,
Dec 10
,
Dec 10
Have you verified this thoroughly in canary? How safe is this merge overall (in terms of complexity and chances of introducing new regressions)? And why is this critical for 72 vs 73?
,
Dec 10
Yes, I've verified this fix in Canary. This is a simple fix and not risky fix.
,
Dec 10
,
Dec 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/45ab1ac9b42a8d554dc3869dba9ef8366cc100c7 commit 45ab1ac9b42a8d554dc3869dba9ef8366cc100c7 Author: Vadym Doroshenko <dvadym@chromium.org> Date: Mon Dec 10 16:39:24 2018 Do not take 1, 2 digits numbers as usernames. This CL implements checking on IsProbablyNotUsername for candidates in username values as it is in the old parser (IsProbablyNotUsername in password_form_manager.cc) TBR=dvadym@chromium.org (cherry picked from commit 013b4399c4e89a9034a2a8c35849e2bacb937bd0) Bug: 831123, 912918 Change-Id: I54fe88d5b87b26c5b5829c3db1f7cf5d3513170c Reviewed-on: https://chromium-review.googlesource.com/c/1367726 Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#614715} Reviewed-on: https://chromium-review.googlesource.com/c/1369858 Reviewed-by: Vadym Doroshenko <dvadym@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#202} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/45ab1ac9b42a8d554dc3869dba9ef8366cc100c7/components/password_manager/core/browser/form_parsing/form_parser.cc [modify] https://crrev.com/45ab1ac9b42a8d554dc3869dba9ef8366cc100c7/components/password_manager/core/browser/form_parsing/form_parser_unittest.cc
,
Dec 12
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/45ab1ac9b42a8d554dc3869dba9ef8366cc100c7 Commit: 45ab1ac9b42a8d554dc3869dba9ef8366cc100c7 Author: dvadym@chromium.org Commiter: dvadym@chromium.org Date: 2018-12-10 16:39:24 +0000 UTC Do not take 1, 2 digits numbers as usernames. This CL implements checking on IsProbablyNotUsername for candidates in username values as it is in the old parser (IsProbablyNotUsername in password_form_manager.cc) TBR=dvadym@chromium.org (cherry picked from commit 013b4399c4e89a9034a2a8c35849e2bacb937bd0) Bug: 831123, 912918 Change-Id: I54fe88d5b87b26c5b5829c3db1f7cf5d3513170c Reviewed-on: https://chromium-review.googlesource.com/c/1367726 Commit-Queue: Vadym Doroshenko <dvadym@chromium.org> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#614715} Reviewed-on: https://chromium-review.googlesource.com/c/1369858 Reviewed-by: Vadym Doroshenko <dvadym@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#202} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by dvadym@google.com
, Dec 7