Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect value for rgb and rgba functions #341

Open
pyoor opened this issue Jan 7, 2019 · 5 comments
Open

Incorrect value for rgb and rgba functions #341

pyoor opened this issue Jan 7, 2019 · 5 comments
Labels
idle Issues and pull requests with no activity for three months.

Comments

@pyoor
Copy link
Contributor

pyoor commented Jan 7, 2019

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

"syntax": "rgb( <percentage>{3} [ / <alpha-value> ]? ) | rgb( <number>{3} [ / <alpha-value> ]? ) | rgb( <percentage>#{3} , <alpha-value>? ) | rgb( <number>#{3} , <alpha-value>? )"

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
Copy link
Contributor Author

pyoor commented Jan 7, 2019

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?

@lahmatiy
Copy link
Contributor

lahmatiy commented Jun 26, 2019

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?

@kanaka
Copy link

kanaka commented Aug 16, 2019

@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> )"

@lahmatiy
Copy link
Contributor

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, w3c/csswg-drafts#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
Copy link
Contributor

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).

kanaka added a commit to kanaka/html5-css3-ebnf that referenced this issue Aug 29, 2019
The VDS syntax allows a non-context free mode of automatic comma
removal (requires at least some sort of lookahead/backtracking). So
workaround the main two grammars where we encounter this: rgb/rgba. We
group the comma and the following alpha value and make the whole group
optional.

Add to ticket mdn/data#341. However, looks
like the workaround is unlikely to be incorporated since the
contextual comma removal appears desired for ease of specifying those
cases.
@github-actions github-actions bot added the idle Issues and pull requests with no activity for three months. label Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idle Issues and pull requests with no activity for three months.
Projects
None yet
Development

No branches or pull requests

3 participants