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

Issue 1699993002: [Autofill] Fill fields if they contain a default value. (Closed)

Created:
4 years, 10 months ago by Mathieu
Modified:
4 years, 10 months ago
Reviewers:
vabr (Chromium), sebsg
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, rouslan+autofill_chromium.org, jam, browser-components-watch_chromium.org, jdonnelly+autofillwatch_chromium.org, bondd+autofillwatch_chromium.org, darin-cc_chromium.org, mkwst+moarreviews-renderer_chromium.org, estade+watch_chromium.org, vabr+watchlistautofill_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Autofill] Fill fields if they contain a default value. Previously, would only fill empty fields. Now fills fields that are non-empty, but whose values corresponds to the "value" attribute, meaning it's been the same value since page load. BUG=586806 TEST=FormAutofillTest browser_tests Committed: https://crrev.com/a8a3267d71dc78846d8f60b31d429a1d6845b512 Cr-Commit-Position: refs/heads/master@{#375694}

Patch Set 1 #

Total comments: 3

Patch Set 2 : changed from buh.com #

Unified diffs Side-by-side diffs Delta from patch set Stats (+78 lines, -43 lines) Patch
M chrome/renderer/autofill/form_autofill_browsertest.cc View 1 32 chunks +67 lines, -38 lines 0 comments Download
M components/autofill/content/renderer/form_autofill_util.cc View 1 chunk +11 lines, -5 lines 0 comments Download

Messages

Total messages: 22 (11 generated)
Mathieu
Hi Seb, What do you think of this? PTAL.
4 years, 10 months ago (2016-02-15 23:47:45 UTC) #4
sebsg
This is nice! It's going to fix a bunch of sites. I don't think the ...
4 years, 10 months ago (2016-02-16 17:57:48 UTC) #5
Mathieu
Hi Vaclav, please have a look!
4 years, 10 months ago (2016-02-16 18:02:09 UTC) #7
vabr (Chromium)
LGTM, thanks! https://codereview.chromium.org/1699993002/diff/1/chrome/renderer/autofill/form_autofill_browsertest.cc File chrome/renderer/autofill/form_autofill_browsertest.cc (right): https://codereview.chromium.org/1699993002/diff/1/chrome/renderer/autofill/form_autofill_browsertest.cc#newcode3985 chrome/renderer/autofill/form_autofill_browsertest.cc:3985: "<FORM name='TestForm' action='http://buh.com' method='post'>" Bösen & Heinke? ...
4 years, 10 months ago (2016-02-16 18:12:32 UTC) #8
Mathieu
Thanks! Submitting. https://codereview.chromium.org/1699993002/diff/1/chrome/renderer/autofill/form_autofill_browsertest.cc File chrome/renderer/autofill/form_autofill_browsertest.cc (right): https://codereview.chromium.org/1699993002/diff/1/chrome/renderer/autofill/form_autofill_browsertest.cc#newcode3985 chrome/renderer/autofill/form_autofill_browsertest.cc:3985: "<FORM name='TestForm' action='http://buh.com' method='post'>" On 2016/02/16 18:12:32, ...
4 years, 10 months ago (2016-02-16 20:03:45 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1699993002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1699993002/20001
4 years, 10 months ago (2016-02-16 20:06:31 UTC) #12
commit-bot: I haz the power
Failed to request the patch to try. Please note that binary files are still unsupported ...
4 years, 10 months ago (2016-02-16 21:49:03 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1699993002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1699993002/20001
4 years, 10 months ago (2016-02-16 22:48:55 UTC) #17
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 10 months ago (2016-02-16 22:58:11 UTC) #19
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/a8a3267d71dc78846d8f60b31d429a1d6845b512 Cr-Commit-Position: refs/heads/master@{#375694}
4 years, 10 months ago (2016-02-16 23:00:16 UTC) #21
vabr (Chromium)
4 years, 10 months ago (2016-02-17 09:21:38 UTC) #22
Message was sent while issue was closed.
https://codereview.chromium.org/1699993002/diff/1/chrome/renderer/autofill/fo...
File chrome/renderer/autofill/form_autofill_browsertest.cc (right):

https://codereview.chromium.org/1699993002/diff/1/chrome/renderer/autofill/fo...
chrome/renderer/autofill/form_autofill_browsertest.cc:3985: "<FORM
name='TestForm' action='http://buh.com' method='post'>"
On 2016/02/16 20:03:45, Mathieu Perreault wrote:
> On 2016/02/16 18:12:32, vabr (Chromium) wrote:
> > Bösen & Heinke? :)
> 
> wasn't sure if that was a serious comment, so I changed it to http://abc.com
:)

I was merely curious, what made you chose Bösen & Heinke homepage as the
example, but I had nothing against it (neither I have anything against abc.com).
:)

Powered by Google App Engine
This is Rietveld 408576698