User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.6) Gecko/20100114 Gentoo Firefox/3.5.6 Build Identifier: We should implement this new input element type. Basically, it lets the user put something like "#ffffff" and, according to the specifications, we can add a color picker. The color picker should probably be another bug. Reproducible: Always

It looks like the color state has a small issue : it can't suffer from a type mismatch but it looks needed to know if the color value is correct. Considering WebKit has implemented the type mismatch for the color state, should we do the same or wait for a change in the specifications ? I've open a bug for HTML WG: http://www.w3.org/Bugs/Public/show_bug.cgi?id=9357

This is a mockup of what I think could be a good first step. The input in the color state would have a text field limited to [0-9a-f] characters. If the user try to delete a character, it would be replaced by '0' to make sure the color value is always valid. The '#' character can't be deleted. The 'button' on the right of the field will show the typed color. The second step would be to have a color picker appearing when the button is clicked. This color picker will be able to change the text value. This style is consistent with the input search state if we add a magnifying glass like in search boxes (we can also imagine adding an email icon for email types, a telephone icon for telephone ones, ...). Would that also be better for accessibility than just just a color picker ?

Adding in CC Alex and David for UI and accessibility.

Comment on attachment 436665 [details] Mockup of the simple UI It would be nice for us to get early support for this before we implement a full color picker, but exposing the color's hexadecimal value breaks the healthy barrier between the implementation and the user interface. Also since this element is new and we would potentially be one of the first browsers to support it, we don't want to give Web developers the impression that the control isn't meant for mainstream users. Or even worse, another browser without a UI review process might actually copy our lead and we cold accidentally set a precedent for forcing humans to speak in hex. So I can't really give this ui-r+, but if we still want to land it for testing or for Web developers to start to build rough applications that will later use the real color picker, that's perfectly fine, we just can't actually ship an element that displays hexidemical notation to the user.

I agree with Alex. The average user is does not care for the hex codes (or any other form of code-way to present colors). I think we should go with a color-picker only approach, and (perhaps we must do this, I would certainly use it) perhaps add an "Advanced" button, where the color is presented with both hex and rgb colors in separate forms.

Oh, and of course all of the things mentioned in my comment would have to be located in a separate window which can be opened through the original type="color"-input somehow. I am not sure how we want to present the chosen color through the input/form itself.

Thank you for your feedback Alex. I thought having the hexadecimal field would be helpful for some advanced users who know what is the hexa code for some simple colors. I will try to attach another mock-up this week including 'list' and 'autocomplete' attributes.

What's wrong with using the the color picker code that Firefox already uses in Tools->Options->Content tab, Font & Colors -> Colors ? (Click on a colored box for the color picker to appear)

Kurt, with this color picker, you have a limited choice of colors. The best would be to have a choice of colors and be able to choose one more or less like the gimp color picker.

What about a hybrid setup? Where it shows a palette of the last X used colours, with a button to show a full blown colour picker?

I think this: http://ficml.org/jemimap/style/color/wheel.html is what we are looking for. A color wheel. When putting together a mock-up in my head, I see a color wheel, a "current color" box and a button saying "Advanced >>". If you click this button, the window with the color wheel extends to the right, revealing two form fields where one contains the current color in hex (#000000) and the other with rgb (rgb(0, 0, 0)). Focusing either of these fields puts the color wheel in a disabled state, as we assume that the user wants to choose the color "manually", with codes, instead. As soon as a real color is chosen through one of the fields, the "current color" box is updated to reflect this. If the user leaves the box with an unfinished code, it is reset to the value it had before it got edited. I can already see that my description is messy, so I'll stop there and will try to create a real mock-up later today/tomorrow.

This, and the following attachments should give you an idea of what I am thinking. (Of course, this will require lots of polish by the UI-team)

In addition of the attachment, this document from Google may be interesting: http://docs.google.com/View?id=dch3zh37_0cf8kc8c4 There is a part about the color picker but it may interest people who are wondering how to render html5 forms element.

Note: There are better ways to illustrate "disabledness", but this should give you an idea.

This "draft" is showing more or less all the elements I think should be in the color picker. As suggested Magne, we could use an "Advanced" button or a tab to show/hide the advanced fields (RGB + Hex). I think this color picker is better than the Windows one because it needs less precision than the color picker with bars so it would be easier to use on touchscreens. In addition, RGB sliders are easy to use with touchscreens. Finally, maybe we can add a button to pick a color from the screen. Alex, what is your opinions about those screens ?

By the way, the colors in the bottom of the color picker window would represent the 'autocomplete' attribute. In other words, it will show last used colors.

I just noticed that Twitter implements a nice color picker of their own, in "Settings -> Design -> Change design colors". It contains the basic features of what we've discussed. You might want to take a look at it.

The input element in the color state will only show a button with a color in the web page. When the user would click on it, if the list attribute is specified, it will show the colors and let the user choose another one with 'Other' which will show the color picker. Otherwise, it will directly show the color picker. This is highly inspired from the Google's document.

By the way, it seems we've agreed to create a color-picker independent from the OS, unlike what is discussed for Webkit at http://docs.google.com/View?id=dch3zh37_0cf8kc8c4. I think an independent implementation has several advantages, including more control and making easier to add functionality in the future and, perhaps most importantly, supply a color picker on platforms that does not supply one themselves (which the Webkit implementation will lack on these OS's). Great!

Comment on attachment 437574 [details] Color Picker Mockup Draft v1 yep, general functionality is all stuff we will want. Might want to make some changes to layout, and I'm debating in my head if we want to start the user with the "box of crayons" model (but not literally, similar to the widget used for font color selection in prefs). The circle to control hue can feel a bit overwhelming, but definitely needs to be available. If we were to say have two tabs, one consisting of the box of crayons model and the other this full set of controls, would there be any reasonable way for us to expose the control set initially displayed to the Web developer? Unfortunately this control might have wildly different use cases for us to anticipate. If you are setting the color of a cell in a spreadsheet you likely want the crayon approach. If you are selecting the color of your new snowboard, you might want considerably more control.

