-
Notifications
You must be signed in to change notification settings - Fork 718
[css-color-hdr-1] use one-or-more for dynamic-range-limit-mix
grammar production
#11349
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
[css-color-hdr-1] use one-or-more for dynamic-range-limit-mix
grammar production
#11349
Conversation
Sorry, this was my mistake. I intended for commas to be required.
My inclination would be to allow single-argument issues in |
Uploaded a patch with WPT tests at: |
And uploaded a PR to update Is that the extent of the changes that are desired here? |
I think so. Is there any reason this PR won't suffice for the change? I can't see a difference between this one and #11351 |
Add tests for the edge cases brought up in: w3c/csswg-drafts#11349 Change-Id: Ia387c339d8555068cb70899bff850f4005423f82
Add tests for the edge cases brought up in: w3c/csswg-drafts#11349 Change-Id: Ia387c339d8555068cb70899bff850f4005423f82 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6087798 Reviewed-by: Keith Cirkel <chromium@keithcirkel.co.uk> Commit-Queue: ccameron chromium <ccameron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1403188}
Add tests for the edge cases brought up in: w3c/csswg-drafts#11349 Change-Id: Ia387c339d8555068cb70899bff850f4005423f82 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6087798 Reviewed-by: Keith Cirkel <chromium@keithcirkel.co.uk> Commit-Queue: ccameron chromium <ccameron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1403188}
… a=testonly Automatic update from web-platform-tests dynamic-range-limit: Add more WPT tests Add tests for the edge cases brought up in: w3c/csswg-drafts#11349 Change-Id: Ia387c339d8555068cb70899bff850f4005423f82 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6087798 Reviewed-by: Keith Cirkel <chromium@keithcirkel.co.uk> Commit-Queue: ccameron chromium <ccameron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1403188} -- wpt-commits: d2d172058342f786290e219b18288f40dc773ba9 wpt-pr: 49639
… a=testonly Automatic update from web-platform-tests dynamic-range-limit: Add more WPT tests Add tests for the edge cases brought up in: w3c/csswg-drafts#11349 Change-Id: Ia387c339d8555068cb70899bff850f4005423f82 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6087798 Reviewed-by: Keith Cirkel <chromium@keithcirkel.co.uk> Commit-Queue: ccameron chromium <ccameron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1403188} -- wpt-commits: d2d172058342f786290e219b18288f40dc773ba9 wpt-pr: 49639
Add tests for the edge cases brought up in: w3c/csswg-drafts#11349 Change-Id: Ia387c339d8555068cb70899bff850f4005423f82 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6087798 Reviewed-by: Keith Cirkel <chromium@keithcirkel.co.uk> Commit-Queue: ccameron chromium <ccameron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1403188}
… a=testonly Automatic update from web-platform-tests dynamic-range-limit: Add more WPT tests Add tests for the edge cases brought up in: w3c/csswg-drafts#11349 Change-Id: Ia387c339d8555068cb70899bff850f4005423f82 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6087798 Reviewed-by: Keith Cirkel <chromiumkeithcirkel.co.uk> Commit-Queue: ccameron chromium <ccameronchromium.org> Cr-Commit-Position: refs/heads/main{#1403188} -- wpt-commits: d2d172058342f786290e219b18288f40dc773ba9 wpt-pr: 49639 UltraBlame original commit: 7c7d7cd0908d881fd0dd873abe1e28d0a3c4f087
… a=testonly Automatic update from web-platform-tests dynamic-range-limit: Add more WPT tests Add tests for the edge cases brought up in: w3c/csswg-drafts#11349 Change-Id: Ia387c339d8555068cb70899bff850f4005423f82 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6087798 Reviewed-by: Keith Cirkel <chromiumkeithcirkel.co.uk> Commit-Queue: ccameron chromium <ccameronchromium.org> Cr-Commit-Position: refs/heads/main{#1403188} -- wpt-commits: d2d172058342f786290e219b18288f40dc773ba9 wpt-pr: 49639 UltraBlame original commit: 7c7d7cd0908d881fd0dd873abe1e28d0a3c4f087
… a=testonly Automatic update from web-platform-tests dynamic-range-limit: Add more WPT tests Add tests for the edge cases brought up in: w3c/csswg-drafts#11349 Change-Id: Ia387c339d8555068cb70899bff850f4005423f82 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6087798 Reviewed-by: Keith Cirkel <chromiumkeithcirkel.co.uk> Commit-Queue: ccameron chromium <ccameronchromium.org> Cr-Commit-Position: refs/heads/main{#1403188} -- wpt-commits: d2d172058342f786290e219b18288f40dc773ba9 wpt-pr: 49639 UltraBlame original commit: 7c7d7cd0908d881fd0dd873abe1e28d0a3c4f087
Based on the parsing tests in WPT (https://github.com/web-platform-tests/wpt/blob/master/css/css-color-hdr/parsing.html) it seems as though commas are required for each set of ident && groups here, however the spec is using the
+
sigil which allows for "one or more" but doesn't specify commas. I think what might be desired here is to use#
which is "one or more comma separated". This would satisfy the requirement for commas.I think, in addition, we might want to specify something like
#{2,}
- considering all examples in the parse tests expect two or more values, and the spec talks about summing all values, I am unsure if it is redundant or not to specify just one value (e.g.dynamic-range-limit-mix(high 10%)
).This doesn't resolve the recursive affordances that the WPTs allude to, but baby steps 😆