CSS serialization doesn't escape characters that need escaping

RESOLVED FIXED

Status

()

--
minor
RESOLVED FIXED
10 years ago
2 years ago

People

(Reporter: jruderman, Unassigned)

Tracking

(Blocks: 1 bug, {sec-low, testcase})

Trunk
sec-low, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Updated

10 years ago
Blocks: 476744
Some of these are more than just escaping issues, like the IsPseudoElement function in nsCSSStyleRule.cpp, and perhaps bug 280443.
Depends on: 280443
Target Milestone: --- → mozilla13
Version: Trunk → 15 Branch
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: mozilla13 → ---
Version: 15 Branch → Trunk
(Reporter)

Comment 3

6 years ago
This can lead to security surprises for sites that sanitize user-generated CSS per spec and then use "elem.innerHTML += ...":

http://www.slideshare.net/x00mario/the-innerhtml-apocalypse
Keywords: sec-low
(Reporter)

Updated

6 years ago
Blocks: 301375
Current behavior across different browsers:

Firefox (Stylo & non-Stylo):
a\:b > \: { counter-increment: \d \\ 1; font-family: \;; }

Chrome:
a\:b > \: { counter-increment: \d \\ 1; font-family: ";"; }

Edge:
a:b > : { font-family: ;; }

I have no idea what the "correct" behavior is supposed to be here.
Has Regression Range: --- → irrelevant
All our escaping in the serialization in that test appears to be correct.  (That Chrome turns the identifier list font-family name into a string isn't a huge deal, though I think our serialization is very slightly more defensible.)

The one thing that looks incorrect is the inclusion of the "1" in the counter-increment serialization.  It looks like Firefox, Chrome and Safari all include the 1 but Edge doesn't.  While 1 is the amount that the counter would increment by default when not specified, this shouldn't affect what the specified value actually serializes to.
Filed bug 1408257 for the counter-increment serialization issue.  Since the escaping issues here are fixed, closing this bug.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.