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

Syntax simplification of CSS <source-size-list> #8961

Merged
merged 1 commit into from
Mar 14, 2023
Merged

Syntax simplification of CSS <source-size-list> #8961

merged 1 commit into from
Mar 14, 2023

Conversation

cdoublev
Copy link
Contributor

@cdoublev cdoublev commented Feb 28, 2023

[<source-size-list>#,]? <source-size-value> is nearly equivalent to <source-size-list>#? , <source-size-value>. If you think this is not a breaking change, I propose the second syntax definition.

The current syntax definition may predates comma-elision rules and #?.

Commas specified in the grammar are implicitly omissible in some circumstances [...]:

  • all items preceding the comma have been omitted
  • [...]

https://drafts.csswg.org/css-values-4/#comb-comma

similarly, the # and ? multipliers may be stacked as #?. These stacks each represent the later multiplier applied to the result of the earlier multiplier.

https://drafts.csswg.org/css-values-4/#component-multipliers

However it can make a difference with grammar-driven parsers, which could produce different data representations:

  • list(list(commaList(...sourceSizes), comma), sourceSizeValue) instead of list(commaList(...sourceSizes), comma, sourceSizeValue)
  • list(omittedGroup, sourceSizeValue) instead of list(omittedGroup, omittedComma, sourceSizeValue])

The second results (from the proposed syntax) are simpler and more consistent.


/images.html ( diff )

@zcorpan
Copy link
Member

zcorpan commented Mar 10, 2023

@tabatkins is this equivalent?

@zcorpan
Copy link
Member

zcorpan commented Mar 10, 2023

Note that parsing is defined here https://html.spec.whatwg.org/multipage/images.html#parsing-a-sizes-attribute

The grammar is effectively only for conformance checking.

@cdoublev
Copy link
Contributor Author

The grammar is effectively only for conformance checking.

Indeed. I created this issue after looking for occurrences of [... , ]? ... in CSS productions but I had forgotten about this algorithm.

I take the opportunity of Tab's review to suggest a simplification (in another PR if needed).

<source-size> = <media-condition>? <length [0,]>

When asked to parse a sizes attribute from an element, parse a comma-separated list according to the CSS grammar, with <source-size> as grammar, and the value of the element's sizes attribute (or the empty string, if the attribute is absent) as input, and let unparsed sizes list be the result.

  1. Remove all failure items from unparsed sizes list.
  2. If unparsed sizes list is empty, return 100vw.
  3. For each unparsed size representing <source-size> in unparsed sizes list
    • If <media-condition> is omitted or evaluates to true, return <length>. If this was not the last item in unparsed sizes list and <media-condition> is omitted, that is a parse error.

@tabatkins
Copy link
Collaborator

Yes, the grammar is equivalent. We don't concern ourselves with what internal representation particular parsers might generate for themselves; the two are identical in terms of accept/reject sets.

@zcorpan
Copy link
Member

zcorpan commented Mar 13, 2023

@tabatkins thanks!

@cdoublev I'd rather not change the parsing algorithm, it was carefully written to behave as intended.

Copy link
Member

@zcorpan zcorpan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but needs a rebase

@cdoublev
Copy link
Contributor Author

Oups, I forgot to update the head before making this PR, sorry. Done.

source Show resolved Hide resolved
@zcorpan zcorpan merged commit 86a37fe into whatwg:main Mar 14, 2023
@cdoublev cdoublev deleted the source-size-list branch March 15, 2023 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants