data icon indicating copy to clipboard operation
data copied to clipboard

Incorrect value for rgb and rgba functions

Open pyoor opened this issue 6 years ago • 5 comments

It appears that both the rgb and rgba value definitions are incorrect.

https://github.com/mdn/data/blob/a2e2d7adb53f7906985b50e30dc3c6fd9c200652/css/syntaxes.json#L531 rgb( <percentage>{3} [ / <alpha-value> ]? ) | rgb( <number>{3} [ / <alpha-value> ]? ) | rgb( <percentage>#{3} , <alpha-value>? ) | rgb( <number>#{3} , <alpha-value>? )

From https://drafts.csswg.org/css-color/#rgb-functions: rgb() = rgb( <percentage>{3} [ / <alpha-value> ]? ) | rgb( <number>{3} [ / <alpha-value> ]? )

I'm not sure if the rgb( <number>#{3} , <alpha-value>? ) format is listed in another spec but if so, it likely should be the following so as not to allow for a trailing comma. rgb( <number>#{3} [, <alpha-value>]? )

pyoor avatar Jan 07 '19 17:01 pyoor

It appears that my original assessment is incorrect due to the rules for handling combining commas: https://drafts.csswg.org/css-values-4/#comb-comma

However, can you point me to the spec that defines the comma-separated alpha value?

pyoor avatar Jan 07 '19 18:01 pyoor

CSS Color L4 introduces new syntaxes for rgb() and hsl():

  • no comma between color components
  • alpha-value can be specified (optional) And with those changes rgba() and hsla() is not needed anymore.

However, spec claims:

For legacy reasons, rgb() also supports an alternate syntax that separates all of its arguments with commas

So, the syntax of rgb() in mdn/data is a combination of new and old syntaxes.

Spec also claims:

Also for legacy reasons, an rgba() function also exists, with an identical grammar and behavior to rgb()

That's why rgb() and rgba() syntaxes have their form. The same for hsl() and hsla().

Following syntaxes are similar:

rgb( <number>#{3} , <alpha-value>? )
rgb( <number>#{3} [, <alpha-value>]? )

They work the same: exactly 3 comma separated numbers and optional alpha value. In case alpha value is used it must be preceded by a comma. First syntax is valid due to the rule that commas are implicitly omissible (as you pointed up in your second comment – https://drafts.csswg.org/css-values-4/#comb-comma).

So, no mistake in those syntaxes.

It's not clear to me what do you mean: "spec that defines the comma-separated alpha value"? Could you clarify?

lahmatiy avatar Jun 26 '19 22:06 lahmatiy

@pyoor the format with the rgb( <number>#{3} , <alpha-value>? ) is later in the same section at: https://drafts.csswg.org/css-color/#rgb-functions

@lahmatiy I have a browser testing project that I'm working on (https://github.com/kanaka/bartender) and part of is translating HTML and CSS standards data into EBNF grammars (https://github.com/kanaka/html5-css3-ebnf). The comma combining rule is a pain when doing this translation because it is contextual and requires backtracking or lookahead. The rgb and rgba definitions seem like they might be the only use of that construct within this repo. What do you think of the idea of updating the rgb and rgba syntax definitions so that they are easier to parse with the comma and alpha value wrapped in brackets? It actually makes it more consistent with the [ / <alpha-value> ]? that appears earlier in both of those rules.

--- a/css/syntaxes.json
+++ b/css/syntaxes.json
@@ -564,10 +564,10 @@
     "syntax": "repeating-radial-gradient( [ <ending-shape> || <size> ]? [ at <position> ]? , <color-stop-list> )"
   },
   "rgb()": {
-    "syntax": "rgb( <percentage>{3} [ / <alpha-value> ]? ) | rgb( <number>{3} [ / <alpha-value> ]? ) | rgb( <percentage>#{3} , <alpha-value>? ) | rgb( <number>#{3} , <alpha-value>? )"
+    "syntax": "rgb( <percentage>{3} [ / <alpha-value> ]? ) | rgb( <number>{3} [ / <alpha-value> ]? ) | rgb( <percentage>#{3} [ , <alpha-value> ]? ) | rgb( <number>#{3} [ , <alpha-value> ]? )"
   },
   "rgba()": {
-    "syntax": "rgba( <percentage>{3} [ / <alpha-value> ]? ) | rgba( <number>{3} [ / <alpha-value> ]? ) | rgba( <percentage>#{3} , <alpha-value>? ) | rgba( <number>#{3} , <alpha-value>? )"
+    "syntax": "rgba( <percentage>{3} [ / <alpha-value> ]? ) | rgba( <number>{3} [ / <alpha-value> ]? ) | rgba( <percentage>#{3} [ , <alpha-value> ]? ) | rgba( <number>#{3} [ , <alpha-value> ]? )"
   },
   "rotate()": {
     "syntax": "rotate( <angle> )"

kanaka avatar Aug 16 '19 20:08 kanaka

First of all, mdn/data mostly reflects specifications with no changes. If it will start to adapt syntaxes for any tool it ended up as unmaintainable dictionary.

The rgb and rgba definitions seem like they might be the only use of that construct within this repo.

Nope, there are much more syntaxes with such case. Moreover, patterns [, <whatever>]? are replacing to , <whatever>? in new spec editions for simplicity. I suggested some PRs with such changes recently that were merged to specs (for example, https://github.com/w3c/csswg-drafts/pull/4054)

It actually makes it more consistent with the [ / <alpha-value> ]? that appears earlier in both of those rules

Delimiter / has no implicit omissible behaviour, so that's different actually. And, since mdn/data just reflects specs, any suggestions on syntax definition should be proposed to https://github.com/w3c/csswg-drafts

lahmatiy avatar Aug 22 '19 15:08 lahmatiy

Btw, I'm not sure it's possible to express CSS syntax definition in EBNF. Because it has rules much complicated than optional omissible comma, for example low priority matching (see some notes about it here) or &&- and ||- groups which requires context buffer (your implementation of such groups is incorrect at the moment).

lahmatiy avatar Aug 22 '19 15:08 lahmatiy