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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 23236002: Prepare for color input datalist support on Android (Closed)

Created:
7 years, 4 months ago by keishi
Modified:
7 years, 1 month ago
CC:
blink-reviews, jamesr, eae+blinkwatch, leviw+renderwatch, abarth-chromium, dglazkov+blink, jchaffraix+rendering
Visibility:
Public.

Description

Prepare for color input datalist support on Android We are passing datalist suggestions when opening the color chooser. Also adding color to input types that support datalist on Android. BUG=242455 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=161893

Patch Set 1 #

Total comments: 4

Patch Set 2 : Added labels #

Total comments: 9

Patch Set 3 : Fixed, added color suggestion struct #

Total comments: 8

Patch Set 4 : Fixed #

Total comments: 3

Patch Set 5 : Removed valueAsText, added 1000 char limit to label #

Patch Set 6 : #

Patch Set 7 : #

Patch Set 8 : #

Total comments: 2

Patch Set 9 : Fixed include #

Patch Set 10 : #

Patch Set 11 : #

Patch Set 12 : #

Patch Set 13 : #

Patch Set 14 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+78 lines, -46 lines) Patch
M Source/core/html/forms/ColorInputType.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M Source/core/html/forms/ColorInputType.cpp View 1 2 3 4 5 3 chunks +11 lines, -3 lines 0 comments Download
M Source/core/rendering/RenderThemeChromiumAndroid.cpp View 1 2 3 4 5 2 chunks +3 lines, -1 line 0 comments Download
M Source/platform/ColorChooserClient.h View 1 2 3 4 5 3 chunks +2 lines, -3 lines 0 comments Download
A + Source/platform/ColorSuggestion.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +13 lines, -8 lines 0 comments Download
M Source/web/ChromeClientImpl.h View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -1 line 0 comments Download
M Source/web/ChromeClientImpl.cpp View 1 2 3 4 5 2 chunks +1 line, -7 lines 0 comments Download
M Source/web/ColorChooserPopupUIController.cpp View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M Source/web/ColorChooserUIController.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +7 lines, -1 line 0 comments Download
A + Source/web/WebColorSuggestion.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +10 lines, -8 lines 0 comments Download
M Source/web/web.gypi View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
A + public/web/WebColorSuggestion.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +17 lines, -10 lines 0 comments Download
M public/web/WebViewClient.h View 1 2 3 4 5 6 7 8 9 2 chunks +10 lines, -1 line 0 comments Download

Messages

