If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

nsCSSScanner::Backup doesn't handle escapes correctly




CSS Parsing and Computation
4 years ago
a year ago


(Reporter: dbaron, Unassigned)




Firefox Tracking Flags

(Not tracked)



(4 attachments)

As I pointed out in bug 543151 comment 81 (which I noticed while reviewing bug 750388), nsCSSScanner::Backup behaves incorrectly when it's being used where escapes are involved.  I don't see an obvious fix here.

I haven't yet managed to find a testcase where we incorrectly accept something that we should reject, but it wouldn't surprise me if such a testcase exists.
Created attachment 754678 [details]
testcase (post-bug 876570)
Attachment #754678 - Attachment description: testcase (post-bug 750388) → testcase (post-bug 876570)
Created attachment 754683 [details]
testcase (independent of recent/known future changes)
Created attachment 754835 [details]
another testcase

Here's another testcase: Firefox 21 displays alternating green and blue rows, trunk displays all green.  This may be functionally equivalent to attachment 754683 [details] but I'm not sure.

We can fix most of this pretty easily by abandoning the notion of backing up, instead parsing 'b' directly out of the dimension suffix when necessary.  However, when the value of 'b' consists entirely of escaped digits (as in this testcase) I'm not sure whether it's supposed to be accepted.
Assignee: nobody → zackw
Correction: Firefox 20 is the one that displays alternating green and blue rows on attachment 754835 [details].
Created attachment 754861 [details] [diff] [review]
WIP patch

Here's a patch which attempts to handle an+b without backing up at all.  It's not quite right yet: in particular, :nth-child(n-/**/2) is rejected, contra test_selectors.html's expectations.  Also, ExtractInteger is probably reinventing the wheel.  Still, this should be good enough for am-I-barking-up-the-wrong-tree-entirely reactions.
Attachment #754861 - Flags: feedback?(dbaron)
We might want to wait on this until the current discussion on www-style about an+b syntax finishes up...
(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #6)
> We might want to wait on this until the current discussion on www-style
> about an+b syntax finishes up...

OK, I sent a note to www-style so they know about this wrinkle as well as the other stuff being discussed under that head.


4 years ago
Attachment #754861 - Flags: feedback?(dbaron)

Comment 8

a year ago
Resetting owner to default per Zack's request.
Assignee: zackw → nobody
You need to log in before you can comment on or make changes to this bug.