Re: [css3-syntax] comments on parsing states

Thanks for the review!

On Wed, Aug 29, 2012 at 12:21 AM, L. David Baron <dbaron@dbaron.org> wrote:
> I did a quick read through of all the parsing states in
> http://dev.w3.org/csswg/css3-syntax/#tree-construction .  Comments
> follow:
>
> 3.6.3 At-rule mode
>   { token
>     Otherwise, this is a parse error:
>       The sentences "Switch to the next-block error mode. Reconsume
>       the current input token." should probably be in the other
>       order.

I use that particular statement order (switch, followed by reconsume)
throughout the spec.  It's just a quirk copied from the HTML parser
algorithm.

> But it would be simpler to just say "Consume a
> primitive and ignore the return value" as you do in "Top-level
> mode.

Done.

>   additional case needed:
>     You need a case for the } token.  To be compatible with CSS 2.1
>     it should only apply when the at-rule is inside another rule
>     (which I think means when the stack of open rules has greater
>     than two items).  (However, I think it would be a reasonable
>     change to make it apply always... but that probably doesn't work
>     for the other cases where you need this same change.)  This case
>     should say:
>       Discard the current rule.  Reconsume the current input token.

Changed.  I've also added the same recovery to the selector mode.
Testing shows that the two situations switch into different error
modes.

> 3.6.4 Rule mode
>   { needs to be a parse error just like in 3.6.2 Top-level mode.

Done.

> 3.6.5 Selector mode
>   needs a case for } just as I propose above for 3.6.3.

Done.

>   EOF Token
>     Is discarding the current rule correct?  It seems inconistent
>     with CSS 2.1's EOF rules, which should suggest leaving the rule,
>     or perhaps only leaving the rule if it has a nonempty selector?

It's consistent with FF and WebKit's behavior, at least.  A style rule
isn't preserved until you hit the opening {.  Test case:

<!DOCTYPE html>
<body>
<style>
foo { color: red; }
bar
</style>
<script>
function print(rule) {
  var pre = document.createElement('pre');
  pre.textContent = rule.cssText;
  document.body.appendChild(pre);
}
[].forEach.call(document.styleSheets[0].cssRules, print);
</script>

> 3.6.8 Declaration-value mode
>
>   DELIM with value of !
>     I think you should proceed into a new mode in which you expect
>     important.  (If it's there, go to declaration-end mode.  For
>     EOF, probably discard the current declaration.  And for anything
>     else, reprocess the current token and go into next-declaration
>     error mode.)
>
>     Then again, this does forbid ! within the toplevel of a
>     variable, as fantasai points out.
>
>     You do need to allow whitespace and comments here somehow.
>
>     Also, there are contexts (in particular, keyframe rules) in
>     which !important is a parse error and causes the declaration to
>     be discarded.  You should allow for this.

I'm fine with making these changes.  Done.

>   anything else
>     As I said before [1], I think these quirks should be described
>     as part of the property grammar.

I'm not sure I agree, but I'll pick it up in that thread.

> 3.6.10 Next block error mode
>   I think you can remove this given how flexible your at-rule
>   parsing is (see comments on 3.6.3).  But if you don't, you need a
>   case for } like for 3.6.5 and 3.6.3.

I do still need it - turns out that a stray } in a top-level selector
triggers it!

However, testing shows that I do *not* need a } clause here.  WebKit
and FF just swallow every } until they find a full block, even when
nested inside of another block.

> 3.6.18 Pop the current rule
>   The validity of the rule can depend on whether the rule was
>   terminated by an EOF.  This means it can't be tested in the way
>   you describe, since by this point you've lost that information.  I
>   think this is fixable by having a separate state for "pop the
>   current rule for an end of file".  (For example, media queries
>   have similar "closing open constructs" rules to the ones rules do,
>   and the validity of the @media query determines the validity of an
>   @import rule.)

I don't understand how validity can depend on EOF-ness.  I don't see
any such effects from basic testing - as far as I can tell, EOF just
acts as if everything currently open was properly closed.  Can you
give an example?

>   It's probably also worth explicitly saying that "according to the
>   appropriate grammar rules" means that errors that were handled by
>   some smaller piece being discarded don't count.

That *should* be implicit by the fact that anything thrown away
earlier won't be part of the rule being validated.  If you dont' think
it's enough, could you suggest some text that would address your
concern?  I'm not really sure what to say.

>   Finally, whether the current rule is valid also depends on
>   context.  Some at-rules are only valid at top-level and not within
>   other at rules.  Some (actually, currently, all) of these are only
>   valid if the prior rules that were appended to the new current
>   rule (i.e., the style sheet) were in certain categories (i.e.,
>   @charset, @import, @namespace).

I've slightly amended the text to also make validity depend on context.

> In general, "Reprocess the current input token" and "Reconsume the
> current input token" aren't defined.  Presumably they cause the next
> occurrence of "Consume the next input token" to do nothing.  You
> should probably also stick to one wording.  But you also often use
> them after the "switch to the ... state" wording, and I tend to
> think it should be before.

Ugh, I should only be using one of those two phrasings, not both. :/
Time for a search/replace.

I've now provided a definition for the phrase.

I just took the ordering of the statements from the HTML parser.

> You need to define that "discard the current rule" removes the rule
> from the stack of open rules.

Done.

> You need to define when the "current declaration" is set and unset.
> It's not defined at all.

Done.

~TJ

Received on Friday, 7 September 2012 01:03:25 UTC