Closed
Bug 1355352
Opened 7 years ago
Closed 7 years ago
Parser doesn't completely reject unicode-range value when part of it is invalid
Categories
(Core :: CSS Parsing and Computation, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla55
| Tracking | Status | |
|---|---|---|
| firefox55 | --- | fixed |
People
(Reporter: xidorn, Assigned: xidorn)
References
Details
(Whiteboard: [stylo])
Attachments
(1 file)
Example code:
<!DOCTYPE HTML>
<style>
@font-face { unicode-range: U+100-17F,U+200-17F; }
</style>
<script>
alert(document.styleSheets[0].cssRules[0].style.getPropertyValue('unicode-range'));
</script>
In this case, the unicode-range descriptor should be completely rejected, but we currently keeping the valid part (U+100-17F).
Blink and Stylo do the right thing that the whole value is rejected.
(NOTE: This is not a Stylo bug, but a Gecko bug which has test on this.)| Comment hidden (mozreview-request) |
|
Assignee | |
Comment 2•7 years ago
|
||
(Probably better asking :jtd for review but I guess he's not active here nowadays...) So the spec says the following [1]: > Unicode codepoint values must be between 0 and 10FFFF inclusive. > For interval ranges, ... the end codepoint must be greater than or equal to the start codepoint. which indicates that those value are invalid. And per Simon Sapin, invalid value generally invalidates the whole declaration (descriptor here) in CSS, rather than being partially ignored. [1] https://drafts.csswg.org/css-fonts-3/#unicode-range-desc
Assignee: nobody → xidorn+moz
|
||
Comment 3•7 years ago
|
||
The spec even says explicitly that "Ranges that do not fit one of the these forms are invalid and cause the declaration to be ignored." From skimming discussion back in bug 443976, e.g. comment 18 there, it looks like at one time the spec had text that matched our current behavior, but it has since been tightened up and we have not changed to match. But if Blink behaves this way already, there shouldn't be much compat risk in fixing our implementation to follow the current spec.
|
|
||
Comment 4•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8867949 [details] Bug 1355352 - Reject unicode-range descriptor when its value contains invalid part. https://reviewboard.mozilla.org/r/139478/#review142960
Attachment #8867949 -
Flags: review?(jfkthame) → review+
|
||
Comment 5•7 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s e30cc47c3023 -d 9fa32a431bc8: rebasing 395967:e30cc47c3023 "Bug 1355352 - Reject unicode-range descriptor when its value contains invalid part. r=jfkthame" (tip) merging layout/style/nsCSSParser.cpp merging layout/style/test/stylo-failures.md warning: conflicts while merging layout/style/test/stylo-failures.md! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
| Comment hidden (mozreview-request) |
Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/538468358017 Reject unicode-range descriptor when its value contains invalid part. r=jfkthame
|
||
Comment 8•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/538468358017
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•