Comment on attachment 437584 [details] Input type='color' Mockup v2 Yeah this looks good (although we'll want to carefully select the default color palette). Also this approach addresses the comment I just wrote. Sorry about that, gmail didn't thread the two requests.

(In reply to comment #25) > (From update of attachment 437584 [details]) > Yeah this looks good (although we'll want to carefully select the default color > palette). > > Also this approach addresses the comment I just wrote. Sorry about that, gmail > didn't thread the two requests. I don't think we specify any default colors. That attachment shows a few colors, depending on if the list attribute (see bug 556007) is present. The list attribute will allow the website itself to specify some standard colors that might fit in with whatever the colors are going to be used for.

(In reply to comment #24) > Unfortunately this control might have wildly different use cases for us to > anticipate. If you are setting the color of a cell in a spreadsheet you likely > want the crayon approach. If you are selecting the color of your new > snowboard, you might want considerably more control. Perhaps HTML5 should have some sort of way to indicate this distinction. Anybody feel like starting a thread on the whatwg list on the topic?

...er, never mind me, hadn't read the very latest comment. :-)

Material for follow up bugs, but worth considering now: Accessibility. I know a few completely blind people who do like to set color on their blogs...

Would/should there be a way for the user to specify a transparent colour? I personally think just using HSL/HSV and hiding the RGB and hexadecimal widgets behind an advanced button is a good idea, but transparency is a fairly basic aspect.

This is Gnome gcolor2 color selector. The HTML5 variant would not need the opacity setting. Compared to attachment 437567 [details] this one allows easy selecting of different grey (zero saturation) values. Also notice that the user can save colors with a specified name (a click on "Save" button will open a prompt for new color name). It's barely visible in the screenshot, but every saved/named color has a small thumbnail in front of the hex code.

Also notice the pipette button in attachment 437782 [details] - it allows picking the color from any pixel on the whole screen. This allows one to easily pick color from, say an open PDF document or from another web page.

(In reply to comment #30) > Would/should there be a way for the user to specify a transparent colour? > > I personally think just using HSL/HSV and hiding the RGB and hexadecimal > widgets behind an advanced button is a good idea, but transparency is a fairly > basic aspect. I thought about transparency earlier, but the HTML5 spec says that only "simple colors" should be allowed, so that would be colors that can be reproduced through RGB, which transparency can't.

Indeed, the specs say the color value has to be #RRGGBB so the transparency has not to be showed. In my opinion, the Gnome color picker is really bad because it is not clear and far too complex. For example, we do not need 'Hue', 'Saturation' and 'Value'. If you want to choose a variation of a color, the vertical slider of my draft may be more helpful than anything else for most users. About the 'Save' button, I think the autocomplete values should be enough.

As a side note, I think OS X users are going to want to have their native color palette. This would apply to windows as well, but it's color palette is pretty awful. Either we are going to need to implement our own since there are platforms like Android that don't provide a native implementation. But we might want to make an exception for the OS X users and allow them to use the control that they are already familiar with.

(In reply to comment #35) > As a side note, I think OS X users are going to want to have their native color > palette. This would apply to windows as well, but it's color palette is pretty > awful. Either we are going to need to implement our own since there are > platforms like Android that don't provide a native implementation. But we > might want to make an exception for the OS X users and allow them to use the > control that they are already familiar with. As a Mac user, I'd just like to ask, how does our native color-palette look? ;-)

Open up Activity Monitor (either in Applications or Applications/Utilities, I don't remember which) and click on a color indicating a type of memory (free/wired/active/&c.).

Ah, I see. It looks pretty good, IMO. I think we could use several parts/get ideas from it when we build our own.

(In reply to comment #38) > Ah, I see. It looks pretty good, IMO. I think we could use several parts/get > ideas from it when we build our own. Well... The coolest thing of all would be to call the native color picker on each platform ! I have no idea if the native colorpickers are accessible.

>Well... The coolest thing of all would be to call the native color picker >on each platform ! Definetly on OS X we'll want to use the native color picker (it aparently even has its own extension system that people use). However the native color picker on Windows isn't that great (at least I think we can make a better one), and Android as far as I know doens't have a native one at all. Not sure what the sitation is on Linux, but either way I think we are probably looking at a mixture of using the native one versus providing our own so that we can give the user the best possible color picking interface.

any update on this? Fx4 supports a good range of html5 elements now including some native form elements, but unfortunately input type=color isnt one of them. the last update on this bug is from October, would be nice if this could get slotted into the impending fx4.0 release

4.0 has been in feature freeze for months now, so this bug certainly won't make it into 4.0. There's a good chance it'll be in the next release, 3 months after 4.0, though.

I, as a Linux user, would love to have the native color picker (my Firefox is compiled with GTK+ so the GTK+ one). I love the GIMP one but there is no point to use it (see below). I, as web developer, would like my visitors to see their native color picker (the GTK+ one, the Qt one, the Windows one or the Mac OS X one). They use to see these native ones, even if they're not perfect (the Windows one is not that bad, to my view). The "returned" value, if I understand the W3C spec correctly, is the 6 hexadecimal-digits value (default to '#000000'). Any native picker is able to give this value, and any user should be used to see the native one. Anyway, any news on that ? :-)

This is now implemented in Chrome: https://plus.google.com/102860501900098846931/posts/iWujARgWLcn

I started to try to implement this. I've implemented the control frame part: color input is supported and displayed as a button with a colored rectangle inside. For a first step, it looks nice IMHO :) But we may change this later if necessary I suppose. It's black by default but the |value| attribute is supported, so other color can be used (e.g. "#ff0000", "black", etc.) The next big part is to implement color pickers (i.e. the big TODO I've added in nsHTMLInputElement): currently user can't select another color. I agree with the idea of using system's color picker, as we do for file picker. I'm OK to write the interface and the color pickers for Linux, but I will need some help for other systems. But I would like to have some feedback on this before starting to try to implement color pickers.

Comment on attachment 668925 [details] [diff] [review] Color input: frame part Review of attachment 668925 [details] [diff] [review]: ----------------------------------------------------------------- This looks generally good but I do not think you are following all the rules the HTML specifications requires: http://www.whatwg.org/specs/web-apps/current-work/multipage/states-of-the-type-attribute.html#color-state-%28type=color%29 I think it could be good to have different patches to isolate components but also to make reviews easier (I'm not a layout peer for example). Do you think you can focus on having the HTML specs implemented correctly in content/ with tests? I could easily review that and you could easily add a layout on top of it. Also, in the general behaviour of the patch, I'm not user I understand how bgcolor is used. Shouldn't you use the value of the element instead? ::: content/html/content/src/nsHTMLInputElement.cpp @@ +128,4 @@ > { "tel", NS_FORM_INPUT_TEL }, > { "text", NS_FORM_INPUT_TEXT }, > { "url", NS_FORM_INPUT_URL }, > + { "color", NS_FORM_INPUT_COLOR }, nit: keep the list alphabetically ordered. @@ +132,5 @@ > { 0 } > }; > > // Default type is 'text'. > +static const nsAttrValue::EnumTable* kInputDefaultType = &kInputTypeTable[14]; That's changing the default type to color, right? @@ +330,5 @@ > { > + if (mInput->GetType() == NS_FORM_INPUT_FILE) { > + return InitFilePicker(); > + } else if (mInput->GetType() == NS_FORM_INPUT_COLOR) { > + return InitColorPicker(); Instead of that I would prefer to have a ColorClickHandler and a FileClickHandler, both inheriting from a base class implementing the common parts. In ::FireAsyncClickHandler, you would then be able to launch the correct runnable. ::: layout/forms/nsColorControlFrame.cpp @@ +58,5 @@ > + nsString color = GetColor(); > + > + nsCOMPtr<nsIDocument> doc = mContent->GetDocument(); > + nsCOMPtr<nsINodeInfo> nodeInfo = > + doc->NodeInfoManager()->GetNodeInfo(nsGkAtoms::input, nullptr, You can also create a button, it will save you the type setting. ::: layout/style/forms.css @@ +493,5 @@ > button, > input[type="reset"], > input[type="button"], > +input[type="submit"], > +input[type="color"] { Not sure if we really want to have "color" sharing the rules of regular buttons.

Also, Arnaud, do you have access to the try server? It's also good to provide a try build so people can easily test your patch. If you don't have an access to the try server, let me know, I can vouch you.

> This looks generally good but I do not think you are following all the rules > the HTML specifications requires: Indeed maybe not everything is working as expected. For the color parsing, I do nothing and let the attribute parsing (while doing |mColorContent->SetAttr(kNameSpaceID_None, nsGkAtoms::bgcolor, ...|) do all the work, to be coherent with all the other uses of colors within the Firefox. For the other rules, indeed, I may not have implemented all of them. But maybe some can be done in a second step. (IMO it will be better to have a partially implemented attribute than nothing). > I think it could be good to have different patches to isolate components but > also to make reviews easier (I'm not a layout peer for example). > Do you think you can focus on having the HTML specs implemented correctly in > content/ with tests? I could easily review that and you could easily add a > layout on top of it. > > Also, in the general behaviour of the patch, I'm not user I understand how > bgcolor is used. Shouldn't you use the value of the element instead? Indeed, I did everything in one part. For me, it was natural to have everything implemented, so I can see/test the final result within Firefox. Actually, I haven't written a layout part: just specify the CSS, as you've seen. I thought this color input should be a kind of "button"; that's why |nsColorControlFrame| is very inspired by |nsGfxButtonControlFrame| and also inherits |nsHTMLButtonControlFrame|. And |nsHTMLButtonControlFrame| uses |nsButtonFrameRenderer| (defined in layout/) which will render my color button. And just modifying the bg color attribute in the control frame does the trick. I thought it will be enough and it was not necessary to rewrite a layout renderer which will shared most of its code with |nsButtonFrameRenderer|. But maybe I forgot to also change value attribute in |nsColorControlFrame::AttributeChanged| method. > nit: keep the list alphabetically ordered. > > // Default type is 'text'. > > +static const nsAttrValue::EnumTable* kInputDefaultType = &kInputTypeTable[14]; > That's changing the default type to color, right? I thought I initially kept the list alphabetically ordered, so I incremented the kInputDefaultType accordingly, to have it still pointing to |text|. Don't know why I've changed this but it's wrong indeed. I will correct this. > Instead of that I would prefer to have a ColorClickHandler and a > FileClickHandler, both inheriting from a base class implementing the common > parts. > In ::FireAsyncClickHandler, you would then be able to launch the correct > runnable. OK, why not. Actually I didn't have a deep look to this, as I this is part of the second big step (i.e. implementing the color picker widget). Maybe this change can be part of this second drop, as I don't know yet which part can/should be factorized, then if it's worth to have txo ClickHandler classes with a common base class. > You can also create a button, it will save you the type setting. How can I do this? > Not sure if we really want to have "color" sharing the rules of regular > buttons. OK. Any particularly points which may be problematic? Do you think I can leave this like this for now, or should I create another (very similar) rule in this patch? > If you don't have an access to the try server, let me know, I can vouch you. I don't have access to the try server, I would be glad to have one if you can vouch me :) Thanks for you quick feedback about this!

(In reply to Arnaud from comment #48) > For the other rules, indeed, I may not have implemented all of them. > But maybe some can be done in a second step. (IMO it will be better to have > a partially implemented attribute than nothing). Generally speaking, we do not ship half backed web facing features in Firefox. > Actually, I haven't written a layout part: just specify the CSS, as you've > seen. The frame implementation you have is the layout part, isn't it? > > Instead of that I would prefer to have a ColorClickHandler and a > > FileClickHandler, both inheriting from a base class implementing the common > > parts. > > In ::FireAsyncClickHandler, you would then be able to launch the correct > > runnable. > > OK, why not. > Actually I didn't have a deep look to this, as I this is part of the second > big step (i.e. implementing the color picker widget). Maybe this change can > be part of this second drop, as I don't know yet which part can/should be > factorized, then if it's worth to have txo ClickHandler classes with a > common base class. Fine by me. > > You can also create a button, it will save you the type setting. > > How can I do this? When you create the anonymous content, you create a nsINodeInfo that is a nsGkAtoms::input, just change this to nsGkAtoms::button so you will not have to set the type. > > Not sure if we really want to have "color" sharing the rules of regular > > buttons. > > OK. Any particularly points which may be problematic? Do you think I can > leave this like this for now, or should I create another (very similar) rule > in this patch? We should not do that generally speaking. I didn't look carefully to see what was needed or not. It's more than likely that we could remove some properties from that list for color. > > If you don't have an access to the try server, let me know, I can vouch you. > > I don't have access to the try server, I would be glad to have one if you > can vouch me :) Will do that. > Thanks for you quick feedback about this! Any time!

(In reply to Mounir Lamouri (:mounir) from comment #49) > The frame implementation you have is the layout part, isn't it? Yes indeed, even if it's quite simple. Sorry for the confusion. The content implementation should go in |nsHTMLInputElement|. About the spec, I've a deeper look and indeed there are rules that I'm not following: the value sanitization algorithm isn't implemented (should be easy to do using existing code though), and I was wrong when modifying some Does*Apply methods in |nsHTMLInputElement|. Also, I'm not sure all the events specifies in the spec will apply as expected with my patch. I will review and fix that. But one thing that bothers me in the spec is the support of autocomplete and list attributes. I have no idea how autocomplete should be implemented for color (didn't thought too much about it). Something like what you suggested in comment 20, for native color pickers which have something similar in their interface might be a good idea. Also, I would prefer to leave list attribute support for later: currently I only have a simple color button with the color inside. Implementing list support in the layout part to have something like what you suggested in comment 22 (which I agree is probably is the best idea for implementing the list attribute) would required a more complex layout part, and I don't know how to implement this for now. So I would prefer to leave that for later, and maybe open a follow-up bug for this. Would you be OK with this?

would it not be better to implement the colour picker as per other vendors (Opera/Webkit) in that the button displays the colour, clicking it displays a default list of colours, with an option for "other" which opens the native colour picker? that would make it easier to implement custom lists, as you just replace the default colours with those selected by the developer. support for custom lists should be in "version 1" of this implementation IMO, as where I've used input=color so far, I have always selected some preset colours for people to select from. eg on a shopping website, we may want to display available colours for an item, or range of colours available in a product search. custom lists help us to inform/guide the user to what colours they should select from when using the form.

(In reply to Gavin from comment #51) > would it not be better to implement the colour picker as per other vendors > (Opera/Webkit) in that the button displays the colour, clicking it displays > a default list of colours, with an option for "other" which opens the native > colour picker? As far as I know, Chrome doesn't display a default list of colors. But Opera does, and, indeed, I think it's a nice idea. IMO, it can be done in a second step, as list is supposed to give suggestions, not to restrict user input, and because I still believe having a color button without list support is better than nothing, as long as list support implementation is planned.

html5 forms是很重要的功能，为什么不支持color,datetime类型的表单输入呢

I finally had some time to look at this again. I cleaned-up my previous patch and correct it to (hopefully) implement the spec correctly (and I've also updated tests accordingly). The color picker is still missing: I will start to look at this now. One thing I'm not sure about: In the nsColorControlFrame, I'm creating a NS_NewHTMLElement (of type "input") to represent the colored area of the color button, and modify its |bgcolor| attribute. It's working because I mapped |bgcolor| in |HTMLInputElement|. I don't know if it's the best solution: maybe should I use "div" instead of "input", then "map" bgcolor in the corresponding class instead of |HTMLInputElement|. Maybe it just doesn't matter doing it this way or another. @Mounir: could you please have a look to this patch? Or delegate it to someone else? It also contains the layout part because it was easier for me to see the result if the layout is implemented, but feel free to let someone review this part if you prefer.

I've updated my previous patch a bit to correct the CSS and fix some broken tests I missed.

Comment on attachment 740012 [details] [diff] [review] Color input: frame part Review of attachment 740012 [details] [diff] [review]: ----------------------------------------------------------------- Arnaud, I think it would be easier to review this if you split the patches in different chunks. The minimum would be content + layout being separated. Also, there is no widget implementation. Is that for later? What is the current UI? Just a text field?

> Arnaud, I think it would be easier to review this if you split the patches > in different chunks. The minimum would be content + layout being separated. OK, I will do so. > Also, there is no widget implementation. Is that for later? What is the > current UI? Just a text field? I left that for later (actually I'm currently working on this and I should have something working very soon) so there no UI to change the value currently: only to show the current one (a colored button instead of a the default text field).

As discussed, only the content part.

And here is the layout part. Mounir: I've requested you as a reviewer for this patch, but feel free to delegate it to someone else if you prefer (as you told me you're not a layout peer).

Oops. I just realized I missed the most important part (new nsControlColorFrame class) in my previous patch :( This one should be better ;)

Btw, I've pushed these two patches to try: https://tbpl.mozilla.org/?tree=Try&rev=6953948ed801 if you want to quickly check how the layout I've implemented looks like using the builds. The tests are "almost" green: only test_input_typing_sanitization is sometimes failing (timeout) but I'm not sure why it fails...

I understood why test_input_typing_sanitization failed and fixed it. 'color' is a bit different from other input types tested in test_input_typing_sanitization, as it's not a text frame. So for color, I skip |sendKeyEventToSubmitForm| tests. Also, for color, for the same reasons, I have to set the value directly instead of using |sendString|. It's a bit hacky to add this special checks for color, but most of the test logic is the same, so I don't think it would be worth to write another test just for color input sanitization. But maybe I can make this nicer by adding a |hasTextFrame| flag in test |data| instead? I've pushed this new changes to try: https://tbpl.mozilla.org/?tree=Try&rev=5bd196378ee5

And here is the last part: the widget to select the color. As discussed, it uses the system's native color picker, as for file picker. The patch contains the color picker interface and the implementation for gtk2. Should we also implement a default, system independent, color picker? Which might be used for platforms where color picker interface isn't implemented and/or used depending on a user pref, as for file picker? I've pushed this to try: https://tbpl.mozilla.org/?tree=Try&rev=9b2c32fa53b0 so you can now test the full feature with the build :) (only on Linux though as the color picker will be missing on other platforms). The build failed on OS X because nsColorPickerShownCallback's destructor isn't virtual (I guess it failed because compiler flags are slightly different for OS X). I will changed this but this is just minor thing so this patch can be reviewed anyway IMO. Mounir: once again I've added you for the review, but feel free to delegate it to someone else.

Comment on attachment 742812 [details] [diff] [review] Color input: content part (2nd) Review of attachment 742812 [details] [diff] [review]: ----------------------------------------------------------------- Awesome! :) I have quite a few comments but I think addressing them will be very easy and I'm pretty sure the next patch will get a r+. You should have added NS_FORM_INPUT_COLOR to ::Clone(), ::RestoreState() and ::SaveState() but I just wrote a patch to refactor those methods (bug 867950) so you will not need to explicitly add this type there. (This is just for information.) Could add NS_FORM_INPUT_COLOR in ::GetValueMode() ? In nsWebBrowserPersist::CloneNodeWithFixedUpAttributes embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp, could you add NS_FORM_INPUT_COLOR in the big switch (around line 3254). Also, could you update the following tests to take into account 'color': - content/html/content/test/forms/test_required_attribute.html - content/html/content/test/forms/test_pattern_attribute.html - content/html/content/test/forms/test_stepup_stepdown.html Finally, a few items that should be taken care of in follow-ups: - we probably need a preference to toggle this, something like "dom.forms.color"; - we should think about making <input type='color'> handling readonly. The specs says we shouldn't do that but Webkit and Presto do. - we should implement something for list; - we should implement something for autocomplete; - I wonder how much @required would be useful, it's not doable with the current model but I wonder how much the model should change to make that possible. ::: content/html/content/src/HTMLInputElement.cpp @@ +1116,5 @@ > GetValueInternal(aValue); > > + // Don't return non-sanitized value for types that are experimental on mobile, > + // and for color > + if (IsExperimentalMobileType(mType) || mType == NS_FORM_INPUT_COLOR) { I do not think you need that for type='color'. @@ +3170,5 @@ > nsMouseEvent::eRightButton)) { > if (mType == NS_FORM_INPUT_BUTTON || > mType == NS_FORM_INPUT_RESET || > + mType == NS_FORM_INPUT_SUBMIT || > + mType == NS_FORM_INPUT_COLOR) { Why did you add that? I'm not sure why this code is needed for type={button,reset,submit} in the first place so I wonder why it is needed for type='color' and maybe it will help me understand the need for those other types :) @@ +3649,5 @@ > + aValueString.Length() - 1); > + // nscolor is required by NS_HexToRGB, but not used, as just want to > + // check the string parsing > + nscolor color; > + if (NS_HexToRGB(withoutHash, &color)) { I wonder if using NS_HexToRGB() is really what we want to do here. We have to rely on GFX code that might change on us and that is doing more than what we actually want (handles 3 or 6 characters, requires to pass a string without '#' and needs a nscolor as a return value and actually do some computing to set this object). Why not instead writing the parsing code here. It should be easy I believe. Something like: if (aValue.Length() == 7 && aValue[0] == '#') { bool success = true; for (int i=1; i<=7; ++i) { if (!nsCRT::IsAsciiDigit(aValue[i]) && (aValue[i] < 'a' || aValue[i] > 'f') && (aValue[i] < 'A' || aValue[i] > 'F') { success = false; } } if (success) { ToLowerCase(aValue); return; } } aValue.AssignLiteral("#000000"); You could also create a IsValidColor() method but as long as we don't need this code in two places, this is not really required. You could also call ToLowerCase() before parsing. It would make the parsing code simpler. BTW, this code isn't guaranteed to work as-is, It's just to give an idea. @@ +5495,5 @@ > SetBarredFromConstraintValidation(mType == NS_FORM_INPUT_HIDDEN || > mType == NS_FORM_INPUT_BUTTON || > mType == NS_FORM_INPUT_RESET || > mType == NS_FORM_INPUT_SUBMIT || > + mType == NS_FORM_INPUT_COLOR || I think that's wrong. We should allow <input type='color'> to be invalid if the author wants to. For example, by calling setCustomValidity(). ::: content/html/content/src/HTMLInputElement.h @@ +1226,5 @@ > public: > AsyncClickHandler(HTMLInputElement* aInput); > NS_IMETHOD Run(); > + NS_IMETHOD InitFilePicker(); > + NS_IMETHOD InitColorPicker(); Those two methods could probably be protected and return |nsresult| instead of |NS_IMETHOD|. ::: content/html/content/test/forms/test_input_sanitization.html @@ +151,5 @@ > ok(false); > return ""; > case "color": > + match = /^#[0-9A-Fa-f]{6}$/.exec(aValue); > + if (!match) { nit: you could do return /[...]/.exec(aValue) ? aValue.toLowerCase() : "#000000"; @@ +295,5 @@ > + "#00ff00", > + "#000000", > + "red", > + "#0f0", > + "#FFFFAA", Could you add the followings: "FFAABB", "fFAaBb", "FFAAZZ", "ABCDEF", "#7654321" ::: content/html/content/test/forms/test_input_typing_sanitization.html @@ +191,5 @@ > + '#000000', // colors should be "valid simple color" > + '#abcdef', // i.e. start with a '#' followed by a six-length > + '#aabb22', // hex code > + '#ffffff', > + ], I think you should simply remove "{ type: 'color', todo: true }" from the test. We should simply not test input types that shouldn't have a text entry (like <input type='color'> or <input type='range'>). ::: content/html/content/test/test_bug590363.html @@ +71,5 @@ > + testData[j][0] == 'time') { > + expectedValue = '' > + } else { > + expectedValue = '#000000'; > + } This test is terrible (my fault). Let say that if it passes with those changes, this is good enough :) ::: toolkit/components/satchel/test/test_form_autocomplete.html @@ +158,5 @@ > fh.addEntry("field10", "42"); > fh.addEntry("field11", "2010-10-10"); > fh.addEntry("field12", "21:21"); > fh.addEntry("field13", "32"); // not used, since type=range doesn't have a drop down menu > +// fh.addEntry("field14", "#ffffff"); // not used, since type=color doesn't have autocomplete currently You should uncomment this and also write the actual test. Look at <input type=range>'s test.

Also Arnaud, could you give me an inter-diff when you will attach the new patch? (This is the diff representing the changes you did, it makes things easier.)

(In reply to Mounir Lamouri (:mounir) from comment #64) > - we should think about making <input type='color'> handling readonly. The > specs says we shouldn't do that but Webkit and Presto do. https://www.w3.org/Bugs/Public/show_bug.cgi?id=21901

Comment on attachment 742075 [details] [diff] [review] Color input: layout part (2nd) Review of attachment 742075 [details] [diff] [review]: ----------------------------------------------------------------- I haven't checked in detail this patch because there are high level things that I believe should be changed. I'm not sure that we should have nsColorControlFrame inherits from nsHTMLButtonControlFrame. I think the color frame should be a ContainerFrame with one anonymous child that would be a simple div. In other words, if it had to be defined in HTML, it would look like: <div> <div></div> </div> The outer div would be the nsColorControlFrame and the inner div the anonymous element. The outer div would look like a button and the inner div would be a plain div reflecting the current value by having the value as a background color value. The reason why I want to do that is that systems usually have a native color picker button. This is at least the case on GTK and MacOS X. We should show that native widget when we can. Also, we need to make sure the element is stylable. That means that we should provide a pseudo-element for the inner-div. I suggest that we could use ::value (per CSS-3 Selectors). The inner-div might have a few styling restrictions (likely the same as <progress> inner-div) and will not allow the authors to change its 'background' property for obvious reasons. Feel free to ping me here, on IRC or via email if you have any question.

Comment on attachment 743031 [details] [diff] [review] Color input: widget Review of attachment 743031 [details] [diff] [review]: ----------------------------------------------------------------- This patch is missing two major features: - a default color picker the same way there is a default file picker (we could probably use the XUL one for the moment); - showing the native widget for the in-page UI (see layout patch review). Again, feel free to ping me if you have any questions. ::: content/html/content/src/HTMLInputElement.cpp @@ +374,5 @@ > +HTMLInputElement::nsColorPickerShownCallback::nsColorPickerShownCallback( > + HTMLInputElement* aInput, > + nsIColorPicker* aColorPicker) > + : mInput(aInput), > + mColorPicker(aColorPicker) nit: coding style: : foo , bar @@ +385,5 @@ > + if (aResult == nsIColorPicker::returnCancel) { > + return NS_OK; > + } > + > + nsCString color; nsAutoCString (or nsCAutoString but they might both work nowadays) @@ +392,5 @@ > + mInput->SetValue(NS_ConvertUTF8toUTF16(color)); > + > + // The control frame (if there is one) isn't going to send a change > + // event because it will think this is done by a script. > + // So, we can safely send one by ourself. Did you check if that's actually the case for the color frame? @@ +426,5 @@ > { > + // Get parent nsPIDOMWindow object. > + nsCOMPtr<nsIDocument> doc = mInput->OwnerDoc(); > + > + nsPIDOMWindow* win = doc->GetWindow(); nsPIDOMWindow* win = mInput->OwnerDoc()->GetWindow(); ::: content/html/content/src/HTMLInputElement.h @@ +1266,5 @@ > + nsColorPickerShownCallback(HTMLInputElement* aInput, > + nsIColorPicker* aColorPicker); > + > + private: > + ~nsColorPickerShownCallback() {} Why do you need that? ::: widget/gtk2/Makefile.in @@ +35,5 @@ > nsToolkit.cpp \ > nsBidiKeyboard.cpp \ > nsLookAndFeel.cpp \ > nsGtkKeyUtils.cpp \ > + nsColorPicker.cpp \ nit: fix indentation ::: widget/gtk2/nsColorPicker.h @@ +23,5 @@ > + > + nsColorPicker(); > + > +private: > + ~nsColorPicker(); No need to initialize the ctor and dtor if you don't actually use them. @@ +25,5 @@ > + > +private: > + ~nsColorPicker(); > + > +protected: Why not private? ::: widget/gtk2/nsWidgetFactory.cpp @@ +164,5 @@ > + > + nsCOMPtr<nsIColorPicker> picker; > + picker = new nsColorPicker; > + if (!picker) { > + return NS_ERROR_OUT_OF_MEMORY; I believe you can simply do: nsCOMPtr<nsIColorPicker> picker = new nsColorPicker(); |new| should be infallible. ::: widget/nsIColorPicker.idl @@ +29,5 @@ > +[scriptable, uuid(aba40bb6-b15f-43f3-9fbc-30016004ac24)] > +interface nsIColorPicker : nsISupports > +{ > + const short returnOK = 0; // User hit Ok, process selection > + const short returnCancel = 1; // User hit cancel, ignore selection nit: I am wondering if we should have returnOK=1 and returnCancel=0 because that would prevent bugs with developers checking if the return value is true. Though, it would be inconsistent with nsIFilePicker. @@ +41,5 @@ > + * @param title The title for the file widget > + * @param color The color which will selected as the default color > + * > + */ > + void init(in nsIDOMWindow parent, in AString title, in nscolor color); Isn't the use of nscolor going to make the method not callable from JS? @@ +49,5 @@ > + * > + * @return Returns the color currently selected, as a valid simple color (i.e. > + * 7-length hex string starting with a '#') > + */ > + readonly attribute ACString color; We should probably try to stay consistent and have a nscolor if we take a nscolor in init() or take a string in both cases. We should think whether having '#' is really needed.

(In reply to Mounir Lamouri (:mounir) from comment #68) > @@ +41,5 @@ > > + * @param title The title for the file widget > > + * @param color The color which will selected as the default color > > + * > > + */ > > + void init(in nsIDOMWindow parent, in AString title, in nscolor color); > > Isn't the use of nscolor going to make the method not callable from JS? Seems like it: http://www-archive.mozilla.org/scriptable/xpidl/idl-authors-guide/best-practices.html#natives

(In reply to Mounir Lamouri (:mounir) from comment #64) > Finally, a few items that should be taken care of in follow-ups: > - we probably need a preference to toggle this, something like > "dom.forms.color"; How can I do this? I haven't seen similar preference. I thought "color" support is recognized because it's statically added in kInputTypeTable. > - we should implement something for list; I thought this might be added in the second step. About this, I think it will be nice to have something similar to Chrome and Opera: a drop-down list available with the list of colors when |list| attribute is specified. I prefer the Chrome's one, as it displays the list only when |list| is specified, while Opera always displays it, with a default list of colors if |list| isn't specified, but I'm not sure this is really useful. Or we can add those colors to the "custom colors" part of the color picker, if it has one? > - we should implement something for autocomplete; This can also be a drop-down list of colors, or some "custom colors" added to the color picker when possible. But I think the content can be implemented without worrying about the layout part. > > + // Don't return non-sanitized value for types that are experimental on mobile, > > + // and for color > > + if (IsExperimentalMobileType(mType) || mType == NS_FORM_INPUT_COLOR) { > > I do not think you need that for type='color'. I thought we need it because otherwise it might return a non sanitized value, which is not allowed by the spec. I will check though. > Why did you add that? I'm not sure why this code is needed for > type={button,reset,submit} in the first place so I wonder why it is needed > for type='color' and maybe it will help me understand the need for those > other types :) I'm not sure either, I added it just in case. It can probably be removed. > I wonder if using NS_HexToRGB() is really what we want to do here. I used it to avoid duplicated code, and because I thought the unnecessary computation was insignificant. But I can change this and indeed, the parsing algorithm is not very complicated. > I think you should simply remove "{ type: 'color', todo: true }" from the > test. We should simply not test input types that shouldn't have a text entry > (like <input type='color'> or <input type='range'>). IMO we should test values are correctly sanitized anyway, and I'm wondering if it's worth to write another test for non-text input. IMHO it's fine to keep this test with the changes I made. I will update my patch regarding your comments. In the meantime could you please review my questions? Mainly how to use a preference, and if you have some ideas about how we should implement list and autocomplete?

(In reply to Arnaud from comment #70) > (In reply to Mounir Lamouri (:mounir) from comment #64) > > Finally, a few items that should be taken care of in follow-ups: > > - we probably need a preference to toggle this, something like > > "dom.forms.color"; > > How can I do this? > I haven't seen similar preference. > I thought "color" support is recognized because it's statically added in > kInputTypeTable. This is done for <input type='range'>, look for "dom.experimental_forms_range" in HTMLInputElement.cpp (and in mxr to see how the pref is set up). > > - we should implement something for list; > > I thought this might be added in the second step. Sure, that was the idea. > > - we should implement something for autocomplete; That's also for later. I listed those two items to keep in mind what needs to be done to reach 100% completion. > > > + // Don't return non-sanitized value for types that are experimental on mobile, > > > + // and for color > > > + if (IsExperimentalMobileType(mType) || mType == NS_FORM_INPUT_COLOR) { > > > > I do not think you need that for type='color'. > > I thought we need it because otherwise it might return a non sanitized > value, which is not allowed by the spec. > I will check though. Indeed. But given the UI, that shouldn't happen. > > I think you should simply remove "{ type: 'color', todo: true }" from the > > test. We should simply not test input types that shouldn't have a text entry > > (like <input type='color'> or <input type='range'>). > > IMO we should test values are correctly sanitized anyway, and I'm wondering > if it's worth to write another test for non-text input. IMHO it's fine to > keep this test with the changes I made. We should but this patch is to check what is happening when you write something in a field. <input type='color'> will not have an in-page field for users to write a value so this test simply doesn't make sense for that widget. The same way <input type='range'> isn't tested here. > I will update my patch regarding your comments. > In the meantime could you please review my questions? Mainly how to use a > preference, and if you have some ideas about how we should implement list > and autocomplete? List and autocomplete should just have a follow-up bug open, no need to worry more about them for the moment. The UI you proposed look good to me. We might want to use the XUL color picker (that is basically a list of colors). We should probably involve the UI team though. But anyway, this wouldn't block shipping the new type.

Here is the patch updated regarding your comments. I've added a preference to activate color, false by default but activated for tests, called "dom.experimental_forms_color" to be coherent with the already existing "dom.experimental_forms_range".

And here is the inter-diff. The only thing which is missing is the update of test_input_typing_sanitization.html, but, as you can see in the complete patch I just attached, I just removed the color from the list of types to test for this test, as discussed.

Comment on attachment 750105 [details] [diff] [review] Color input: content part (3rd) Review of attachment 750105 [details] [diff] [review]: ----------------------------------------------------------------- I just noticed few minor things which should be fixed. This should not prevent you to review the patch though. ::: content/html/content/src/HTMLInputElement.cpp @@ +81,5 @@ > #include "nsImageLoadingContent.h" > #include "imgRequestProxy.h" > > +// input type=color > +#include "nsColor.h" This was needed to have NS_HexToRGB, which we don't use anymore. Should be removed. @@ +395,5 @@ > + } > + return NS_ERROR_FAILURE; > +} > + > +NS_IMETHODIMP I forgot to update this one (nsresult instead of NS_IMETHODIMP). Windows 7 build is failing because of this. @@ +403,5 @@ > + return NS_OK; > +} > + > +NS_IMETHODIMP > +HTMLInputElement::AsyncClickHandler::InitFilePicker() And this one. ::: content/html/content/test/forms/test_required_attribute.html @@ +366,5 @@ > checkInputRequiredNotApply(type, true); > } > > // Then, checks for the types which do not use the required attribute. > // TODO: check 'color' when they will be implemented. Oops: I updated the code but forgot to remove this (now useless) comment. ::: modules/libpref/src/init/all.js @@ +745,5 @@ > > // Don't use new input types > pref("dom.experimental_forms", false); > > // Don't enable <input type=range> yet: Btw, this comment isn't valid anymore right?

Comment on attachment 750105 [details] [diff] [review] Color input: content part (3rd) Review of attachment 750105 [details] [diff] [review]: ----------------------------------------------------------------- r=me with your comments (#include and s/NS_IMETHODIMP/nsresult) and my comments below applied. ::: content/html/content/src/HTMLInputElement.cpp @@ +3577,5 @@ > +} > + > +bool HTMLInputElement::IsValidSimpleColor(const nsAString& aValue) const > +{ > + if (aValue.Length() == 7 && aValue.First() == '#') { Given that we can do early return, you can improve the readability by doing: if (aValue.Length() != 7 || aValue.First() != '#') { return false; } @@ +3578,5 @@ > + > +bool HTMLInputElement::IsValidSimpleColor(const nsAString& aValue) const > +{ > + if (aValue.Length() == 7 && aValue.First() == '#') { > + for (int i = 1; i < 7; i++) { nit: s/i++/++i/ @@ +3587,5 @@ > + } > + } > + return true; > + } > + return false; If you do the early return at the beginning, you can simply |return true;| after the loop. @@ +3797,5 @@ > !Preferences::GetBool("dom.experimental_forms", false)) || > (newType == NS_FORM_INPUT_RANGE && > + !Preferences::GetBool("dom.experimental_forms_range", false)) || > + (newType == NS_FORM_INPUT_COLOR && > + !Preferences::GetBool("dom.experimental_forms_color", false))) { I would prefer "dom.forms.color". Those other names are terrible. ::: content/html/content/test/forms/test_pattern_attribute.html @@ +265,1 @@ > // TODO: 'datetime', 'month', 'week', 'datetime-local' and 'color' nit: could you remove 'color' from the TODO list? ::: modules/libpref/src/init/all.js @@ +749,5 @@ > // Don't enable <input type=range> yet: > pref("dom.experimental_forms_range", true); > > +// Don't enable <input type=color> yet: > +pref("dom.experimental_forms_color", false); Could you call this: "dom.forms.color" ? ::: testing/profiles/prefs_general.js @@ +9,5 @@ > user_pref("dom.allow_scripts_to_close_windows", true); > user_pref("dom.disable_open_during_load", false); > user_pref("dom.experimental_forms", true); // on for testing > user_pref("dom.experimental_forms_range", true); // on for testing > +user_pref("dom.experimental_forms_color", true); // on for testing ditto

Comment on attachment 750106 [details] [diff] [review] Color input: content part (3rd) (intermediate diff from 2nd) Review of attachment 750106 [details] [diff] [review]: ----------------------------------------------------------------- Thank you for providing the inter-diff :)

Here is the updated patch, including your comments applied. I've pushed it to try: https://tbpl.mozilla.org/?tree=Try&rev=4f13f8fa0e5a

Comment on attachment 752417 [details] [diff] [review] Color input: content part (4th) Review of attachment 752417 [details] [diff] [review]: ----------------------------------------------------------------- r=me For the record, when you get a "r=me with ...", you don't need to ask for another review if you apply the requested changes. I think the best way to proceed would be to open different bugs for content/layout/osx/linux/android/fxos/windows and mark all of them blocking this one. It would be easier, don't you think? If you do that, you can just attach this patch in the content bug and land (or put 'checkin-needed'). You can also do all the work in this bug but it might be a bit hard to follow at some point. As you prefer.

Indeed, it will be easier to split this in different bugs and keep that one as the master bug. I've already created bug 875274 for the content part. I will create bugs for the remaining steps. Just one thing, it seems I can't assign the bugs to myself. Could you please do it? At least for the content and the layout part, as I'm not sure I will implement widgets for every platform. Or let me know if I should ask for special permissions to be able to do it myself.

Comment on attachment 752417 [details] [diff] [review] Color input: content part (4th) Marking the patch as obsolete given that it has been landed in bug 875274.

(In reply to Arnaud from comment #79) > Just one thing, it seems I can't assign the bugs to myself. > Could you please do it? At least for the content and the layout part, as I'm > not sure I will implement widgets for every platform. > Or let me know if I should ask for special permissions to be able to do it > myself. You should be able to modify bugs yourself now ;)

Hi there, Things seem to go pretty well! I believe this won't be an easy question, but by any chance would you have in mind any approximate release date for this feature? Thank you! S.

In Firefox features are not date driven. Until the code is checked in to our Nightly builds we can't say. When this bug is fixed you can correlate the 'Target Milestone' value at https://wiki.mozilla.org/RapidRelease/Calendar to the earliest possible release date. Occasionally features are disabled in the Aurora or Beta cycle because of issues with the implementation, thus the date is not set in stone.

The color widget is currently a very narrow vertical bar, see screenshot or see live at http://var.mpopp.net/bdemo/forms.php (with dom.forms.color enabled of course).

(In reply to Markus Popp from comment #84) > The color widget is currently a very narrow vertical bar, see screenshot or > see live at http://var.mpopp.net/bdemo/forms.php (with dom.forms.color > enabled of course). Yes, that is normal: this bug isn't fixed which means the feature isn't fully implemented. dom.forms.color enable the content part of this feature. Bug 875275 is about implementing the layout part.

(In reply to Markus Popp from comment #84) > Created attachment 785905 [details] > color.png > > The color widget is currently a very narrow vertical bar, see screenshot or > see live at http://var.mpopp.net/bdemo/forms.php (with dom.forms.color > enabled of course). Note that if you use Linux/GTK, you should get a color picker when clicking on that narrow vertical bar. This is work in progress for the moment.

(In reply to Mounir Lamouri (:mounir) from comment #86) > Note that if you use Linux/GTK, you should get a color picker when clicking > on that narrow vertical bar. This is work in progress for the moment. That's right. This is why it seems this feature is almost ready for use if only this box was a bit wider.

Comment on attachment 743031 [details] [diff] [review] Color input: widget That patch has moved to another bug.

Comment on attachment 742075 [details] [diff] [review] Color input: layout part (2nd) That one too.

(In reply to Markus Popp from comment #87) > (In reply to Mounir Lamouri (:mounir) from comment #86) > > Note that if you use Linux/GTK, you should get a color picker when clicking > > on that narrow vertical bar. This is work in progress for the moment. > > That's right. This is why it seems this feature is almost ready for use if > only this box was a bit wider. The layout code is being worked on in bug 875275. When this will be fixed, you should see something more useful ;)

Now layout has been landed (bug 875275) this input is more usable :) But "dom.forms.color" pref is still false by default and should be toggle to have this input working. I guess we will probably wait a bit before defaulting it to true. We only missed few backend implementations. But everything has been implemented for some platforms (namely Linux/Gtk, Windows and Mac OS) so I'm wondering if it would be possible to turn on the pref only for these platforms? (not necessarily now, if we want to keep it in testing stage for some time) Looking at all.js, it seems we can do so, but I'm not sure it's common practice to do this for form related preferences. And I don't know who should decide when it's time to change a pref to have a new feature activated by default.

@Jonathan: as you flipped the pref for input range (bug 841950 and bug 841948), you might be able to help me/answer my questions (see comment 91 above).

I've not been closely following progress here, but if you consider this ready for release on those platforms, then it sounds reasonable to enable it for them now. We can always disable it on branches if wider testing reveals issues that should block release. Since Daniel has been more closely involved it would be good to have his input too. That can be in the form of a patch to flip the pref though where you request review from him.

Tested it in Firefox Nightly 27.0a1 (2013-10-27) and here are some things i've noticed: - If i change the color in the color-picker the color of the input should also be changed (but no change event should be fired) - The cursor should be "default" => actual i get "text" cursor - The display of the color is too wide

(In reply to Stefan Kienzle from comment #94) > Tested it in Firefox Nightly 27.0a1 (2013-10-27) and here are some things > i've noticed: The attribute isn't fully implemented, and that's why this bug isn't closed. That's also why the related pref isn't turned on by default. > - If i change the color in the color-picker the color of the input should > also be changed (but no change event should be fired) On Mac OS, we do something like this IIRC (and we don't fire a "change" event but an "update" event I think). I'm not sure, but we might have do this because of a system limitation. We might do something like this on other platforms, but I'm not sure about the benefits: when you select a color in the color picker, the color you have selected is clearly visible. I don't see the advantage of also updating the input, and this might be confusing for users (they don't have "hit" "OK" yet, and the input is updated anyway). > - The cursor should be "default" => actual i get "text" cursor > - The display of the color is too wide These have been changed as part of bug 928891 (whose patches have not been landed yet, but pushed to try if you want to test: https://tbpl.mozilla.org/?tree=Try&rev=f7dbf60e0cad).

Filed bug 943966 which is related to <input type="color"> implementation.

What's the status here? Arnaud, are you still on this?

This bug was kept as a meta bug to track the implementation of input type color, which is now implemented on all platforms. Android is implemented, but has a very basic color picker, so bug 875750 is still opened to have a better one implemented. A patch is attached, but I never had the time to polish it. What else is missing is implementing the list (bug 960984) and autocomplete (bug 960989) attributes, because I wasn't sure how this should look like. There is also a minor bug to make it look nicer (bug 949403). So we kept this bug opened (we might have closed it and mark the others as "depends on" this one instead of blockers). I don't think I will work on those improvements anytime soon. However, I willing to help fixing bugs reported related to the basic functionality of this not working (like bug 1389908).

(In reply to Arnaud Bienner from comment #98) > This bug was kept as a meta bug to track the implementation of input type > color, which is now implemented on all platforms. If we want a tracker for improvements to what we shipped then we should do what we normally do and spin off a tracking bug. Otherwise there's no delination between the bug for shipping, and the probably never done list of improvements that could be done, and that's a pain.

I filed bug 1445061 as a tracker bug for further improvements.

We made a good job documenting this a while ago: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/color