Commit ba54acda authored by Chris Nardi's avatar Chris Nardi Committed by Commit Bot

Leave out the default value when serializing font-feature-settings

According to the shortest-serialization principle [1], values should be
omitted if their omission wouldn't change the value of reparsing. As
"1"/"on" is the default value for font-feature-settings, omit this when
serializing, matching the behavior of Firefox.

[1]: https://github.com/w3c/csswg-drafts/issues/1564

Bug: 807744
Change-Id: Ieb8b86aa66aa303a82a895c42373177cf7f13d07
Reviewed-on: https://chromium-review.googlesource.com/896203Reviewed-by: default avatarDarren Shen <shend@chromium.org>
Commit-Queue: Chris Nardi <cnardi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533566}
parent 3ebc01b3
...@@ -5,16 +5,16 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE ...@@ -5,16 +5,16 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
- Tests valid inputs. - Tests valid inputs.
PASS parseResultOf("valid_normal") is "normal" PASS parseResultOf("valid_normal") is "normal"
PASS parseResultOf("valid_value_1") is "\"dlig\" 1" PASS parseResultOf("valid_value_1") is "\"dlig\""
PASS parseResultOf("valid_value_2") is "\"swsh\" 2" PASS parseResultOf("valid_value_2") is "\"swsh\" 2"
PASS parseResultOf("valid_value_on") is "\"smcp\" 1" PASS parseResultOf("valid_value_on") is "\"smcp\""
PASS parseResultOf("valid_value_off") is "\"liga\" 0" PASS parseResultOf("valid_value_off") is "\"liga\" 0"
PASS parseResultOf("valid_value_omit") is "\"c2sc\" 1" PASS parseResultOf("valid_value_omit") is "\"c2sc\""
PASS parseResultOf("valid_valuelist") is "\"tnum\" 1, \"hist\" 1" PASS parseResultOf("valid_valuelist") is "\"tnum\", \"hist\""
PASS parseResultOf("valid_singlequote") is "\"PKRN\" 1" PASS parseResultOf("valid_singlequote") is "\"PKRN\""
PASS parseResultOf("valid_unusual_tag") is "\"!@#$\" 1" PASS parseResultOf("valid_unusual_tag") is "\"!@#$\""
PASS parseResultOf("valid_tag_space") is "\"a bc\" 1" PASS parseResultOf("valid_tag_space") is "\"a bc\""
PASS parseResultOf("valid_composite") is "\"dlig\" 1, \"smcp\" 1, \"lig \" 0" PASS parseResultOf("valid_composite") is "\"dlig\", \"smcp\", \"lig \" 0"
- Tests invalid inputs. Results should be "normal". - Tests invalid inputs. Results should be "normal".
PASS parseResultOf("invalid_ident") is "normal" PASS parseResultOf("invalid_ident") is "normal"
PASS parseResultOf("invalid_cases") is "normal" PASS parseResultOf("invalid_cases") is "normal"
...@@ -34,10 +34,10 @@ PASS parseResultOf("invalid_beginning_comma") is "normal" ...@@ -34,10 +34,10 @@ PASS parseResultOf("invalid_beginning_comma") is "normal"
PASS parseResultOf("invalid_on") is "normal" PASS parseResultOf("invalid_on") is "normal"
PASS parseResultOf("invalid_0") is "normal" PASS parseResultOf("invalid_0") is "normal"
- Tests inherit. - Tests inherit.
PASS parseResultOf("outer") is "\"dlig\" 1" PASS parseResultOf("outer") is "\"dlig\""
PASS parseResultOf("inner") is "\"dlig\" 1" PASS parseResultOf("inner") is "\"dlig\""
- Tests @font-face. - Tests @font-face.
PASS fontFaceRuleValid is "\"liga\" 1" PASS fontFaceRuleValid is "\"liga\""
PASS fontFaceRuleInvalid is "" PASS fontFaceRuleInvalid is ""
PASS successfullyParsed is true PASS successfullyParsed is true
......
...@@ -175,16 +175,16 @@ function parseResultOf(id) { ...@@ -175,16 +175,16 @@ function parseResultOf(id) {
debug('- Tests valid inputs.'); debug('- Tests valid inputs.');
shouldBeEqualToString('parseResultOf("valid_normal")', "normal"); shouldBeEqualToString('parseResultOf("valid_normal")', "normal");
shouldBeEqualToString('parseResultOf("valid_value_1")', '"dlig" 1'); shouldBeEqualToString('parseResultOf("valid_value_1")', '"dlig"');
shouldBeEqualToString('parseResultOf("valid_value_2")', '"swsh" 2'); shouldBeEqualToString('parseResultOf("valid_value_2")', '"swsh" 2');
shouldBeEqualToString('parseResultOf("valid_value_on")', '"smcp" 1'); shouldBeEqualToString('parseResultOf("valid_value_on")', '"smcp"');
shouldBeEqualToString('parseResultOf("valid_value_off")', '"liga" 0'); shouldBeEqualToString('parseResultOf("valid_value_off")', '"liga" 0');
shouldBeEqualToString('parseResultOf("valid_value_omit")', '"c2sc" 1'); shouldBeEqualToString('parseResultOf("valid_value_omit")', '"c2sc"');
shouldBeEqualToString('parseResultOf("valid_valuelist")', '"tnum" 1, "hist" 1'); shouldBeEqualToString('parseResultOf("valid_valuelist")', '"tnum", "hist"');
shouldBeEqualToString('parseResultOf("valid_singlequote")', '"PKRN" 1'); shouldBeEqualToString('parseResultOf("valid_singlequote")', '"PKRN"');
shouldBeEqualToString('parseResultOf("valid_unusual_tag")', '"!@#$" 1'); shouldBeEqualToString('parseResultOf("valid_unusual_tag")', '"!@#$"');
shouldBeEqualToString('parseResultOf("valid_tag_space")', '"a bc" 1'); shouldBeEqualToString('parseResultOf("valid_tag_space")', '"a bc"');
shouldBeEqualToString('parseResultOf("valid_composite")', '"dlig" 1, "smcp" 1, "lig " 0'); shouldBeEqualToString('parseResultOf("valid_composite")', '"dlig", "smcp", "lig " 0');
debug('- Tests invalid inputs. Results should be "normal".'); debug('- Tests invalid inputs. Results should be "normal".');
shouldBe('parseResultOf("invalid_ident")', '"normal"'); shouldBe('parseResultOf("invalid_ident")', '"normal"');
...@@ -206,13 +206,13 @@ shouldBe('parseResultOf("invalid_on")', '"normal"'); ...@@ -206,13 +206,13 @@ shouldBe('parseResultOf("invalid_on")', '"normal"');
shouldBe('parseResultOf("invalid_0")', '"normal"'); shouldBe('parseResultOf("invalid_0")', '"normal"');
debug('- Tests inherit.'); debug('- Tests inherit.');
shouldBeEqualToString('parseResultOf("outer")', '"dlig" 1'); shouldBeEqualToString('parseResultOf("outer")', '"dlig"');
shouldBeEqualToString('parseResultOf("inner")', '"dlig" 1'); shouldBeEqualToString('parseResultOf("inner")', '"dlig"');
debug('- Tests @font-face.'); debug('- Tests @font-face.');
var fontFaceRuleValid = document.styleSheets[1].cssRules[0].style['-webkit-font-feature-settings']; var fontFaceRuleValid = document.styleSheets[1].cssRules[0].style['-webkit-font-feature-settings'];
var fontFaceRuleInvalid = document.styleSheets[1].cssRules[1].style['-webkit-font-feature-settings']; var fontFaceRuleInvalid = document.styleSheets[1].cssRules[1].style['-webkit-font-feature-settings'];
shouldBeEqualToString('fontFaceRuleValid', '"liga" 1'); shouldBeEqualToString('fontFaceRuleValid', '"liga"');
shouldBeEqualToString('fontFaceRuleInvalid', ""); shouldBeEqualToString('fontFaceRuleInvalid', "");
</script> </script>
......
This is a testharness.js-based test. This is a testharness.js-based test.
Harness Error. harness_status.status = 1 , harness_status.message = 2 duplicate test names: "fontFeatureSettings of '@font-face { font-feature-settings: "smcp" 1; }' should be '"smcp" 1'", "webkitFontFeatureSettings of '@font-face { font-feature-settings: "smcp" 1; }' should be '"smcp" 1'" Harness Error. harness_status.status = 1 , harness_status.message = 2 duplicate test names: "fontFeatureSettings of '@font-face { font-feature-settings: "smcp"; }' should be '"smcp"'", "webkitFontFeatureSettings of '@font-face { font-feature-settings: "smcp"; }' should be '"smcp"'"
PASS 'fontFeatureSettings' in style PASS 'fontFeatureSettings' in style
PASS 'webkitFontFeatureSettings' in style PASS 'webkitFontFeatureSettings' in style
PASS fontFeatureSettings of '@font-face { font-feature-settings: "smcp" 1; }' should be '"smcp" 1' PASS fontFeatureSettings of '@font-face { font-feature-settings: "smcp"; }' should be '"smcp"'
PASS webkitFontFeatureSettings of '@font-face { font-feature-settings: "smcp" 1; }' should be '"smcp" 1' PASS webkitFontFeatureSettings of '@font-face { font-feature-settings: "smcp"; }' should be '"smcp"'
PASS fontFeatureSettings of '@font-face { font-feature-settings: "smcp" 1; }' should be '"smcp" 1' PASS fontFeatureSettings of '@font-face { font-feature-settings: "smcp"; }' should be '"smcp"'
PASS webkitFontFeatureSettings of '@font-face { font-feature-settings: "smcp" 1; }' should be '"smcp" 1' PASS webkitFontFeatureSettings of '@font-face { font-feature-settings: "smcp"; }' should be '"smcp"'
PASS fontFeatureSettings of 'font-feature-settings:"smcp" 1' should be '"smcp" 1' PASS fontFeatureSettings of 'font-feature-settings:"smcp" 1' should be '"smcp"'
PASS webkitFontFeatureSettings of 'font-feature-settings:"smcp" 1' should be '"smcp" 1' PASS webkitFontFeatureSettings of 'font-feature-settings:"smcp" 1' should be '"smcp"'
PASS fontFeatureSettings of '-webkit-font-feature-settings:"smcp" 1' should be '"smcp" 1' PASS fontFeatureSettings of '-webkit-font-feature-settings:"smcp" 1' should be '"smcp"'
PASS webkitFontFeatureSettings of '-webkit-font-feature-settings:"smcp" 1' should be '"smcp" 1' PASS webkitFontFeatureSettings of '-webkit-font-feature-settings:"smcp" 1' should be '"smcp"'
Harness: the test ran to completion. Harness: the test ran to completion.
...@@ -12,7 +12,7 @@ ...@@ -12,7 +12,7 @@
<div style='font-feature-settings:"smcp" 1'></div> <div style='font-feature-settings:"smcp" 1'></div>
<div style='-webkit-font-feature-settings:"smcp" 1'></div> <div style='-webkit-font-feature-settings:"smcp" 1'></div>
<script> <script>
var expected = '"smcp" 1'; var expected = '"smcp"';
var style = document.createElement("foo").style; var style = document.createElement("foo").style;
test(function () { test(function () {
......
...@@ -20,7 +20,7 @@ ...@@ -20,7 +20,7 @@
const div = document.querySelector("#test"); const div = document.querySelector("#test");
const div1 = document.querySelector("#test1"); const div1 = document.querySelector("#test1");
test(function() { test(function() {
assert_equals(getComputedStyle(div).fontFeatureSettings, '"vert" 1'); assert_equals(getComputedStyle(div).fontFeatureSettings, '"vert"');
assert_equals(getComputedStyle(div1).fontFeatureSettings, '"vert" 1'); assert_equals(getComputedStyle(div1).fontFeatureSettings, '"vert"');
}, "font-feature-settings should have its feature tag serialized with double quotes"); }, "font-feature-settings should be serialized with double quotes, and the default value of 1 should be omitted");
</script> </script>
...@@ -8,7 +8,7 @@ PASS ahemFace.style is "italic" ...@@ -8,7 +8,7 @@ PASS ahemFace.style is "italic"
PASS ahemFace.weight is "300" PASS ahemFace.weight is "300"
PASS ahemFace.unicodeRange is "U+0-3FF" PASS ahemFace.unicodeRange is "U+0-3FF"
PASS ahemFace.variant is "small-caps" PASS ahemFace.variant is "small-caps"
PASS ahemFace.featureSettings is "\"dlig\" 1" PASS ahemFace.featureSettings is "\"dlig\""
PASS ahemFace.display is "block" PASS ahemFace.display is "block"
PASS defaultFace.family is "defaultFace" PASS defaultFace.family is "defaultFace"
...@@ -33,7 +33,7 @@ PASS modifiedFace.style is "italic" ...@@ -33,7 +33,7 @@ PASS modifiedFace.style is "italic"
PASS modifiedFace.weight is "900" PASS modifiedFace.weight is "900"
PASS modifiedFace.unicodeRange is "U+0-3FF" PASS modifiedFace.unicodeRange is "U+0-3FF"
PASS modifiedFace.variant is "small-caps" PASS modifiedFace.variant is "small-caps"
PASS modifiedFace.featureSettings is "\"dlig\" 1, \"liga\" 0" PASS modifiedFace.featureSettings is "\"dlig\", \"liga\" 0"
PASS modifiedFace.display is "fallback" PASS modifiedFace.display is "fallback"
PASS face.style = '' threw exception SyntaxError: Failed to set the 'style' property on 'FontFace': Failed to set '' as a property value.. PASS face.style = '' threw exception SyntaxError: Failed to set the 'style' property on 'FontFace': Failed to set '' as a property value..
......
...@@ -31,7 +31,7 @@ function runTests() { ...@@ -31,7 +31,7 @@ function runTests() {
shouldBeEqualToString('ahemFace.weight', '300'); shouldBeEqualToString('ahemFace.weight', '300');
shouldBeEqualToString('ahemFace.unicodeRange', 'U+0-3FF'); shouldBeEqualToString('ahemFace.unicodeRange', 'U+0-3FF');
shouldBeEqualToString('ahemFace.variant', 'small-caps'); shouldBeEqualToString('ahemFace.variant', 'small-caps');
shouldBeEqualToString('ahemFace.featureSettings', '"dlig" 1'); shouldBeEqualToString('ahemFace.featureSettings', '"dlig"');
shouldBeEqualToString('ahemFace.display', 'block'); shouldBeEqualToString('ahemFace.display', 'block');
debug(''); debug('');
...@@ -76,7 +76,7 @@ function runTests() { ...@@ -76,7 +76,7 @@ function runTests() {
shouldBeEqualToString('modifiedFace.weight', '900'); shouldBeEqualToString('modifiedFace.weight', '900');
shouldBeEqualToString('modifiedFace.unicodeRange', 'U+0-3FF'); shouldBeEqualToString('modifiedFace.unicodeRange', 'U+0-3FF');
shouldBeEqualToString('modifiedFace.variant', 'small-caps'); shouldBeEqualToString('modifiedFace.variant', 'small-caps');
shouldBeEqualToString('modifiedFace.featureSettings', '"dlig" 1, "liga" 0'); shouldBeEqualToString('modifiedFace.featureSettings', '"dlig", "liga" 0');
shouldBeEqualToString('modifiedFace.display', 'fallback'); shouldBeEqualToString('modifiedFace.display', 'fallback');
debug(''); debug('');
......
test1 -webkit-font-feature-settings: "dlig" 1 test1 -webkit-font-feature-settings: "dlig"
test2 -webkit-font-feature-settings: normal test2 -webkit-font-feature-settings: normal
test1 -webkit-font-smoothing: antialiased test1 -webkit-font-smoothing: antialiased
test2 -webkit-font-smoothing: auto test2 -webkit-font-smoothing: auto
......
...@@ -37,8 +37,12 @@ String CSSFontFeatureValue::CustomCSSText() const { ...@@ -37,8 +37,12 @@ String CSSFontFeatureValue::CustomCSSText() const {
StringBuilder builder; StringBuilder builder;
builder.Append('"'); builder.Append('"');
builder.Append(tag_); builder.Append(tag_);
builder.Append("\" "); builder.Append('"');
builder.AppendNumber(value_); // Omit the value if it's 1 as 1 is implied by default.
if (value_ != 1) {
builder.Append(' ');
builder.AppendNumber(value_);
}
return builder.ToString(); return builder.ToString();
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment