Closed Bug 489871 Opened 11 years ago Closed 9 years ago

CSS comments should not be allowed in presentation attribute values

Categories

(Core :: SVG, defect, minor)

defect
Not set
minor

Tracking

()

RESOLVED FIXED

People

(Reporter: jwatt, Assigned: longsonr)

Details

(Keywords: testcase)

Attachments

(2 files, 1 obsolete file)

CSS comments should not be allowed in presentation attribute values. Currently we allow that.
Attached image testcase
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → longsonr
Attachment #487243 - Flags: review?(dbaron)
Comment on attachment 487243 [details] [diff] [review]
patch

This is the wrong place for the check; it will cause characters to be consumed and then EOF to be reported.  Callers in the parser can (and probably do) assume that once EOF has been reported once it will continue to be reported.  This would violate that assumption.

Instead, you should move the IsSVGMode() check to be along with the if (ch == '/') condition (probably after, for speed, i.e., if (ch == '/' && !IsSVGMode())).
Attachment #487243 - Flags: review?(dbaron) → review-
Attachment #487243 - Attachment is obsolete: true
Attachment #495376 - Flags: review?(dbaron)
Comment on attachment 495376 [details] [diff] [review]
address review comment

r=dbaron
Attachment #495376 - Flags: review?(dbaron) → review+
Comment on attachment 495376 [details] [diff] [review]
address review comment

One line change (if you exclude the ifdef removal) with reftest. Improves compliance.
Attachment #495376 - Flags: approval2.0?
Whiteboard: [need approval]
Pushed http://hg.mozilla.org/mozilla-central/rev/d239dedd96d3
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [need approval]
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.