Open
Bug 1471772
Opened 6 years ago
Updated 2 years ago
U+0000 in a custom property value is not replaced with U+FFFD
Categories
(Core :: CSS Parsing and Computation, defect, P3)
Core
CSS Parsing and Computation
Tracking
()
NEW
People
(Reporter: SimonSapin, Unassigned)
Details
Reported in https://github.com/w3c/csswg-drafts/issues/2757#issuecomment-396525340 Test case: https://codepen.io/anon/pen/WyYeLv <div id="a"></div> <div id="b"></div> <script type="text/javascript"> var css = document.createElement("style"); css.innerHTML = '* { --a: hello\0world; }'; document.body.appendChild(css); a.innerHTML = JSON.stringify(css.sheet.cssRules[0].style.getPropertyValue("--a")); b.innerHTML = JSON.stringify(getComputedStyle(document.body).getPropertyValue('--a')); </script> The tokenizer does replace correctly, but for custom properties we store a slice of the unmodified original input as a single String, since that is more compact than Vec<Token>. We should probably do the replacement around here: https://searchfox.org/mozilla-central/rev/d2966246905102b36ef5221b0e3cbccf7ea15a86/servo/components/style/custom_properties.rs#305 There is a second issue where that string is null-truncated at some point between specified value and computed value, but fixing that first issue should stop NULLs from reaching far enough to trigger this.
Comment 1•6 years ago
|
||
Yeah, that sounds like the right place to do the replacement. Also I noticed that we can actually convert the string to owned in that function directly because every callsite invokes into_owned as well, so it doesn't really make much sense to keep Cow<str> there...
Reporter | ||
Comment 2•6 years ago
|
||
Re owned String, sounds good.
Comment 3•6 years ago
|
||
Reading the spec issue, it sounds like there is no need to fix this bug actually... We can just discard the whole thing afterward directly in this case, right?
Reporter | ||
Comment 4•6 years ago
|
||
Well that is one of the alternatives of a proposal under discussion, but this proposal doesn’t make consensus so far.
Updated•6 years ago
|
Priority: -- → P3
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•