Error recognizing the string "none" as a value for counter-increment and counter-reset property

RESOLVED FIXED in mozilla1.9alpha5

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla1.9alpha5
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 years ago
When counter-increment is set manually to "none" and then read, it comes back as "none 1".  Similarly, when counter-reset is set to "none" and then read, it comes back as "none 0"

Was marked as a "todo" in the mochitest file layout/style/test/ test_value_computation.html.  Looking into this as my first bugfix -- dbaron says it's likely a parsing bug in nsCSSParser.cpp
(Assignee)

Comment 1

11 years ago
Created attachment 263623 [details] [diff] [review]
Proposed patch. (change LowerCaseEqualsLiteral to LowerCaseEqualsASCII)

This is my proposed fix.  After doing a GDB trace, I've found that the problem occurs because the compiler incorrectly tells LowerCaseEqualsLiteral that sv->str is a string-literal of length 13, even when it's shorter, because sv->str is a statically declared char-array with a length of 13.

(and i.e. for comparing "none" to "none", LowerCaseEqualsLiteral compares all 13 characters, going past the null terminus., and returns false when it should return true)
Attachment #263623 - Flags: review?(dbaron)
This looks good; it just also needs the diff for the mochitest to remove the expected failures -- and potentially equivalent changes for some other tests.
(Assignee)

Comment 3

11 years ago
Created attachment 263675 [details] [diff] [review]
Proposed patch #2 (removed todo's in mochi tests)

Same as previous patch, plus I removed TODO's in 2 mochitest files, for tests that this patch fixes.
Attachment #263623 - Attachment is obsolete: true
Attachment #263675 - Flags: review?(dbaron)
Attachment #263623 - Flags: review?(dbaron)
Attachment #263675 - Flags: superreview+
Attachment #263675 - Flags: review?(dbaron)
Attachment #263675 - Flags: review+
(Assignee)

Updated

11 years ago
Assignee: nobody → dholbert
(Assignee)

Comment 4

11 years ago
Fix checked in to trunk by dbaron on my behalf.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Flags: in-testsuite+
OS: Linux → All
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha5
You need to log in before you can comment on or make changes to this bug.