Closed
Bug 383075
Opened 18 years ago
Closed 18 years ago
"red;" (with a semicolon) should not be accepted as a color
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha6
People
(Reporter: jruderman, Assigned: MatsPalmgren_bugz)
References
Details
(Keywords: testcase)
Attachments
(5 files)
666 bytes,
text/html
|
Details | |
1.44 KB,
text/html
|
Details | |
1.58 KB,
patch
|
dbaron
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
2.54 KB,
patch
|
Details | Diff | Splinter Review | |
4.59 KB,
patch
|
Details | Diff | Splinter Review |
Setting elem.style.background to "red;" or even "red; yellow" turns the element red. I think it should have no effect, like in WebKit trunk and Opera, and cause an error message to appear in the error console.
Assignee | ||
Comment 1•18 years ago
|
||
It affects all properties.
Assignee: nobody → mats.palmgren
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•18 years ago
|
||
Attachment #267138 -
Flags: superreview?(dbaron)
Attachment #267138 -
Flags: review?(dbaron)
Comment on attachment 267138 [details] [diff] [review]
Patch rev. 1
>+ if (parsedOK) {
>+ // Junk at end of property value.
>+ REPORT_UNEXPECTED_TOKEN(PEExpectEndProperty);
>+ errorCode = NS_ERROR_FAILURE;
>+ }
This should come at the very beginning of the else to be consistent with other types of error reporting, I think.
With that, and if you add a mochitest for the bug, r+sr=dbaron.
Attachment #267138 -
Flags: superreview?(dbaron)
Attachment #267138 -
Flags: superreview+
Attachment #267138 -
Flags: review?(dbaron)
Attachment #267138 -
Flags: review+
Reporter | ||
Comment 4•18 years ago
|
||
Should the mochitest for this bug be in the form of modifying the existing comprehensive tests (/layout/style/test/) to try every valid property value with a semicolon at the end, and make sure it's not accepted? Or would that be overkill?
Comment 5•18 years ago
|
||
Please remove the XXXbz comment before the function in question, since you're fixing the thing it complains about?
And doesn't this change make such sets throw NS_ERROR_FAILURE? Is that really what we want? I'd think not...
Assignee | ||
Updated•18 years ago
|
Flags: in-testsuite?
OS: Mac OS X → All
Hardware: PC → All
Assignee | ||
Comment 6•18 years ago
|
||
1. remove the XXX comment
2. move the error message as dbaron suggested
3. leave errorCode as is, which should be NS_OK in this case
Assignee | ||
Comment 7•18 years ago
|
||
Assignee | ||
Comment 8•18 years ago
|
||
The mochitest tests both style.prop=val and style.setProperty(..., null)
but it doesn't test style.setProperty(..., "important") because that takes
a different path through the code and we have bugs there too.
I'll file a separate bug on that.
Assignee | ||
Comment 9•18 years ago
|
||
Checked in to trunk at 2007-06-06 10:56 PDT.
-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha6
Assignee | ||
Comment 10•18 years ago
|
||
Filed bug 383488 on style.setProperty(..., "important")
Assignee | ||
Updated•17 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•