Last Comment Bug 553805 - :not() is not a valid CSS 3 selector
: :not() is not a valid CSS 3 selector
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: P3 normal (vote)
: mozilla1.9.3a4
Assigned To: Daniel Glazman (:glazou)
:
:
Mentors:
: 564788 (view as bug list)
Depends on:
Blocks: ietestcenter
  Show dependency treegraph
 
Reported: 2010-03-20 08:16 PDT by Daniel Glazman (:glazou)
Modified: 2010-06-05 01:00 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
proposed trivial fix (535 bytes, patch)
2010-03-20 08:22 PDT, Daniel Glazman (:glazou)
dbaron: review-
Details | Diff | Splinter Review

Description Daniel Glazman (:glazou) 2010-03-20 08:16:23 PDT
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) 2010-03-20 08:22:29 PDT
Created attachment 433731 [details] [diff] [review]
proposed trivial fix
Comment 2 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2010-03-20 17:22:26 PDT
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) 2010-03-20 21:09:47 PDT
(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-7 (busy September 14-25) 2010-03-21 12:24:22 PDT
Updated patch is:

http://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/9a6abdccd518/forbid-empty-not
Comment 5 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2010-03-21 17:41:57 PDT
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-7 (busy September 14-25) 2010-03-22 07:57:20 PDT
http://hg.mozilla.org/mozilla-central/rev/4c99e481192d
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2010-05-10 06:29:42 PDT
*** Bug 564788 has been marked as a duplicate of this bug. ***

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