780337 - @supports rule should not parse if property value is empty

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Cameron McCormack (:heycam)]

In

  @supports (a:) { ... }

the "a:" doesn't match declaration, so we should drop the whole at-rule.

Comment 1

[Cameron McCormack (:heycam)]

Attachment: patch

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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)]

(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)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/c9564834bfe1

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c9564834bfe1
https://hg.mozilla.org/mozilla-central/rev/92e7a70cc292