Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(372)

Issue 2906383003: Teach PasswordAutofillAgent sometimes match prefixes of usernames (Closed)

Created:
3 years, 6 months ago by melandory
Modified:
3 years, 6 months ago
Reviewers:
vabr (Chromium), dvadym
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, jam, vabr+watchlistpasswordmanager_chromium.org, rouslan+autofill_chromium.org, rogerm+autofillwatch_chromium.org, sebsg+autofillwatch_chromium.org, browser-components-watch_chromium.org, mathp+autofillwatch_chromium.org, darin-cc_chromium.org, vabr+watchlistautofill_chromium.org, estade+watch_chromium.org, gcasto+watchlist_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Teach PasswordAutofillAgent sometimes match prefixes of usernames. This CL fixes the issue that is specific to email provider web sites in following scenario: 1. The user is able to log in with username "someuser". 2) Chrome stores the credentials with "someuser" as the username. 3) On a next visit to a web site the full username someuser@example.com is prefilled. 5) Chrome does not fill in the stored password, because its username "someuser" does not exactly match the content of the read-only field. The CL relaxes the matching condition in case the read-only username field and when the prefilled username looks like email. BUG=543435 Review-Url: https://codereview.chromium.org/2906383003 Cr-Commit-Position: refs/heads/master@{#481569} Committed: https://chromium.googlesource.com/chromium/src/+/d2290e01a65d77cc3b21bbfe67295938f97f5e3e

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : . #

Total comments: 8

Patch Set 4 : dvadym@ comments #

Patch Set 5 : dvadym@ comments #

Total comments: 30

Patch Set 6 : some Vaclav comments #

Patch Set 7 : drop check #

Patch Set 8 : format #

Patch Set 9 : IsPrefixEndingAtSign IsPrefixOfEmailEndingAtSign #

Patch Set 10 : . #

Patch Set 11 : more browser tests #

Total comments: 12

Patch Set 12 : . #

Total comments: 2

Patch Set 13 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+164 lines, -51 lines) Patch
M chrome/renderer/autofill/password_autofill_agent_browsertest.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +47 lines, -0 lines 0 comments Download
M components/autofill/content/renderer/password_autofill_agent.cc View 1 2 3 4 5 6 7 8 9 10 11 5 chunks +63 lines, -51 lines 0 comments Download
M components/autofill/core/common/autofill_util.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +6 lines, -0 lines 0 comments Download
M components/autofill/core/common/autofill_util.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +6 lines, -0 lines 0 comments Download
M components/autofill/core/common/autofill_util_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +42 lines, -0 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 73 (61 generated)
melandory
PTAL at *password* files. Thanks!
3 years, 6 months ago (2017-06-01 09:37:40 UTC) #24
dvadym
In general it looks good. Some comments below. https://codereview.chromium.org/2906383003/diff/80001/components/autofill/content/renderer/password_autofill_agent.cc File components/autofill/content/renderer/password_autofill_agent.cc (right): https://codereview.chromium.org/2906383003/diff/80001/components/autofill/content/renderer/password_autofill_agent.cc#newcode277 components/autofill/content/renderer/password_autofill_agent.cc:277: current_username, ...
3 years, 6 months ago (2017-06-01 12:15:56 UTC) #25
melandory
https://codereview.chromium.org/2906383003/diff/80001/components/autofill/content/renderer/password_autofill_agent.cc File components/autofill/content/renderer/password_autofill_agent.cc (right): https://codereview.chromium.org/2906383003/diff/80001/components/autofill/content/renderer/password_autofill_agent.cc#newcode277 components/autofill/content/renderer/password_autofill_agent.cc:277: current_username, potential_suggestion, true)); On 2017/06/01 12:15:56, dvadym wrote: > ...
3 years, 6 months ago (2017-06-12 14:56:12 UTC) #34
melandory
vabr@chromium.org: Please review changes in this CL.
3 years, 6 months ago (2017-06-12 14:56:30 UTC) #36
vabr (Chromium)
Hi Tanja, First, could you please expand on the CL title and description? Please make ...
3 years, 6 months ago (2017-06-12 18:56:59 UTC) #37
melandory
https://codereview.chromium.org/2906383003/diff/120001/chrome/renderer/autofill/password_autofill_agent_browsertest.cc File chrome/renderer/autofill/password_autofill_agent_browsertest.cc (right): https://codereview.chromium.org/2906383003/diff/120001/chrome/renderer/autofill/password_autofill_agent_browsertest.cc#newcode723 chrome/renderer/autofill/password_autofill_agent_browsertest.cc:723: AutocompletePasswordForReadonlyUsernamePrefixMatched) { On 2017/06/12 18:56:58, vabr (Chromium) wrote: > ...
3 years, 6 months ago (2017-06-19 10:53:49 UTC) #57
melandory
3 years, 6 months ago (2017-06-19 10:53:50 UTC) #58
vabr (Chromium)
Thanks, Tanja! Almost there, just a discussion about the e-mail-related restrictions + a handful of ...
3 years, 6 months ago (2017-06-19 12:54:16 UTC) #59
melandory
https://codereview.chromium.org/2906383003/diff/240001/components/autofill/content/renderer/password_autofill_agent.cc File components/autofill/content/renderer/password_autofill_agent.cc (right): https://codereview.chromium.org/2906383003/diff/240001/components/autofill/content/renderer/password_autofill_agent.cc#newcode377 components/autofill/content/renderer/password_autofill_agent.cc:377: // |current_username| by scanning |fill_data|. the result is written ...
3 years, 6 months ago (2017-06-21 12:00:42 UTC) #60
vabr (Chromium)
Thanks, Tanja. The current code LGTM, but I found one edge-case I missed before (see ...
3 years, 6 months ago (2017-06-21 12:28:07 UTC) #61
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2906383003/280001
3 years, 6 months ago (2017-06-22 15:39:35 UTC) #70
commit-bot: I haz the power
3 years, 6 months ago (2017-06-22 16:59:10 UTC) #73
Message was sent while issue was closed.
Committed patchset #13 (id:280001) as
https://chromium.googlesource.com/chromium/src/+/d2290e01a65d77cc3b21bbfe6729...

Powered by Google App Engine
This is Rietveld 408576698