diff options
author | bors-servo <lbergstrom+bors@mozilla.com> | 2017-01-26 01:18:00 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-01-26 01:18:00 -0800 |
commit | 58f4804ef5ae8d1aca1790a1567214b6443d23b2 (patch) | |
tree | ce080693b551559c307cab8cb5d93c4a2a4214ad | |
parent | e4a1cb6f87ac1ba2035c833dca5d9619edeefb8d (diff) | |
parent | 8b0d8cbbf070fdbe1194c38649f7f4f25cf40671 (diff) | |
download | servo-58f4804ef5ae8d1aca1790a1567214b6443d23b2.tar.gz servo-58f4804ef5ae8d1aca1790a1567214b6443d23b2.zip |
Auto merge of #15183 - servo:font-family, r=nox
Fix parsing and serialization of font-family
<!-- Please describe your changes on the following line: -->
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #15059 (github issue number if applicable).
<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15183)
<!-- Reviewable:end -->
-rw-r--r-- | components/style/properties/build.py | 4 | ||||
-rw-r--r-- | components/style/properties/longhand/font.mako.rs | 34 | ||||
-rw-r--r-- | components/style/properties/properties.mako.rs | 4 | ||||
-rw-r--r-- | tests/wpt/mozilla/meta/MANIFEST.json | 6 | ||||
-rw-r--r-- | tests/wpt/mozilla/tests/css/test_font_family_parsing.html | 276 |
5 files changed, 320 insertions, 4 deletions
diff --git a/components/style/properties/build.py b/components/style/properties/build.py index cee20455871..2c0d167c89f 100644 --- a/components/style/properties/build.py +++ b/components/style/properties/build.py @@ -49,7 +49,9 @@ def abort(message): def render(filename, **context): try: - lookup = TemplateLookup(directories=[BASE]) + lookup = TemplateLookup(directories=[BASE], + input_encoding="utf8", + strict_undefined=True) template = Template(open(filename, "rb").read(), filename=filename, input_encoding="utf8", diff --git a/components/style/properties/longhand/font.mako.rs b/components/style/properties/longhand/font.mako.rs index fed23f14b71..d143398b2d8 100644 --- a/components/style/properties/longhand/font.mako.rs +++ b/components/style/properties/longhand/font.mako.rs @@ -19,7 +19,8 @@ impl NoViewportPercentage for SpecifiedValue {} pub mod computed_value { - use std::fmt; + use cssparser::CssStringWriter; + use std::fmt::{self, Write}; use Atom; use style_traits::ToCss; pub use self::FontFamily as SingleComputedValue; @@ -72,7 +73,16 @@ impl ToCss for FontFamily { fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - self.atom().with_str(|s| dest.write_str(s)) + match *self { + FontFamily::FamilyName(ref name) => { + dest.write_char('"')?; + write!(CssStringWriter::new(dest), "{}", name)?; + dest.write_char('"') + } + + // All generic values accepted by the parser are known to not require escaping. + FontFamily::Generic(ref name) => write!(dest, "{}", name), + } } } @@ -112,16 +122,36 @@ // FIXME(bholley): The fast thing to do here would be to look up the // string (as lowercase) in the static atoms table. We don't have an // API to do that yet though, so we do the simple thing for now. + let mut css_wide_keyword = false; match_ignore_ascii_case! { first_ident, "serif" => return Ok(FontFamily::Generic(atom!("serif"))), "sans-serif" => return Ok(FontFamily::Generic(atom!("sans-serif"))), "cursive" => return Ok(FontFamily::Generic(atom!("cursive"))), "fantasy" => return Ok(FontFamily::Generic(atom!("fantasy"))), "monospace" => return Ok(FontFamily::Generic(atom!("monospace"))), + + // https://drafts.csswg.org/css-fonts/#propdef-font-family + // "Font family names that happen to be the same as a keyword value + // (‘inherit’, ‘serif’, ‘sans-serif’, ‘monospace’, ‘fantasy’, and ‘cursive’) + // must be quoted to prevent confusion with the keywords with the same names. + // The keywords ‘initial’ and ‘default’ are reserved for future use + // and must also be quoted when used as font names. + // UAs must not consider these keywords as matching the <family-name> type." + "inherit" => css_wide_keyword = true, + "initial" => css_wide_keyword = true, + "unset" => css_wide_keyword = true, + "default" => css_wide_keyword = true, _ => {} } let mut value = first_ident.into_owned(); + // These keywords are not allowed by themselves. + // The only way this value can be valid with with another keyword. + if css_wide_keyword { + let ident = input.expect_ident()?; + value.push_str(" "); + value.push_str(&ident); + } while let Ok(ident) = input.try(|input| input.expect_ident()) { value.push_str(" "); value.push_str(&ident); diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index ca30e123dcc..e87105d3bf4 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -410,7 +410,9 @@ pub enum CSSWideKeyword { impl Parse for CSSWideKeyword { fn parse(_context: &ParserContext, input: &mut Parser) -> Result<Self, ()> { - match_ignore_ascii_case! { try!(input.expect_ident()), + let ident = input.expect_ident()?; + input.expect_exhausted()?; + match_ignore_ascii_case! { ident, "initial" => Ok(CSSWideKeyword::InitialKeyword), "inherit" => Ok(CSSWideKeyword::InheritKeyword), "unset" => Ok(CSSWideKeyword::UnsetKeyword), diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index bdff5ad1ed3..11c5fcc2ea2 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -6788,6 +6788,12 @@ "url": "/_mozilla/css/offset_properties_inline.html" } ], + "css/test_font_family_parsing.html": [ + { + "path": "css/test_font_family_parsing.html", + "url": "/_mozilla/css/test_font_family_parsing.html" + } + ], "css/test_variable_legal_values.html": [ { "path": "css/test_variable_legal_values.html", diff --git a/tests/wpt/mozilla/tests/css/test_font_family_parsing.html b/tests/wpt/mozilla/tests/css/test_font_family_parsing.html new file mode 100644 index 00000000000..a5551aad03b --- /dev/null +++ b/tests/wpt/mozilla/tests/css/test_font_family_parsing.html @@ -0,0 +1,276 @@ +<!DOCTYPE HTML> +<html> +<head> + <meta charset=utf-8> + <title>Font family name parsing tests</title> + <link rel="author" title="John Daggett" href="mailto:jdaggett@mozilla.com"> + <link rel="help" href="http://www.w3.org/TR/css3-fonts/#font-family-prop" /> + <link rel="help" href="http://www.w3.org/TR/css3-fonts/#font-prop" /> + <meta name="assert" content="tests that valid font family names parse and invalid ones don't" /> + <script type="text/javascript" src="/resources/testharness.js"></script> + <script type="text/javascript" src="/resources/testharnessreport.js"></script> + <style type="text/css"> + </style> +</head> +<body> +<div id="log"></div> +<pre id="display"></pre> +<style type="text/css" id="testbox"></style> + +<script type="text/javascript"> + +function fontProp(n, size, s1, s2) { return (s1 ? s1 + " " : "") + (s2 ? s2 + " " : "") + size + " " + n; } +function font(n, size, s1, s2) { return "font: " + fontProp(n, size, s1, s2); } + +// testrules +// namelist - font family list +// invalid - true if declarations won't parse in either font-family or font +// fontonly - only test with the 'font' property +// single - namelist includes only a single name (@font-face rules only allow a single name) + +var testFontFamilyLists = [ + + /* basic syntax */ + { namelist: "simple", single: true }, + { namelist: "'simple'", single: true }, + { namelist: '"simple"', single: true }, + { namelist: "-simple", single: true }, + { namelist: "_simple", single: true }, + { namelist: "quite simple", single: true }, + { namelist: "quite _simple", single: true }, + { namelist: "quite -simple", single: true }, + { namelist: "0simple", invalid: true, single: true }, + { namelist: "simple!", invalid: true, single: true }, + { namelist: "simple()", invalid: true, single: true }, + { namelist: "quite@simple", invalid: true, single: true }, + { namelist: "#simple", invalid: true, single: true }, + { namelist: "quite 0simple", invalid: true, single: true }, + { namelist: "納豆嫌い", single: true }, + { namelist: "納豆嫌い, ick, patooey" }, + { namelist: "ick, patooey, 納豆嫌い" }, + { namelist: "納豆嫌い, 納豆大嫌い" }, + { namelist: "納豆嫌い, 納豆大嫌い, 納豆本当に嫌い" }, + { namelist: "納豆嫌い, 納豆大嫌い, 納豆本当に嫌い, 納豆は好みではない" }, + { namelist: "arial, helvetica, sans-serif" }, + { namelist: "arial, helvetica, 'times' new roman, sans-serif", invalid: true }, + { namelist: "arial, helvetica, \"times\" new roman, sans-serif", invalid: true }, + + { namelist: "arial, helvetica, \"\\\"times new roman\", sans-serif" }, + { namelist: "arial, helvetica, '\\\"times new roman', sans-serif" }, + { namelist: "arial, helvetica, times 'new' roman, sans-serif", invalid: true }, + { namelist: "arial, helvetica, times \"new\" roman, sans-serif", invalid: true }, + { namelist: "\"simple", single: true }, + { namelist: "\\\"simple", single: true }, + { namelist: "\"\\\"simple\"", single: true }, + { namelist: "İsimple", single: true }, + { namelist: "ßsimple", single: true }, + { namelist: "ẙsimple", single: true }, + + /* escapes */ + { namelist: "\\s imple", single: true }, + { namelist: "\\073 imple", single: true }, + + { namelist: "\\035 simple", single: true }, + { namelist: "sim\\035 ple", single: true }, + { namelist: "simple\\02cinitial", single: true }, + { namelist: "simple, \\02cinitial" }, + { namelist: "sim\\020 \\035 ple", single: true }, + { namelist: "sim\\020 5ple", single: true }, + { namelist: "\\@simple", single: true }, + { namelist: "\\@simple\\;", single: true }, + { namelist: "\\@font-face", single: true }, + { namelist: "\\@font-face\\;", single: true }, + { namelist: "\\031 \\036 px", single: true }, + { namelist: "\\031 \\036 px", single: true }, + { namelist: "\\1f4a9", single: true }, + { namelist: "\\01f4a9", single: true }, + { namelist: "\\0001f4a9", single: true }, + { namelist: "\\AbAb", single: true }, + + /* keywords */ + { namelist: "italic", single: true }, + { namelist: "bold", single: true }, + { namelist: "bold italic", single: true }, + { namelist: "italic bold", single: true }, + { namelist: "larger", single: true }, + { namelist: "smaller", single: true }, + { namelist: "bolder", single: true }, + { namelist: "lighter", single: true }, + { namelist: "default", invalid: true, fontonly: true, single: true }, + { namelist: "initial", invalid: true, fontonly: true, single: true }, + { namelist: "inherit", invalid: true, fontonly: true, single: true }, + { namelist: "normal", single: true }, + { namelist: "default, simple", invalid: true }, + { namelist: "initial, simple", invalid: true }, + { namelist: "inherit, simple", invalid: true }, + { namelist: "normal, simple" }, + { namelist: "simple, default", invalid: true }, + { namelist: "simple, initial", invalid: true }, + { namelist: "simple, inherit", invalid: true }, + { namelist: "simple, default bongo" }, + { namelist: "simple, initial bongo" }, + { namelist: "simple, inherit bongo" }, + { namelist: "simple, bongo default" }, + { namelist: "simple, bongo initial" }, + { namelist: "simple, bongo inherit" }, + { namelist: "simple, normal" }, + { namelist: "simple default", single: true }, + { namelist: "simple initial", single: true }, + { namelist: "simple inherit", single: true }, + { namelist: "simple normal", single: true }, + { namelist: "default simple", single: true }, + { namelist: "initial simple", single: true }, + { namelist: "inherit simple", single: true }, + { namelist: "normal simple", single: true }, + { namelist: "caption", single: true }, // these are keywords for the 'font' property but only when in the first position + { namelist: "icon", single: true }, + { namelist: "menu", single: true } + +]; + +if (window.SpecialPowers && SpecialPowers.getBoolPref("layout.css.unset-value.enabled")) { + testFontFamilyLists.push( + { namelist: "unset", invalid: true, fontonly: true, single: true }, + { namelist: "unset, simple", invalid: true }, + { namelist: "simple, unset", invalid: true }, + { namelist: "simple, unset bongo" }, + { namelist: "simple, bongo unset" }, + { namelist: "simple unset", single: true }, + { namelist: "unset simple", single: true }); +} + +var gTest = 0; + +/* strip out just values */ +function extractDecl(rule) +{ + var t = rule.replace(/[ \n]+/g, " "); + t = t.replace(/.*{[ \n]*/, ""); + t = t.replace(/[ \n]*}.*/, ""); + return t; +} + + +function testStyleRuleParse(decl, invalid) { + var sheet = document.styleSheets[1]; + var rule = ".test" + gTest++ + " { " + decl + "; }"; + + while(sheet.cssRules.length > 0) { + sheet.deleteRule(0); + } + + // shouldn't throw but improper handling of punctuation may cause some parsers to throw + try { + sheet.insertRule(rule, 0); + } catch (e) { + assert_unreached("unexpected error with " + decl + " ==> " + e.name); + } + + assert_equals(sheet.cssRules.length, 1, + "strange number of rules (" + sheet.cssRules.length + ") with " + decl); + + var s = extractDecl(sheet.cssRules[0].cssText); + + if (invalid) { + assert_equals(s, "", "rule declaration shouldn't parse - " + rule + " ==> " + s); + } else { + assert_not_equals(s, "", "rule declaration should parse - " + rule); + + // check that the serialization also parses + var r = ".test" + gTest++ + " { " + s + " }"; + while(sheet.cssRules.length > 0) { + sheet.deleteRule(0); + } + try { + sheet.insertRule(r, 0); + } catch (e) { + assert_unreached("exception occurred parsing serialized form of rule - " + rule + " ==> " + r + " " + e.name); + } + var s2 = extractDecl(sheet.cssRules[0].cssText); + assert_not_equals(s2, "", "serialized form of rule should also parse - " + rule + " ==> " + r); + } +} + +var kDefaultFamilySetting = "onelittlepiggywenttomarket"; + +function testFontFamilySetterParse(namelist, invalid) { + var el = document.getElementById("display"); + + el.style.fontFamily = kDefaultFamilySetting; + var def = el.style.fontFamily; + el.style.fontFamily = namelist; + if (!invalid) { + assert_not_equals(el.style.fontFamily, def, "fontFamily setter should parse - " + namelist); + var parsed = el.style.fontFamily; + el.style.fontFamily = kDefaultFamilySetting; + el.style.fontFamily = parsed; + assert_equals(el.style.fontFamily, parsed, "fontFamily setter should parse serialized form to identical serialization - " + parsed + " ==> " + el.style.fontFamily); + } else { + assert_equals(el.style.fontFamily, def, "fontFamily setter shouldn't parse - " + namelist); + } +} + +var kDefaultFontSetting = "16px onelittlepiggywenttomarket"; + +function testFontSetterParse(n, size, s1, s2, invalid) { + var el = document.getElementById("display"); + + el.style.font = kDefaultFontSetting; + var def = el.style.font; + var fp = fontProp(n, size, s1, s2); + el.style.font = fp; + if (!invalid) { + assert_not_equals(el.style.font, def, "font setter should parse - " + fp); + var parsed = el.style.font; + el.style.font = kDefaultFontSetting; + el.style.font = parsed; + assert_equals(el.style.font, parsed, "font setter should parse serialized form to identical serialization - " + parsed + " ==> " + el.style.font); + } else { + assert_equals(el.style.font, def, "font setter shouldn't parse - " + fp); + } +} + +var testFontVariations = [ + { size: "16px" }, + { size: "900px" }, + { size: "900em" }, + { size: "35%" }, + { size: "7832.3%" }, + { size: "xx-large" }, + { size: "larger", s1: "lighter" }, + { size: "16px", s1: "italic" }, + { size: "16px", s1: "italic", s2: "bold" }, + { size: "smaller", s1: "normal" }, + { size: "16px", s1: "normal", s2: "normal" }, + { size: "16px", s1: "400", s2: "normal" }, + { size: "16px", s1: "bolder", s2: "oblique" } +]; + +function testFamilyNameParsing() { + var i; + for (i = 0; i < testFontFamilyLists.length; i++) { + var tst = testFontFamilyLists[i]; + var n = tst.namelist; + var t; + + if (!tst.fontonly) { + t = "font-family: " + n; + test(function() { testStyleRuleParse(t, tst.invalid); }, t); + test(function() { testFontFamilySetterParse(n, tst.invalid); }, t + " (setter)"); + } + + var v; + for (v = 0; v < testFontVariations.length; v++) { + var f = testFontVariations[v]; + t = font(n, f.size, f.s1, f.s2); + test(function() { testStyleRuleParse(t, tst.invalid); }, t); + test(function() { testFontSetterParse(n, f.size, f.s1, f.s2, tst.invalid); }, t + " (setter)"); + } + } +} + +testFamilyNameParsing(); + +</script> +</body> +</html> |