Last Comment Bug 567283 - Add support for 4 and 8 CSS hexcolor values (#RRGGBBAA and #RGBA)
: Add support for 4 and 8 CSS hexcolor values (#RRGGBBAA and #RGBA)
Status: RESOLVED FIXED
[parity-webkit]
: dev-doc-complete, site-compat
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: unspecified
: All All
: -- enhancement with 7 votes (vote)
: mozilla49
Assigned To: David Baron :dbaron: ⌚️UTC+1 (busy September 14-25)
:
Mentors:
Depends on: 1304810
Blocks: 1271191
  Show dependency treegraph
 
Reported: 2010-05-20 17:57 PDT by A. Lepe
Modified: 2016-09-22 15:12 PDT (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed

MozReview Requests
Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:
Show discarded requests

Attachments
Possible changes required to allow #RRGGBBAA and #RGBA in gecko (1.52 KB, text/plain)
2010-05-20 18:04 PDT, A. Lepe
no flags Details
MozReview Request: Bug 567283 patch 1 - Convert if enclosing most of function into early return. r?xidorn (58 bytes, text/x-review-board-request)
2016-05-08 09:56 PDT, David Baron :dbaron: ⌚️UTC+1 (busy September 14-25)
xidorn+moz: review+
Details | Review
MozReview Request: Bug 567283 patch 2 - Rename NS_HexToRGB to NS_HexToRGBA and add parameter saying whether 4 and 8 digit colors are allowed. r?xidorn (58 bytes, text/x-review-board-request)
2016-05-08 09:56 PDT, David Baron :dbaron: ⌚️UTC+1 (busy September 14-25)
xidorn+moz: review+
Details | Review
MozReview Request: Bug 567283 patch 3 - Make TestColorNames use the new alpha capabilities. r?xidorn (58 bytes, text/x-review-board-request)
2016-05-08 09:56 PDT, David Baron :dbaron: ⌚️UTC+1 (busy September 14-25)
xidorn+moz: review+
Details | Review
MozReview Request: Bug 567283 patch 4 - Support #rgba and #rrggbbaa colors in CSS. r?xidorn (58 bytes, text/x-review-board-request)
2016-05-08 09:56 PDT, David Baron :dbaron: ⌚️UTC+1 (busy September 14-25)
xidorn+moz: review+
Details | Review

Description A. Lepe 2010-05-20 17:57:33 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.5pre) Gecko/20100514 Ubuntu/9.10 (karmic) Firefox/3.6.3pre
Build Identifier: 

This feature is proposed for CSS3 Color, worst case will be in CSS4 Color.
It has been also reported in Webkit:
https://bugs.webkit.org/show_bug.cgi?id=39140
And filed accordingly in:
http://www.w3.org/Style/CSS/Tracker/issues/124
(related email discussion can be found in the previous link)

Reproducible: Always

Actual Results:  
unsupported


Some of the software using #rrggbbaa annotation:
   Google Charts API
   Inkscape
   Java Synth Look and Feel
   3DMLW (3D Markup Language)
   GML (Graph Modeling Language)
   iCal for Mac (in theme files)
   Maemo (Nokia)
   swfmill
   gbui for JME (Java Game engine), based in CSS
   wxPython
   Qt UI framework
   lcd4linux
   xine (OSD config)
   xmCHART for FileMaker Pro
   gwyddion
   skEdit (themes)
   Pixane
Comment 1 A. Lepe 2010-05-20 18:04:15 PDT
Created attachment 446629 [details]
Possible changes required to allow #RRGGBBAA and #RGBA in gecko

It is not exactly a patch, as my skills in C++ are limited, but I believe those are the required changes. I suggest to review it by perform the required modifications.
Comment 2 Boris Zbarsky [:bz] (TPAC) 2010-05-20 18:18:29 PDT
This has the same issues as the webkit patch; it changes the way HTML color attributes are handled in a way that doesn't obiously seem compatible to me...
Comment 3 Zack Weinberg (:zwol) 2010-05-20 18:30:50 PDT
The conservative thing would be to define a new NS_HexToRGBA() that allowed the four/eight digit syntax, and use that for #xxxxx colors in CSS but leave the old parsing for HTML color attributes.  We discourage use of HTML color attributes anyway, so if they don't get the new feature, no great loss.

On the other hand, if we *can* allow the new notation in HTML color attributes without compatibility issues, it seems pointless not to (and then we wouldn't need two parsers).

ccing hixie for HTML5 perspective.
Comment 4 Boris Zbarsky [:bz] (TPAC) 2010-05-20 18:34:58 PDT
As I just commented in the webkit bug, if this syntax is supported then:

  color="000000ff"

goes from blue in quirks mode and ignored in standards mode to black.

  color="0000"

goes from black in quirks mode and ignored in standards mode to transparent.

I really don't think we can start supporting this syntax in HTML attributes without at least some breakage.

Doing it just in CSS might be easier, since we don't do NS_LooseHexToRGB there.
Comment 5 A. Lepe 2010-05-20 19:10:05 PDT
<html>
<head>
</head>
<body>
1: <font color="#005F">This is blue</font><br>
2: <font color="#000055FF">This is blue</font><br>
3: <font style="color: #005F">This is blue</font><br>
4: <font style="color: #000055FF">This is blue</font><br>
</body>
</html>

Firefox (quirk mode):
 (1) green
 (2) blue
 (3,4) black.

Firefox (standard mode):
 (all) black
Comment 6 Boris Zbarsky [:bz] (TPAC) 2010-05-20 19:14:40 PDT
> (3,4) black.
> (all) black

Not black.  Whatever the color would have been without the attempt to change it.

But yes, that's what comment 4 said.
Comment 7 Tab Atkins Jr. 2010-05-20 19:41:34 PDT
Agreed that the parsing of HTML colors may run into compat concerns.

I've done some compat testing on CSS colors, though, using the 450k pages in the DotBot index.  Only about 150 pages showed up as using 4/8 hexit colors in color or *-color properties, from about 100 unique sites.  Most of the uses would cause minor breakage when they start being valid, though.
Comment 8 A. Lepe 2010-05-20 23:13:44 PDT
I also agree that this should only be added into CSS colors and not in HTML properties. As Zack commented, people should not be using the HTML color attribute anyway.
Comment 9 Hixie (not reading bugmail) 2010-05-22 01:55:04 PDT
We should just do whatever the specs say. Not sure what that is for CSS (is there a proposal to support this in CSS3? If so, and if it's not yet in CR, then the thing to do is to support it as -moz-#... rather than just #.... In HTML, the attributes are to be parsed as described here:

   http://www.whatwg.org/specs/web-apps/current-work/complete/common-microsyntaxes.html#rules-for-parsing-a-legacy-color-value

I wouldn't recommend changing that.
Comment 10 Xidorn Quan [:xidorn] (UTC+10) 2015-11-08 17:00:34 PST
This has been implemented in WebKit in this bug: https://bugs.webkit.org/show_bug.cgi?id=150853
Comment 11 Ian Thomas ('thelem') 2016-02-22 03:27:08 PST
This is in CSS Color Level 4 drafts https://drafts.csswg.org/css-color/#hex-notation
Comment 12 noel gordon 2016-05-06 21:25:33 PDT
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #10)
> This has been implemented in WebKit in this bug: https://bugs.webkit.org/show_bug.cgi?id=150853

... and https://bugs.webkit.org/show_bug.cgi?id=157402
Comment 14 David Baron :dbaron: ⌚️UTC+1 (busy September 14-25) 2016-05-07 13:35:52 PDT
Does anybody know whether what is being implemented in Chromium and WebKit affects only colors specified in CSS, or whether it also affects colors in HTML attributes?
Comment 15 David Baron :dbaron: ⌚️UTC+1 (busy September 14-25) 2016-05-07 14:12:05 PDT
(In reply to David Baron [:dbaron] ⌚️UTC-7 (review requests must explain patch) (busy May 9-13) from comment #14)
> Does anybody know whether what is being implemented in Chromium and WebKit
> affects only colors specified in CSS, or whether it also affects colors in
> HTML attributes?

Even if they did (which I hope they didn't), it seems like a bad idea per the discussion above, so I'll just implement this for CSS.
Comment 16 David Baron :dbaron: ⌚️UTC+1 (busy September 14-25) 2016-05-07 22:56:53 PDT
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b84463a26494
Comment 17 Robert Longson 2016-05-08 02:05:05 PDT
Pach 3, the comment that ends (usually multiplier, different behavior that percent)

I think that should be replaced by than
Comment 18 David Baron :dbaron: ⌚️UTC+1 (busy September 14-25) 2016-05-08 09:56:52 PDT
Created attachment 8750091 [details]
MozReview Request: Bug 567283 patch 1 - Convert if enclosing most of function into early return.  r?xidorn

This conversion just reindents most of the function, but it then avoids
having indentation changes in patch 2.

Review commit: https://reviewboard.mozilla.org/r/51291/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51291/
Comment 19 David Baron :dbaron: ⌚️UTC+1 (busy September 14-25) 2016-05-08 09:56:52 PDT
Created attachment 8750092 [details]
MozReview Request: Bug 567283 patch 2 - Rename NS_HexToRGB to NS_HexToRGBA and add parameter saying whether 4 and 8 digit colors are allowed.  r?xidorn

This patch tells all callers to use the existing behavior, so it is
intended not to change behavior.  Callers that will be modified in later
patches are marked with "FIXME" comments that will be removed in those
later patches (patches 3 and 4).

Review commit: https://reviewboard.mozilla.org/r/51293/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51293/
Comment 20 David Baron :dbaron: ⌚️UTC+1 (busy September 14-25) 2016-05-08 09:56:52 PDT
Created attachment 8750093 [details]
MozReview Request: Bug 567283 patch 3 - Make TestColorNames use the new alpha capabilities.  r?xidorn

Review commit: https://reviewboard.mozilla.org/r/51295/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51295/
Comment 21 David Baron :dbaron: ⌚️UTC+1 (busy September 14-25) 2016-05-08 09:56:52 PDT
Created attachment 8750094 [details]
MozReview Request: Bug 567283 patch 4 - Support #rgba and #rrggbbaa colors in CSS.  r?xidorn

This adds support for #rgba and #rrggbbaa colors to CSS.  This feature
is specified in https://drafts.csswg.org/css-color-4/#hex-notation .

This adds new types to nsCSSValue so that we can serialize the syntax
that was specified, as we do for other distinctions in how colors are
specified.

It does not change the behavior of the hashless color quirk, which
continues to support only 3 and 6 digit colors as specified in
https://quirks.spec.whatwg.org/#the-hashless-hex-color-quirk (step 4).

This changes property_database.js to remove various uses of 4 and 8
digit colors as invalid values.  It then adds them in slightly fewer
places as valid values, but more thoroughly testing both initial and
non-initial values on 'color'.

It marks two canvas tests explicitly testing this feature as no longer
known to fail by removing their .ini files.

Finally, it adjusts the web platform test testing the hashless color
quirk to no longer treat 4 and 8 digit colors as invalid values.
Removing the relevant test items seems like the right thing since
they're in a section where 3 and 6 digit colors were skipped but other
lengths were tested.  Modifying this imported test is OK since:
  <jgraham> dbaron: Commit the change you want to m-c, it is
    (semi-)automatically upstreamed every so often (typically
    about once a week)

Review commit: https://reviewboard.mozilla.org/r/51297/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51297/
Comment 22 Xidorn Quan [:xidorn] (UTC+10) 2016-05-08 16:29:48 PDT
Comment on attachment 8750092 [details]
MozReview Request: Bug 567283 patch 2 - Rename NS_HexToRGB to NS_HexToRGBA and add parameter saying whether 4 and 8 digit colors are allowed.  r?xidorn

https://reviewboard.mozilla.org/r/51293/#review47945

::: gfx/src/nsColor.cpp:78
(Diff revision 1)
> -bool NS_HexToRGB(const nsAString& aColorSpec, nscolor* aResult)
> +bool NS_HexToRGBA(const nsAString& aColorSpec, nsHexColorType aType,
> +                  nscolor* aResult)

As far as you are here, probably consider giving the return type it's own line like what you did for the declaration.
Comment 23 Xidorn Quan [:xidorn] (UTC+10) 2016-05-08 16:30:16 PDT
https://reviewboard.mozilla.org/r/51291/#review47947
Comment 24 Xidorn Quan [:xidorn] (UTC+10) 2016-05-08 16:34:45 PDT
Comment on attachment 8750093 [details]
MozReview Request: Bug 567283 patch 3 - Make TestColorNames use the new alpha capabilities.  r?xidorn

https://reviewboard.mozilla.org/r/51295/#review47951
Comment 25 Xidorn Quan [:xidorn] (UTC+10) 2016-05-08 17:19:17 PDT
Comment on attachment 8750094 [details]
MozReview Request: Bug 567283 patch 4 - Support #rgba and #rrggbbaa colors in CSS.  r?xidorn

https://reviewboard.mozilla.org/r/51297/#review47959

> Finally, it adjusts the web platform test testing the hashless color
> quirk to no longer treat 4 and 8 digit colors as invalid values.

When read this sentence, I wondered why, because you said earlier that we do not change the behavior of hashless color quirk in terms of 4 and 8 digit colors. But later I realized that you mean no longer treat 4 and 8 digit (not hashless) colors as invalid values.

::: layout/style/nsCSSParser.cpp:6614
(Diff revision 1)
>    nsCSSToken* tk = &mToken;
>    nscolor rgba;
>    switch (tk->mType) {
>      case eCSSToken_ID:
>      case eCSSToken_Hash:
>        // #xxyyzz

This comment could probably be updated accordingly?
Comment 26 Tab Atkins Jr. 2016-05-08 20:47:04 PDT
(In reply to David Baron [:dbaron] ⌚️UTC-7 (review requests must explain patch) (busy May 9-13) from comment #14)
> Does anybody know whether what is being implemented in Chromium and WebKit
> affects only colors specified in CSS, or whether it also affects colors in
> HTML attributes?

Chrome's patch only affects CSS.  WebKit accidentally did affect HTML, but Dean just fixed it when we pointed it out to him.
Comment 27 David Baron :dbaron: ⌚️UTC+1 (busy September 14-25) 2016-05-08 22:17:03 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e34b5aaeec01d4c832d1dd8f63fc45b53aa5a9e
Bug 567283 patch 1 - Convert if enclosing most of function into early return.  r=xidorn

https://hg.mozilla.org/integration/mozilla-inbound/rev/c913af2812365dfc8f63e27c15e8763a00db4bae
Bug 567283 patch 2 - Rename NS_HexToRGB to NS_HexToRGBA and add parameter saying whether 4 and 8 digit colors are allowed.  r=xidorn

https://hg.mozilla.org/integration/mozilla-inbound/rev/a9205982d844c80e8d2af5e84994984e631053f7
Bug 567283 patch 3 - Make TestColorNames use the new alpha capabilities.  r=xidorn

https://hg.mozilla.org/integration/mozilla-inbound/rev/cf57eb3d5080fa6d95bc956c6d870366d6f3840a
Bug 567283 patch 4 - Support #rgba and #rrggbbaa colors in CSS.  r=xidorn
Comment 29 Sebastian Zartner [:sebo] 2016-05-18 03:01:23 PDT
This is now described at https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#rgb%28%29 and https://developer.mozilla.org/en-US/Firefox/Releases/49#CSS.

Chris, is that enough or should this be mentioned more broadly? And if so, where?

Sebastian
Comment 30 Ian Thomas ('thelem') 2016-05-18 07:28:20 PDT
Slight error in this part:

> "#", followed by eight hexadecimal characters (0-9, A-F), where the first digit represents the red part, the second the green part, the third two the blue part and the last two the transparency.

That should say "first two digits represent" and "the second two"
Comment 31 Chris Mills (Mozilla, MDN editor) [:cmills] 2016-05-18 09:11:35 PDT
(In reply to Sebastian Zartner [:sebo] from comment #29)
> This is now described at
> https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#rgb%28%29 and
> https://developer.mozilla.org/en-US/Firefox/Releases/49#CSS.
> 
> Chris, is that enough or should this be mentioned more broadly? And if so,
> where?
> 
> Sebastian

Thanks Sebastian! This mostly looked fine; I've moved the descriptions down to here:

https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#rgba()

As they are really ways to represent RGBA colors, not RGB. I also fixed the issue pointed out by Ian.
Comment 32 Chris Mills (Mozilla, MDN editor) [:cmills] 2016-05-18 09:25:06 PDT
I've also updated the support info, as Chrome appears to support it, but Safari doesn't.
Comment 33 Sebastian Zartner [:sebo] 2016-05-18 22:54:06 PDT
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #31)
> Thanks Sebastian! This mostly looked fine; I've moved the descriptions down
> to here:
> 
> https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#rgba()
> 
> As they are really ways to represent RGBA colors, not RGB. I also fixed the
> issue pointed out by Ian.

Thank you for fixing it!

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #32)
> I've also updated the support info, as Chrome appears to support it, but
> Safari doesn't.

I don't know how Google's release strategy looks like, though while it works in Canary, the related bug[1] is not marked as fixed yet and the Chromium platform status[2] also still claims 'In development'. That's why I previously added no compatibility for it. But I agree that it's better to reflect the support in Canary. So, given that, I've updated the status and added a compatibility note.

WebKit, on the other side, claims that it already got implemented[3] in November last year in contrast to the bug status of the original report[4]. Though I don't have OS X, so I couldn't try it out.

Anyway, with those changes in place, I mark this now as ddc.

Sebastian

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=76362
[2] https://www.chromestatus.com/feature/5685348285808640
[3] https://bugs.webkit.org/show_bug.cgi?id=150853
[4] https://bugs.webkit.org/show_bug.cgi?id=39140
Comment 34 noel gordon 2016-05-23 05:03:51 PDT
(In reply to Sebastian Zartner [:sebo] from comment #33)

> I don't know how Google's release strategy looks like, though while it works
> in Canary, the related bug[1] is not marked as fixed yet and the Chromium
> platform status[2] also still claims 'In development'.

I will close the chromium bug [1] when developer tools are done.  I've updated chrome status [1] to state that the feature will ship in Chrome 52.

> [1] [1] https://bugs.chromium.org/p/chromium/issues/detail?id=76362
> [2] https://www.chromestatus.com/feature/5685348285808640
Comment 35 noel gordon 2016-05-23 05:04:49 PDT
ahem ... chrome status _[2]_ to state ...
Comment 36 Sebastian Zartner [:sebo] 2016-05-24 05:10:23 PDT
(In reply to noel gordon from comment #34)
> (In reply to Sebastian Zartner [:sebo] from comment #33)
> 
> > I don't know how Google's release strategy looks like, though while it works
> > in Canary, the related bug[1] is not marked as fixed yet and the Chromium
> > platform status[2] also still claims 'In development'.
> 
> I will close the chromium bug [1] when developer tools are done.  I've
> updated chrome status [1] to state that the feature will ship in Chrome 52.
> 
> > [1] [1] https://bugs.chromium.org/p/chromium/issues/detail?id=76362
> > [2] https://www.chromestatus.com/feature/5685348285808640

Thank you for the info! I've updated the compatibility information accordingly.

Sebastian
Comment 37 Mike Taylor [:miketaylr] 2016-05-27 15:22:08 PDT
Adding site-compat keyword, there can be some unintended side-effects: https://github.com/webcompat/web-bugs/issues/2628.
Comment 38 Kohei Yoshino [:kohei] 2016-05-30 07:55:18 PDT
Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/support-for-rgba-colour-values-may-validate-previously-invalid-values/
Comment 39 Mike Taylor [:miketaylr] 2016-05-31 12:53:07 PDT
(Thanks Kohei!)

Note You need to log in before you can comment on or make changes to this bug.