553805 - :not() is not a valid CSS 3 selector

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Gecko fails on one test of the recent IE9 TestCenter. That test checks if a
rule containing an empty negated selector is considered as an error or not.
Since the negation pseudo-class takes a simple selector as argument and since
the rules for omitting a universal selector are not allowing that case, it's
pretty clear that :not() is invalid. It also pretty useless since it would mean
:not(*)...

Comment 1

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: proposed trivial fix

Comment 2

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

Comment on attachment 433731 [details] [diff] [review]
proposed trivial fix

Looks fine, except that it clearly doesn't compile due to a case error (isSymbol rather than IsSymbol) and a missing close parenthesis.

Also, it's quite trivial to add an automated test for this:  in layout/style/test/test_selectors.html you can add a call to test_balanced_unparseable() near the end of run().


Also, it's better to post diffs with more context.  I'd recommend using an ~/.hgrc like this one:

[ui]
username = Your Name <your@email>

[defaults]
qnew = -U

[diff]
# Use git patch format, which can store diffs of binary files, and
# renames.  Essential when using mq.
git = 1
# needed starting in mercurial 1.0:
showfunc = 1
# allowed starting in mercurial 1.0.1:
unified = 8



Thanks for the patch.  If you want me to do the revisions, let me know.

Comment 3

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

(In reply to comment #2)
> (From update of attachment 433731 [details] [diff] [review])
> Looks fine, except that it clearly doesn't compile due to a case error
> (isSymbol rather than IsSymbol) and a missing close parenthesis.

D'oh... I submitted the wrong version :-( Sorry for that :-(

> Thanks for the patch.  If you want me to do the revisions, let me know.

Sure go ahead and let's show a 100% on these tests asa^p.

Comment 4

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

Updated patch is:

http://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/9a6abdccd518/forbid-empty-not

Comment 5

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

That, and the revised version at http://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/c6137e534a8b/forbid-empty-not with some additional tests, both passed try server, so this is ready to land.

Comment 6

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

http://hg.mozilla.org/mozilla-central/rev/4c99e481192d