475216 - CSS serialization doesn't escape characters that need escaping

Status: RESOLVED FIXED

(Core :: DOM: CSS Object Model, defect)

Description

[Jesse Ruderman]

Attachment: testcase demonstrating a bunch of escaping issues

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Some of these are more than just escaping issues, like the IsPseudoElement function in nsCSSStyleRule.cpp, and perhaps bug 280443.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Bug 478160 and bug 481591 helped a few of the problems here, but there's still a good bit more to fix.

Comment 3

[Jesse Ruderman]

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

Comment 4

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 5

[Cameron McCormack (:heycam)]

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.

Comment 6

[Cameron McCormack (:heycam)]

Filed bug 1408257 for the counter-increment serialization issue.  Since the escaping issues here are fixed, closing this bug.