w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.51k stars 671 forks source link

[css-shadow-parts] fully specify the parser for the forwarding micro-syntax #2412

Open fergald opened 6 years ago

fergald commented 6 years ago

As discussed at Web Components F2F (2018-03, Tokyo) and in TAG issue update spec to include a fully specified parser.

rniwa commented 6 years ago

Isn't this a duplicate of https://github.com/w3c/csswg-drafts/issues/2411 ?

fergald commented 6 years ago

Not exactly. This one is for me to add a section to the spec with a grammar, it's just not worth doing until we have clearer agreement on #2411

fergald commented 6 years ago

13add368a8d60f7b1bc152e23e3695a43596fc11 adds a fully specified parser. I'd appreciate people sanity checking it. A couple of points:

@tabatkins

annevk commented 6 years ago

Yes, you want to define this in terms of https://infra.spec.whatwg.org/ (Infra, not HTML). Since you're using Bikeshed you need to use <a> to get them to link, not <span>.

Strict is good, but it seems you're not totally strict? Validity requires no whitespace, but you are stripping whitespace.

You need to be clearer about what "space characters" means. I suspect you want to reference the ASCII whitespace definition.

fergald commented 6 years ago

I think strict for everything except inter-token whitespace is what I wanted and matches most (all?) of the parsers in the HTML spec. I will update the links. Thanks.

annevk commented 6 years ago

Yeah, but then you should incorporate that in the definition of what's valid and allow ASCII whitespace there.

annevk commented 6 years ago

By the way, does this attribute only apply to HTML elements or to all elements? I guess only to HTML elements since shadow trees are restricted to them?

annevk commented 6 years ago

Sorry for all the short thoughts, one other thing I think we want here is to eventually move this attribute definition into the HTML Standard.

fergald commented 6 years ago

@annevk

Yeah, but then you should incorporate that in the definition of what's valid and allow ASCII whitespace there.

Makes total sense to me but e.g. look at Lists of floating-point numbers where the "valid" definition eplicitly rules out whitespace and the parser copes with it. I assumed that the "valid" one was actually specifying the canonical/serialized verison rather than the only valid format (since other examples in the spec are similarly divergent). I'm happy to go either way, any advice appreciated.

annevk commented 6 years ago

Valid is what is conforming for developers to write and what a validator would complain about. It has no effect on serialization (or parsing). I think HTML varies in where whitespace is allowed and examples in the HTML standard vary as to whether they are valid or not (there's some invalid examples to make a point). Here it probably makes sense to allow whitespace given the examples written thus far.

fergald commented 6 years ago

Yeah, I'll change this spec to clearly allow whitespace.

But just on the topic of the HTML, I'm not talking about examples, the spec itself is contradictory from one paragraph to the next E.g.

A valid list of floating-point numbers is a number of valid floating-point numbers separated by U+002C COMMA characters, with no other characters (e.g. no ASCII whitespace). In addition, there might be restrictions on the number of floating-point numbers that can be given, or on the range of values allowed.

but then

  1. Collect a sequence of code points that are ASCII whitespace, U+002C COMMA, or U+003B SEMICOLON characters from input given position. This skips past any leading delimiters. and

5.6 Collect a sequence of code points that are ASCII whitespace, U+002C COMMA, or U+003B SEMICOLON characters from input given position. This skips past the delimiter.

So the parser for list of floats does not match the definition of "valid". Maybe I just picked a bad example to base my one on.

tabatkins commented 6 years ago

Yeah, what you call "valid" is just "what should a validator check to see if it should complain about this?". It doesn't necessarily have any connection to what's actually accepted; there's often a lot of constraints there that make the set of accepted things much wider than the set of valid things.

In this case, I think it's perfectly reasonable and good to have whitespace in your part mapping; I would be annoyed if a validator complained at me for it. So it should be included in the definition of validity.

(Reviewing the spec now.)

tabatkins commented 6 years ago

K, and reviewed. Lots of comments on the commit itself.

fergald commented 6 years ago

@tabatkins Thanks. a0ec26cefb7b4bc0ee8b0923eb935ffcf3954d87 addresses lots of your comments.

Still to do

fergald commented 6 years ago

One other commit that I didn't tag is

b7893dce452947d01c3ec1f334fca749014abb60 [css-shadow-parts-1] fix auto-select for list refs]

PTAL