Total messages: 35 (0 generated)
keishi
7 years, 4 months ago (2013-08-16 02:15:20 UTC) #1
tkent
https://codereview.chromium.org/23236002/diff/1/Source/web/ColorChooserUIController.cpp File Source/web/ColorChooserUIController.cpp (right): https://codereview.chromium.org/23236002/diff/1/Source/web/ColorChooserUIController.cpp#newcode85 Source/web/ColorChooserUIController.cpp:85: for (size_t i = 0; i < suggestions.size(); i++) ...
7 years, 4 months ago (2013-08-16 02:21:02 UTC) #2
keishi
I've changed createWebColorChooser so the conversion to WebColor happens right before passing to the chromium ...
7 years, 4 months ago (2013-08-19 13:48:17 UTC) #3
Miguel Garcia
Just a pass by review. I am fairly new to blink code so please feel ...
7 years, 4 months ago (2013-08-19 14:17:05 UTC) #4
tkent
https://codereview.chromium.org/23236002/diff/7001/Source/web/ChromeClientImpl.h File Source/web/ChromeClientImpl.h (right): https://codereview.chromium.org/23236002/diff/7001/Source/web/ChromeClientImpl.h#newcode134 Source/web/ChromeClientImpl.h:134: PassOwnPtr<WebColorChooser> createWebColorChooser(WebColorChooserClient*, const WebColor&, const Vector<WebCore::Color>&, const Vector<String>&); In ...
7 years, 4 months ago (2013-08-20 01:09:45 UTC) #5
keishi
https://codereview.chromium.org/23236002/diff/7001/Source/core/html/ColorInputType.h File Source/core/html/ColorInputType.h (right): https://codereview.chromium.org/23236002/diff/7001/Source/core/html/ColorInputType.h#newcode51 Source/core/html/ColorInputType.h:51: virtual Vector<String> suggestionLabels() const OVERRIDE; On 2013/08/19 14:17:05, Miguel ...
7 years, 4 months ago (2013-08-23 15:14:21 UTC) #6
tkent
https://codereview.chromium.org/23236002/diff/14001/Source/core/html/ColorInputType.cpp File Source/core/html/ColorInputType.cpp (right): https://codereview.chromium.org/23236002/diff/14001/Source/core/html/ColorInputType.cpp#newcode245 Source/core/html/ColorInputType.cpp:245: if (suggestions.size() == maxSuggestions) nit. >= is solider. https://codereview.chromium.org/23236002/diff/14001/Source/core/platform/ColorSuggestion.h ...
7 years, 3 months ago (2013-08-25 22:26:00 UTC) #7
keishi
https://codereview.chromium.org/23236002/diff/14001/Source/core/html/ColorInputType.cpp File Source/core/html/ColorInputType.cpp (right): https://codereview.chromium.org/23236002/diff/14001/Source/core/html/ColorInputType.cpp#newcode245 Source/core/html/ColorInputType.cpp:245: if (suggestions.size() == maxSuggestions) On 2013/08/25 22:26:01, tkent wrote: ...
7 years, 3 months ago (2013-08-26 05:29:15 UTC) #8
tkent
Please post "Intent to implement" to blink-dev.
7 years, 3 months ago (2013-08-26 05:30:55 UTC) #9
tkent
https://codereview.chromium.org/23236002/diff/12002/Source/core/platform/ColorSuggestion.h File Source/core/platform/ColorSuggestion.h (right): https://codereview.chromium.org/23236002/diff/12002/Source/core/platform/ColorSuggestion.h#newcode41 Source/core/platform/ColorSuggestion.h:41: // The value represented as text will be used ...
7 years, 3 months ago (2013-08-26 05:34:56 UTC) #10
Miguel Garcia
https://codereview.chromium.org/23236002/diff/12002/Source/core/platform/ColorSuggestion.h File Source/core/platform/ColorSuggestion.h (right): https://codereview.chromium.org/23236002/diff/12002/Source/core/platform/ColorSuggestion.h#newcode41 Source/core/platform/ColorSuggestion.h:41: // The value represented as text will be used ...
7 years, 3 months ago (2013-08-27 14:22:52 UTC) #11
keishi
On 2013/08/27 14:22:52, Miguel Garcia wrote: > https://codereview.chromium.org/23236002/diff/12002/Source/core/platform/ColorSuggestion.h > File Source/core/platform/ColorSuggestion.h (right): > > https://codereview.chromium.org/23236002/diff/12002/Source/core/platform/ColorSuggestion.h#newcode41 ...
7 years, 3 months ago (2013-08-29 05:06:41 UTC) #12
keishi
I got comments that IPC message size might be too big so I added a ...
7 years, 3 months ago (2013-08-29 05:09:15 UTC) #13
tkent
Looks ok. I'll approve this if an "Intent to Implement" is posted to blink-dev.
7 years, 3 months ago (2013-08-29 05:11:20 UTC) #14
tkent
lgtm
7 years, 3 months ago (2013-08-30 04:08:53 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/keishi@chromium.org/23236002/74001
7 years, 1 month ago (2013-11-07 12:09:42 UTC) #16
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 1 month ago (2013-11-07 12:40:29 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/keishi@chromium.org/23236002/224001
7 years, 1 month ago (2013-11-08 06:28:52 UTC) #18
commit-bot: I haz the power
Retried try job too often on blink_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_presubmit&number=10841
7 years, 1 month ago (2013-11-08 06:48:08 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/keishi@chromium.org/23236002/224001
7 years, 1 month ago (2013-11-08 08:36:10 UTC) #20
commit-bot: I haz the power
Retried try job too often on blink_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_presubmit&number=10848
7 years, 1 month ago (2013-11-08 08:57:03 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/keishi@chromium.org/23236002/224001
7 years, 1 month ago (2013-11-08 10:17:25 UTC) #22
commit-bot: I haz the power
Retried try job too often on blink_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_presubmit&number=10854
7 years, 1 month ago (2013-11-08 10:34:56 UTC) #23
tkent
https://codereview.chromium.org/23236002/diff/224001/public/web/WebColorSuggestion.h File public/web/WebColorSuggestion.h (right): https://codereview.chromium.org/23236002/diff/224001/public/web/WebColorSuggestion.h#newcode34 public/web/WebColorSuggestion.h:34: #include "../platform/WebColor.h" Please write it as #include "public/platform/WebColor.h"
7 years, 1 month ago (2013-11-10 22:20:12 UTC) #24
keishi
7 years, 1 month ago (2013-11-11 01:21:49 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/keishi@chromium.org/23236002/394001
7 years, 1 month ago (2013-11-11 01:22:10 UTC) #26
keishi
https://codereview.chromium.org/23236002/diff/224001/public/web/WebColorSuggestion.h File public/web/WebColorSuggestion.h (right): https://codereview.chromium.org/23236002/diff/224001/public/web/WebColorSuggestion.h#newcode34 public/web/WebColorSuggestion.h:34: #include "../platform/WebColor.h" On 2013/11/10 22:20:13, tkent wrote: > Please ...
7 years, 1 month ago (2013-11-11 01:22:21 UTC) #27
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 1 month ago (2013-11-11 01:48:40 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/keishi@chromium.org/23236002/564001
7 years, 1 month ago (2013-11-11 02:14:39 UTC) #29
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 1 month ago (2013-11-11 03:02:38 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/keishi@chromium.org/23236002/714001
7 years, 1 month ago (2013-11-11 03:40:37 UTC) #31
commit-bot: I haz the power
Retried try job too often on linux_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blink_rel&number=11987
7 years, 1 month ago (2013-11-11 05:01:05 UTC) #32
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/keishi@chromium.org/23236002/974001
7 years, 1 month ago (2013-11-11 15:08:40 UTC) #33
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/keishi@chromium.org/23236002/974001
7 years, 1 month ago (2013-11-13 07:18:23 UTC) #34
commit-bot: I haz the power
7 years, 1 month ago (2013-11-13 08:39:16 UTC) #35
Message was sent while issue was closed. 
Change committed as 161893

Powered by Google App Engine
This is Rietveld 408576698