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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 471803003: Adjust autofilled property for <search> input and <select> elements. (Closed)

Created:
6 years, 4 months ago by ziran.sun
Modified:
6 years, 3 months ago
Reviewers:
tkent, Ilya Sherman
CC:
blink-reviews, dglazkov+blink, blink-reviews-html_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Adjust autofilled property for <search> input and <select> elements. Set value of autofilled property to false in the following cases: - Cancel search via cancel button in <search> element. - Delete autofilled selected <select> option. This patch also - Merged test case for editing text in autofilled textarea. R=isherman@chromium.org, tkent@chromium.org BUG=395627 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180550

Patch Set 1 #

Total comments: 13

Patch Set 2 : Update code as per review comments. #

Total comments: 5

Patch Set 3 : Update code as per tkent's further review comments. #

Patch Set 4 : Rebase and add NeedsRebaseLine for window test. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+158 lines, -188 lines) Patch
M LayoutTests/TestExpectations View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M LayoutTests/fast/forms/input-autofilled.html View 1 2 3 1 chunk +119 lines, -112 lines 0 comments Download
M LayoutTests/fast/forms/input-autofilled-expected.txt View 1 2 1 chunk +33 lines, -2 lines 0 comments Download
D LayoutTests/fast/forms/textarea/edit-autofilled-text.html View 1 chunk +0 lines, -67 lines 0 comments Download
D LayoutTests/fast/forms/textarea/edit-autofilled-text-expected.txt View 1 chunk +0 lines, -7 lines 0 comments Download
M Source/core/html/HTMLSelectElement.cpp View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/html/shadow/TextControlInnerElements.cpp View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 23 (0 generated)
ziran.sun
Please review. Thanks!
6 years, 4 months ago (2014-08-14 13:01:34 UTC) #1
tkent
https://codereview.chromium.org/471803003/diff/1/LayoutTests/fast/forms/input-autofilled.html File LayoutTests/fast/forms/input-autofilled.html (right): https://codereview.chromium.org/471803003/diff/1/LayoutTests/fast/forms/input-autofilled.html#newcode1 LayoutTests/fast/forms/input-autofilled.html:1: <head> We should remove |input| from the file name ...
6 years, 4 months ago (2014-08-14 23:10:26 UTC) #2
ziran.sun
Updated code as per review comments. All comments have been addressed. Please review. Thanks! https://codereview.chromium.org/471803003/diff/1/LayoutTests/fast/forms/input-autofilled.html ...
6 years, 4 months ago (2014-08-15 13:58:28 UTC) #3
tkent
lgtm with some comments. https://codereview.chromium.org/471803003/diff/20001/LayoutTests/fast/forms/input-autofilled.html File LayoutTests/fast/forms/input-autofilled.html (right): https://codereview.chromium.org/471803003/diff/20001/LayoutTests/fast/forms/input-autofilled.html#newcode59 LayoutTests/fast/forms/input-autofilled.html:59: if (select2.options.length != 3) Use ...
6 years, 4 months ago (2014-08-15 14:20:39 UTC) #4
ziran.sun
Updated code as per tkent's further review comments. Please review. Thanks!
6 years, 4 months ago (2014-08-15 16:39:33 UTC) #5
tkent
The CQ bit was checked by tkent@chromium.org
6 years, 4 months ago (2014-08-15 17:09:28 UTC) #6
tkent
lgtm
6 years, 4 months ago (2014-08-15 17:09:28 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ziran.sun@samsung.com/471803003/40001
6 years, 4 months ago (2014-08-15 17:11:55 UTC) #8
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_blink_rel on tryserver.blink ...
6 years, 4 months ago (2014-08-15 18:11:19 UTC) #9
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 4 months ago (2014-08-15 19:10:20 UTC) #10
commit-bot: I haz the power
Try jobs failed on following builders: win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/22976)
6 years, 4 months ago (2014-08-15 19:10:21 UTC) #11
tkent
The CQ bit was checked by tkent@chromium.org
6 years, 4 months ago (2014-08-18 00:50:38 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ziran.sun@samsung.com/471803003/40001
6 years, 4 months ago (2014-08-18 00:51:23 UTC) #13
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_blink_rel on tryserver.blink ...
6 years, 4 months ago (2014-08-18 01:25:49 UTC) #14
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 4 months ago (2014-08-18 01:58:21 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/23125)
6 years, 4 months ago (2014-08-18 01:58:22 UTC) #16
ziran.sun
The CQ bit was checked by ziran.sun@samsung.com
6 years, 4 months ago (2014-08-19 12:23:20 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ziran.sun@samsung.com/471803003/60001
6 years, 4 months ago (2014-08-19 12:24:26 UTC) #18
commit-bot: I haz the power
Committed patchset #4 (60001) as 180550
6 years, 4 months ago (2014-08-19 13:22:32 UTC) #19
dglazkov
On 2014/08/19 at 13:22:32, commit-bot wrote: > Committed patchset #4 (60001) as 180550 I am ...
6 years, 4 months ago (2014-08-19 16:28:24 UTC) #20
dglazkov
A revert of this CL (patchset #4) has been created in https://codereview.chromium.org/486333002/ by dglazkov@chromium.org. The ...
6 years, 4 months ago (2014-08-19 16:43:07 UTC) #21
dglazkov
On 2014/08/19 at 16:43:07, dglazkov wrote: > A revert of this CL (patchset #4) has ...
6 years, 4 months ago (2014-08-19 16:58:32 UTC) #22
ziran.sun
6 years, 3 months ago (2014-09-04 14:12:03 UTC) #23
Message was sent while issue was closed. 
On 2014/08/19 16:58:32, dglazkov wrote:
> On 2014/08/19 at 16:43:07, dglazkov wrote:
> > A revert of this CL (patchset #4) has been created in
> https://codereview.chromium.org/486333002/ by mailto:dglazkov@chromium.org.
> > 
> > The reason for reverting is: Broke AutofillDialogControllerTest:
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/android/jav....
> 
> Auto-rebaseline landed already, so I had to revert manually here:
> https://codereview.chromium.org/476023003/

I've run android tests locally with this patch and the
AutofillDialogControllerTest runs successfully. This makes me think that the
failure you saw probably is not related to this patch. I'm resubmitting the
patch if there is no objection. Thanks!

Powered by Google App Engine
This is Rietveld 408576698