Closed
Bug 1265001
Opened 8 years ago
Closed 8 years ago
nth- parser fails with uppercase N followed by negative number
Categories
(Core :: CSS Parsing and Computation, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: algorithmicimperative, Assigned: MatsPalmgren_bugz)
Details
(Keywords: testcase)
Attachments
(2 files)
2.38 KB,
patch
|
heycam
:
review+
|
Details | Diff | Splinter Review |
2.38 KB,
patch
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0 Build ID: 20160309193552 Steps to reproduce: In either CSS or JS (querySelector), use an `:nth-child` selector that includes a capital `N` followed by a negative number. This does not happen with lowercase `n-1` or with a positive number `N+1`. DEMO: https://jsfiddle.net/ajd6Lo5c/1/ CSS: ``` p:nth-child(N-1) { color: red } ``` JS: ``` document.querySelector(":nth-child(N-1)") ``` Actual results: In CSS, the selector fails so the rule is ignored, and the color is not set to `red`. In JS, `querySelector` throws a DOM Exception. Expected results: In CSS, the selector ought to succeed and style the appropriate elements In JS, the querySelector ought to fetch appropriate elements.
Reporter | ||
Updated•8 years ago
|
Component: Untriaged → DOM
Product: Firefox → Core
Assignee | ||
Comment 1•8 years ago
|
||
Right, the spec is clear that the 'n' should be case-insensitive: https://drafts.csswg.org/css-syntax/#the-anb-type
Assignee: nobody → mats
Severity: normal → minor
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: testcase
Priority: -- → P3
Assignee | ||
Comment 2•8 years ago
|
||
Attachment #8742144 -
Flags: review?(cam)
Assignee | ||
Comment 3•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=04c58665c32a
Comment 4•8 years ago
|
||
Comment on attachment 8742144 [details] [diff] [review] fix Review of attachment 8742144 [details] [diff] [review]: ----------------------------------------------------------------- Please add a test that uses an uppercase :nth-child(...N...). ::: layout/style/nsCSSParser.cpp @@ +6279,5 @@ > + const nsASCIICaseInsensitiveStringComparator ciComparator; > + auto TokenBeginsWith = [&] (const nsLiteralString& aStr) { > + return StringBeginsWith(mToken.mIdent, aStr, ciComparator); > + }; > + I don't think it's necessary to factor out the creation of the comparator here. It is cheap to construct nsASCIICaseInsensitiveStringComparator() to pass in to StringBeginsWith directly at the call site. (It would be nice if we had a LowerCaseBeginsWithLiteral, so that we could avoid lowercasing the string we pass in...)
Attachment #8742144 -
Flags: review?(cam) → review+
Comment 5•8 years ago
|
||
(In reply to Cameron McCormack (:heycam) from comment #4) > Please add a test that uses an uppercase :nth-child(...N...). I didn't see the other attachment, but it looks OK. Would be nice to have a reftest that just verifies we correctly match :nth-child(...N...), too.
Comment on attachment 8742144 [details] [diff] [review] fix >+ auto TokenBeginsWith = [&] (const nsLiteralString& aStr) { >+ return StringBeginsWith(mToken.mIdent, aStr, ciComparator); >+ }; This violates the coding style guideline at: https://developer.mozilla.org/en-US/docs/Mozilla/C++_Portability_Guide#Use_C_lambdas_but_with_care which says: > We recommend explicitly listing out the variables that you capture in the lambda, both for documentation purposes, and to double-check that you're only capturing what you expect to capture. Please explicitly list the captured variables (but still capture by reference!).
(I think that would be "[&mToken]".)
Assignee | ||
Comment 8•8 years ago
|
||
> (I think that would be "[&mToken]".)
That fails to compile unfortunately.
What we implicitly capture here is 'this'.
I used [this] instead to make it explicit.
Assignee | ||
Updated•8 years ago
|
Flags: in-testsuite+
https://hg.mozilla.org/integration/mozilla-inbound/rev/1157791f8ec1 https://hg.mozilla.org/integration/mozilla-inbound/rev/4f139d664ccc
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1157791f8ec1 https://hg.mozilla.org/mozilla-central/rev/4f139d664ccc
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•