Closed Bug 553805 Opened 12 years ago Closed 12 years ago

:not() is not a valid CSS 3 selector

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla1.9.3a4

People

(Reporter: glazou, Assigned: glazou)

References

Details

Attachments

(1 file)

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(*)...
Attachment #433731 - Flags: review?(dbaron)
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.
Attachment #433731 - Flags: review?(dbaron) → review-
(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.
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.
Target Milestone: --- → mozilla1.9.3a4
Blocks: ietestcenter
http://hg.mozilla.org/mozilla-central/rev/4c99e481192d
Assignee: nobody → daniel
Status: NEW → RESOLVED
Closed: 12 years ago
Priority: -- → P3
Resolution: --- → FIXED
Duplicate of this bug: 564788
You need to log in before you can comment on or make changes to this bug.