Changeset 230838 in webkit


Ignore:
Timestamp:
Apr 20, 2018 5:08:45 AM (6 years ago)
Author:
commit-queue@webkit.org
Message:

Omit default value when serializing font-feature-settings
​https://bugs.webkit.org/show_bug.cgi?id=182382

Patch by Chris Nardi <​cnardi@chromium.org> on 2018-04-20
Reviewed by Myles C. Maxfield.

Source/WebCore:

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 and Chrome.

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

Updated css3/font-feature-settings-parsing.html, fast/css/inherited-properties-rare-text.html,
and fast/text/font-face-javascript.html.

  • css/CSSFontFeatureValue.cpp:

(WebCore::CSSFontFeatureValue::customCSSText const):

LayoutTests:

Update tests to omit default value when serializing.

  • css3/font-feature-settings-parsing-expected.txt:
  • css3/font-feature-settings-parsing.html:
  • fast/css/inherited-properties-rare-text-expected.txt:
  • fast/text/font-face-javascript-expected.txt:
  • fast/text/font-face-javascript.html:
Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r230829 r230838  
     12018-04-20  Chris Nardi  <cnardi@chromium.org>
     2
     3        Omit default value when serializing font-feature-settings
     4        https://bugs.webkit.org/show_bug.cgi?id=182382
     5
     6        Reviewed by Myles C. Maxfield.
     7
     8        Update tests to omit default value when serializing.
     9
     10        * css3/font-feature-settings-parsing-expected.txt:
     11        * css3/font-feature-settings-parsing.html:
     12        * fast/css/inherited-properties-rare-text-expected.txt:
     13        * fast/text/font-face-javascript-expected.txt:
     14        * fast/text/font-face-javascript.html:
     15
    1162018-04-19  Dirk Schulze  <krit@webkit.org>
    217

  • trunk/LayoutTests/css3/font-feature-settings-parsing-expected.txt

    r227862 r230838  
    66- Tests valid inputs.
    77PASS canonicalize(parseResultOf("valid_normal")) is "normal"
    8 PASS canonicalize(parseResultOf("valid_value_1")) is "\"dlig\" 1"
     8PASS canonicalize(parseResultOf("valid_value_1")) is "\"dlig\""
    99PASS canonicalize(parseResultOf("valid_value_2")) is "\"swsh\" 2"
    10 PASS canonicalize(parseResultOf("valid_value_on")) is "\"smcp\" 1"
     10PASS canonicalize(parseResultOf("valid_value_on")) is "\"smcp\""
    1111PASS canonicalize(parseResultOf("valid_value_off")) is "\"liga\" 0"
    12 PASS canonicalize(parseResultOf("valid_value_omit")) is "\"c2sc\" 1"
    13 PASS canonicalize(parseResultOf("valid_valuelist")) is "\"hist\" 1, \"tnum\" 1"
    14 PASS canonicalize(parseResultOf("valid_singlequote")) is "\"pkrn\" 1"
    15 PASS canonicalize(parseResultOf("valid_unusual_tag")) is "\"!@#$\" 1"
    16 PASS canonicalize(parseResultOf("valid_tag_space")) is "\"a bc\" 1"
    17 PASS canonicalize(parseResultOf("valid_composite")) is "\"dlig\" 1, \"lig \" 0, \"smcp\" 1"
     12PASS canonicalize(parseResultOf("valid_value_omit")) is "\"c2sc\""
     13PASS canonicalize(parseResultOf("valid_valuelist")) is "\"hist\", \"tnum\""
     14PASS canonicalize(parseResultOf("valid_singlequote")) is "\"pkrn\""
     15PASS canonicalize(parseResultOf("valid_unusual_tag")) is "\"!@#$\""
     16PASS canonicalize(parseResultOf("valid_tag_space")) is "\"a bc\""
     17PASS canonicalize(parseResultOf("valid_composite")) is "\"dlig\", \"lig \" 0, \"smcp\""
    1818- Tests invalid inputs.  Results should be "normal".
    1919PASS parseResultOf("invalid_ident") is "normal"
    … …  
    3535PASS parseResultOf("invalid_0") is "normal"
    3636- Tests inherit.
    37 PASS canonicalize(parseResultOf("outer")) is "\"dlig\" 1"
    38 PASS canonicalize(parseResultOf("inner")) is "\"dlig\" 1"
     37PASS canonicalize(parseResultOf("outer")) is "\"dlig\""
     38PASS canonicalize(parseResultOf("inner")) is "\"dlig\""
    3939- Tests @font-face.
    40 PASS canonicalize(fontFaceRuleValid) is "\"liga\" 1"
     40PASS canonicalize(fontFaceRuleValid) is "\"liga\""
    4141PASS fontFaceRuleInvalid is ""
    4242PASS successfullyParsed is true

  • trunk/LayoutTests/css3/font-feature-settings-parsing.html

    r227862 r230838  
    185185debug('- Tests valid inputs.');
    186186shouldBeEqualToString('canonicalize(parseResultOf("valid_normal"))', canonicalize("normal"));
    187 shouldBeEqualToString('canonicalize(parseResultOf("valid_value_1"))', canonicalize("\"dlig\" 1"));
     187shouldBeEqualToString('canonicalize(parseResultOf("valid_value_1"))', canonicalize("\"dlig\""));
    188188shouldBeEqualToString('canonicalize(parseResultOf("valid_value_2"))', canonicalize("\"swsh\" 2"));
    189 shouldBeEqualToString('canonicalize(parseResultOf("valid_value_on"))', canonicalize("\"smcp\" 1"));
     189shouldBeEqualToString('canonicalize(parseResultOf("valid_value_on"))', canonicalize("\"smcp\""));
    190190shouldBeEqualToString('canonicalize(parseResultOf("valid_value_off"))', canonicalize("\"liga\" 0"));
    191 shouldBeEqualToString('canonicalize(parseResultOf("valid_value_omit"))', canonicalize("\"c2sc\" 1"));
    192 shouldBeEqualToString('canonicalize(parseResultOf("valid_valuelist"))', canonicalize("\"tnum\" 1, \"hist\" 1"));
    193 shouldBeEqualToString('canonicalize(parseResultOf("valid_singlequote"))', canonicalize("\"PKRN\" 1"));
    194 shouldBeEqualToString('canonicalize(parseResultOf("valid_unusual_tag"))', canonicalize("\"!@#$\" 1"));
    195 shouldBeEqualToString('canonicalize(parseResultOf("valid_tag_space"))', canonicalize("\"a bc\" 1"));
    196 shouldBeEqualToString('canonicalize(parseResultOf("valid_composite"))', canonicalize("\"dlig\" 1, \"smcp\" 1, \"lig \" 0"));
     191shouldBeEqualToString('canonicalize(parseResultOf("valid_value_omit"))', canonicalize("\"c2sc\""));
     192shouldBeEqualToString('canonicalize(parseResultOf("valid_valuelist"))', canonicalize("\"tnum\", \"hist\""));
     193shouldBeEqualToString('canonicalize(parseResultOf("valid_singlequote"))', canonicalize("\"PKRN\""));
     194shouldBeEqualToString('canonicalize(parseResultOf("valid_unusual_tag"))', canonicalize("\"!@#$\""));
     195shouldBeEqualToString('canonicalize(parseResultOf("valid_tag_space"))', canonicalize("\"a bc\""));
     196shouldBeEqualToString('canonicalize(parseResultOf("valid_composite"))', canonicalize("\"dlig\", \"smcp\", \"lig \" 0"));
    197197
    198198debug('- Tests invalid inputs.  Results should be "normal".');
    … …  
    216216
    217217debug('- Tests inherit.');
    218 shouldBeEqualToString('canonicalize(parseResultOf("outer"))', canonicalize("\"dlig\" 1"));
    219 shouldBeEqualToString('canonicalize(parseResultOf("inner"))', canonicalize("\"dlig\" 1"));
     218shouldBeEqualToString('canonicalize(parseResultOf("outer"))', canonicalize("\"dlig\""));
     219shouldBeEqualToString('canonicalize(parseResultOf("inner"))', canonicalize("\"dlig\""));
    220220
    221221debug('- Tests @font-face.');
    222222var fontFaceRuleValid = document.styleSheets[1].cssRules[0].style['font-feature-settings'];
    223223var fontFaceRuleInvalid = document.styleSheets[1].cssRules[1].style['font-feature-settings'];
    224 shouldBeEqualToString('canonicalize(fontFaceRuleValid)', canonicalize("\"liga\" 1"));
     224shouldBeEqualToString('canonicalize(fontFaceRuleValid)', canonicalize("\"liga\""));
    225225shouldBeEqualToString('fontFaceRuleInvalid', "");
    226226

  • trunk/LayoutTests/fast/css/inherited-properties-rare-text-expected.txt

    r227862 r230838  
    1 test1 font-feature-settings: "dlig" 1
     1test1 font-feature-settings: "dlig"
    22test2 font-feature-settings: normal
    33test1 -webkit-font-smoothing: antialiased

  • trunk/LayoutTests/fast/text/font-face-javascript-expected.txt

    r227862 r230838  
    2323PASS new FontFace('family_name', 'url(\'asdf\')', {'variant': 'small-caps'}).variant is "small-caps"
    2424PASS new FontFace('family_name', 'url(\'asdf\')', {'variant': 'small-caps common-ligatures'}).variant is "common-ligatures small-caps"
    25 PASS new FontFace('family_name', 'url(\'asdf\')', {'featureSettings': '\'titl\''}).featureSettings is "\"titl\" 1"
     25PASS new FontFace('family_name', 'url(\'asdf\')', {'featureSettings': '\'titl\''}).featureSettings is "\"titl\""
    2626PASS everything.style is "italic"
    2727PASS everything.weight is "bold"
    … …  
    2929PASS everything.unicodeRange is "U+26-26"
    3030PASS everything.variant is "small-caps"
    31 PASS everything.featureSettings is "\"titl\" 1"
     31PASS everything.featureSettings is "\"titl\""
    3232PASS everything.family is "other_family_name"
    3333PASS everything.style is "normal"
    … …  
    3636PASS everything.unicodeRange is "U+0-7f"
    3737PASS everything.variant is "stacked-fractions"
    38 PASS everything.featureSettings is "\"smcp\" 1"
     38PASS everything.featureSettings is "\"smcp\""
    3939PASS new FontFace('family_name', 'url(\'asdf\')', {}) did not throw exception.
    4040PASS new FontFace('family_name', newArrayBuffer, {}) did not throw exception.

  • trunk/LayoutTests/fast/text/font-face-javascript.html

    r227862 r230838  
    3131shouldBeEqualToString("new FontFace('family_name', 'url(\\'asdf\\')', {'variant': 'small-caps'}).variant", "small-caps");
    3232shouldBeEqualToString("new FontFace('family_name', 'url(\\'asdf\\')', {'variant': 'small-caps common-ligatures'}).variant", "common-ligatures small-caps");
    33 shouldBeEqualToString("new FontFace('family_name', 'url(\\'asdf\\')', {'featureSettings': '\\'titl\\''}).featureSettings", "\"titl\" 1");
     33shouldBeEqualToString("new FontFace('family_name', 'url(\\'asdf\\')', {'featureSettings': '\\'titl\\''}).featureSettings", "\"titl\"");
    3434
    3535var everything = new FontFace('family_name', 'url(\'asdf\')', {'style': 'italic', 'weight': 'bold', 'stretch': 'extra-expanded', 'unicodeRange': 'U+26', 'variant': 'small-caps', 'featureSettings': '\'titl\''});
    … …  
    3939shouldBeEqualToString("everything.unicodeRange", "U+26-26");
    4040shouldBeEqualToString("everything.variant", "small-caps");
    41 shouldBeEqualToString("everything.featureSettings", "\"titl\" 1");
     41shouldBeEqualToString("everything.featureSettings", "\"titl\"");
    4242
    4343everything.family = "other_family_name";
    … …  
    5454shouldBeEqualToString("everything.variant", "stacked-fractions");
    5555everything.featureSettings = "'smcp'";
    56 shouldBeEqualToString("everything.featureSettings", "\"smcp\" 1");
     56shouldBeEqualToString("everything.featureSettings", "\"smcp\"");
    5757
    5858shouldNotThrow("new FontFace('family_name', 'url(\\'asdf\\')', {})");

  • trunk/Source/WebCore/ChangeLog

    r230836 r230838  
     12018-04-20  Chris Nardi  <cnardi@chromium.org>
     2
     3        Omit default value when serializing font-feature-settings
     4        https://bugs.webkit.org/show_bug.cgi?id=182382
     5
     6        Reviewed by Myles C. Maxfield.
     7
     8        According to the shortest-serialization principle [1], values should be omitted if their omission
     9        wouldn't change the value of reparsing. As "1"/"on" is the default value for font-feature-settings,
     10        omit this when serializing, matching the behavior of Firefox and Chrome.
     11
     12        [1]: https://github.com/w3c/csswg-drafts/issues/1564
     13
     14        Updated css3/font-feature-settings-parsing.html, fast/css/inherited-properties-rare-text.html,
     15        and fast/text/font-face-javascript.html.
     16
     17        * css/CSSFontFeatureValue.cpp:
     18        (WebCore::CSSFontFeatureValue::customCSSText const):
     19
    1202018-04-19  Alexey Proskuryakov  <ap@apple.com>
    221

  • trunk/Source/WebCore/css/CSSFontFeatureValue.cpp

    r227862 r230838  
    4545    for (char c : m_tag)
    4646        builder.append(c);
    47     builder.appendLiteral("\" ");
    48     builder.appendNumber(m_value);
     47    builder.append('"');
     48    // Omit the value if it's 1 as 1 is implied by default.
     49    if (m_value != 1) {
     50        builder.append(' ');
     51        builder.appendNumber(m_value);
     52    }
    4953    return builder.toString();
    5054}
Note: See TracChangeset for help on using the changeset viewer.