Last Comment Bug 780337 - @supports rule should not parse if property value is empty
: @supports rule should not parse if property value is empty
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Cameron McCormack (:heycam)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-03 19:46 PDT by Cameron McCormack (:heycam)
Modified: 2012-08-04 11:16 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (2.76 KB, patch)
2012-08-03 20:41 PDT, Cameron McCormack (:heycam)
dbaron: review+
Details | Diff | Review

Description Cameron McCormack (:heycam) 2012-08-03 19:46:48 PDT
In

  @supports (a:) { ... }

the "a:" doesn't match declaration, so we should drop the whole at-rule.
Comment 1 Cameron McCormack (:heycam) 2012-08-03 20:41:55 PDT
Created attachment 648941 [details] [diff] [review]
patch
Comment 2 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2012-08-03 20:52:39 PDT
Comment on attachment 648941 [details] [diff] [review]
patch

I think you should:

 (1) split the test rather than change it, and make one fork of it test what it was testing, but with "unknown:x" instead of "unknown:", and have your current fork of it.

 (2) move your new code above the 'if (propID == eCSSProperty_UNKNOWN)' test (and probably above the LookupProperty call too)

 (3) add another test for the same thing with a known property

Or is there a reason the same bug wouldn't have been present with a known property?

r=dbaron with that
Comment 3 Cameron McCormack (:heycam) 2012-08-03 21:38:28 PDT
(In reply to David Baron [:dbaron] from comment #2)
>  (1) split the test rather than change it, and make one fork of it test what
> it was testing, but with "unknown:x" instead of "unknown:", and have your
> current fork of it.

I think http://hg.mozilla.org/mozilla-central/file/a7fadfbad932/layout/reftests/w3c-css/submitted/conditional3/css-supports-021.xht already covers that case.

>  (2) move your new code above the 'if (propID == eCSSProperty_UNKNOWN)' test
> (and probably above the LookupProperty call too)

OK.

>  (3) add another test for the same thing with a known property

OK.

> Or is there a reason the same bug wouldn't have been present with a known
> property?

No, you're right the test needs to go earlier.
Comment 4 Cameron McCormack (:heycam) 2012-08-04 00:08:10 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/c9564834bfe1

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