Beginning on October 25th, 2016, Persona will no longer be an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 816045 - Should pass @supports W3C test
: Should pass @supports W3C test
Status: NEW
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal with 1 vote (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
: Jet Villegas (:jet)
Depends on: 816431 816720
  Show dependency treegraph
Reported: 2012-11-28 04:35 PST by Simon Pieters
Modified: 2012-12-05 06:22 PST (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Description Simon Pieters 2012-11-28 04:35:44 PST has @supports tests from Opera. I just submitted the js/ folder. Nightly passes the reftests but fails 9 of 11 subtests in
Comment 1 Simon Pieters 2012-11-28 04:48:45 PST
I also noticed that has a few fails in Opera and Firefox (apart from the test expecting the quotes to be absent, I think CSSOM requires the quotes). I haven't investigated which tests are correct per spec.
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2012-11-28 09:43:41 PST
Does the spec actually define whitespace behavior?  A few of the test fails are about the exact indentation of the serialization of nested @-rules (we seem to only indent the first line; the test expects the first and last lines to be indented, a sane indentation would indent all lines).  By the way, the harness output is NOT useful for figuring this out, since it doesn't use preformatted whitespace...

For the other failures, they are in order:

1)  CSSRule.KEYFRAMES_RULE is undefined; we have CSSRule.MOZ_KEYFRAMES_RULE instead.  We
    should probably fix that, but it has nothing to do with @supports.
2)  insertRule doesn't allow nesting @-rules.  We should change that, since the parser
    does allow it.
3)  Removing a rule we never inserted obviously doesn't work.
4)  The test that expects a TypeError while calling insertRule on a non-group rule is
    clearly just broken.
5)  We don't put extra parens around the supports condition for the and/or cases (last
    two tests).
Comment 3 Cameron McCormack (:heycam) 2012-11-28 16:41:14 PST
For #4, isn't TypeError the right exception to throw, because the style rule is not a CSSGroupingRule, and thus doesn't have an insertRule method?

For #5, I don't think there is anything in the spec that says what the serialization should be.  AFAICT it's still an open issue on the spec.  Our implementation returns the exact string that was used when parsing the rule, trimmed of white space on either end.
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2012-11-28 19:51:24 PST
> because the style rule is not a CSSGroupingRule, and thus doesn't have an insertRule
> method?

Oh, trying to call undefined will throw a TypeError?  Yeah, that works.  Due to #2+#3 the test is currently running insertRule on a rule that actually is a group rule....
Comment 5 Simon Pieters 2012-11-30 07:17:34 PST
I changed a reftest because it looks like the spec had changed:

If the js test is wrong or should be relaxed to allow different serializations, that can be fixed in the test. I don't have time to look into that right now, but if someone else wants to fix it, feel free. :-)
Comment 6 Cameron McCormack (:heycam) 2012-12-03 13:53:28 PST
I think your change to the test there is wrong: "not(" should parse correctly now but be interpreted as a "general_enclosed" that evaluates to false.
Comment 7 Simon Pieters 2012-12-04 02:46:00 PST
Doesn't it match supports_condition_in_parens?
Comment 8 Cameron McCormack (:heycam) 2012-12-04 03:19:37 PST
The token stream for "(not(foo: baz))" is:

  '(' FUNCTION IDENT ':' S IDENT ')' ')'

which I think parses like this:

    => supports_condition_in_parens
    => '(' S* supports_condition ')' S*
    => '(' supports_condition ')' S*
    => '(' supports_condition_in_parens ')' S*
    => '(' general_enclosed ')' S*
    => '(' ( FUNCTION | '(' ) (any|unused)* ')' S* ')' S*
    => '(' FUNCTION (any|unused)* ')' S* ')' S*
    => '(' FUNCTION IDENT ':' S IDENT ')' S* ')' S*
    => '(' FUNCTION IDENT ':' S IDENT ')' ')' S*
    => '(' FUNCTION IDENT ':' S IDENT ')' ')'
Comment 9 Simon Pieters 2012-12-04 04:09:20 PST
Hmm. I'm not sure I follow why you get to 
    => '(' supports_condition_in_parens ')' S*
rather than
    => '(' supports_negation ')' S*
but if that's the case, is that a spec bug or deliberate?
Comment 10 Cameron McCormack (:heycam) 2012-12-04 20:02:44 PST
Because the input "not(" is tokenised as FUNCTION, not as the IDENT "not" followed by "(".  There needs to be a space after the "not" for it to be tokenised as an ident.  (This is despite the fact the grammar has S* after NOT in supports_negation -- the tokeniser has already matched "not(" into FUNCTION.)

I think this is deliberate.

Note You need to log in before you can comment on or make changes to this bug.