Changeset 176983 in webkit


Ignore:
Timestamp:
Dec 8, 2014 3:55:04 PM (9 years ago)
Author:
benjamin@webkit.org
Message:

Fix the parsing of advanced :lang() after r176902
​https://bugs.webkit.org/show_bug.cgi?id=139379

Reviewed by Andreas Kling.

Source/WebCore:

There were two mistakes that were only caught in debug:

The lexer was not calling isIdentifierStart() before parseIdentifier().
Some identifier we were parsing should have been invalid.
This was caught with an assertion in parseIdentifier().

The other issue is that we were accumulating pointer to freed memory.
The tokenizer for LANGRANGE was creating a new string with a StringBuilder.
The problem is that CSSParserString does not keep the source string alive.
Consequently, the list of language range was accumulating pointers to dead
StringImpls.

The fix there is to simply extend the token to take the original asterisk character
from the input. That is not elegant but that's efficient and we know
the buffer lifetime.

  • css/CSSParser.cpp:

(WebCore::CSSParser::realLex):

  • css/CSSGrammar.y.in: Fix the indentation of a language range rule.

LayoutTests:

Unskip and update the tests.

All the interesting cases were covered, I just had to update
the expectations.

  • TestExpectations:
  • fast/css/css-selector-text-expected.txt:
  • fast/css/css-selector-text.html:
  • fast/css/parsing-css-lang-expected.txt:
  • fast/css/parsing-css-lang.html:
Location:
trunk
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r176978 r176983  
     12014-12-08  Benjamin Poulain  <benjamin@webkit.org>
     2
     3        Fix the parsing of advanced :lang() after r176902
     4        https://bugs.webkit.org/show_bug.cgi?id=139379
     5
     6        Reviewed by Andreas Kling.
     7
     8        Unskip and update the tests.
     9
     10        All the interesting cases were covered, I just had to update
     11        the expectations.
     12
     13        * TestExpectations:
     14        * fast/css/css-selector-text-expected.txt:
     15        * fast/css/css-selector-text.html:
     16        * fast/css/parsing-css-lang-expected.txt:
     17        * fast/css/parsing-css-lang.html:
     18
    1192014-12-08  Myles C. Maxfield  <mmaxfield@apple.com>
    220

  • trunk/LayoutTests/TestExpectations

    r176908 r176983  
    130130webkit.org/b/135390 fast/css/fontloader-page-cache.html [ Skip ]
    131131webkit.org/b/135390 http/tests/webfont/fontloader-loading-attribute.html [ Skip ]
    132 
    133 # regression due to r176902:
    134 webkit.org/b/139014 [ Debug ] fast/css/parsing-css-lang.html [ Skip ]
    135 webkit.org/b/139014 [ Debug ] fast/css/css-selector-text.html [ Skip ]
    136132
    137133# Various failures from the W3C CSS Shapes test suite import

  • trunk/LayoutTests/fast/css/css-selector-text-expected.txt

    r176902 r176983  
    4848PASS parseThenSerializeRule(':visited { }') is ':visited { }'
    4949
    50 PASS parseThenSerializeRule(':lang(*-) { }') is ':lang(*-) { }'
    51 PASS parseThenSerializeRule(':lang(*--) { }') is ':lang(*--) { }'
    52 PASS parseThenSerializeRule(':lang(*---) { }') is ':lang(*---) { }'
    53 PASS parseThenSerializeRule(':lang(*----) { }') is ':lang(*----) { }'
    54 
    5550PASS parseThenSerializeRule(':lang(*-ab) { }') is ':lang(*-ab) { }'
    5651PASS parseThenSerializeRule(':lang(*-ab-) { }') is ':lang(*-ab-) { }'
    57 PASS parseThenSerializeRule(':lang(*-1996) { }') is ':lang(*-1996) { }'
    5852PASS parseThenSerializeRule(':lang(*-DE-1996) { }') is ':lang(*-DE-1996) { }'
    5953
    … …  
    325319
    326320PASS parseThenSerializeRule(':lang(\\*    ) { }') is ':lang(*) { }'
    327 PASS parseThenSerializeRule(':lang(*-    ) { }') is ':lang(*-) { }'
    328321PASS parseThenSerializeRule(':lang(*-en    ) { }') is ':lang(*-en) { }'
    329322PASS parseThenSerializeRule(':lang(*-en-\\*    ) { }') is ':lang(*-en-*) { }'
    … …  
    360353PASS parseThenSerializeRule(':lang(   \\*  ,  id-\\*-sumatra  ) { }') is ':lang(*, id-*-sumatra) { }'
    361354PASS parseThenSerializeRule(':lang(   \\*   ,    id-\\*-sumatra  ) { }') is ':lang(*, id-*-sumatra) { }'
    362 
    363 PASS parseThenSerializeRule(':lang(*-1996) { }') is ':lang(*-1996) { }'
    364 PASS parseThenSerializeRule(':lang(*-1996, *-1997) { }') is ':lang(*-1996, *-1997) { }'
    365 PASS parseThenSerializeRule(':lang(*-1996, *-1997 ) { }') is ':lang(*-1996, *-1997) { }'
    366 PASS parseThenSerializeRule(':lang(   *-1996   ,  *-1997   ) { }') is ':lang(*-1996, *-1997) { }'
    367 PASS parseThenSerializeRule(':lang(   *-1996   ,*-1997   ) { }') is ':lang(*-1996, *-1997) { }'
    368 PASS parseThenSerializeRule(':lang(   *-1996,*-1997   ) { }') is ':lang(*-1996, *-1997) { }'
    369355
    370356PASS parseThenSerializeRule(':lang(en-\\*) { }') is ':lang(en-*) { }'
    … …  
    397383PASS parseThenSerializeRule(':lang([]) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
    398384PASS parseThenSerializeRule(':lang(@media screen {}) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
    399 PASS parseThenSerializeRule(':lang(*)') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
    400 PASS parseThenSerializeRule(':lang(**)') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
    401 PASS parseThenSerializeRule(':lang(-*-)') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
     385PASS parseThenSerializeRule(':lang(*-) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
     386PASS parseThenSerializeRule(':lang(*-    ) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
     387PASS parseThenSerializeRule(':lang(*--) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
     388PASS parseThenSerializeRule(':lang(*---) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
     389PASS parseThenSerializeRule(':lang(*----) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
     390PASS parseThenSerializeRule(':lang(*)' { }) threw exception SyntaxError: Unexpected token '{'. Expected ')' to end a argument list..
     391PASS parseThenSerializeRule(':lang(**) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
     392PASS parseThenSerializeRule(':lang(-*-) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
    402393PASS parseThenSerializeRule(':lang(*-*) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
    403394PASS parseThenSerializeRule(':lang(de-*)') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
    … …  
    406397PASS parseThenSerializeRule(':lang(*-en-*fr) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
    407398PASS parseThenSerializeRule(':lang(*-*en-fr) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
    408 PASS parseThenSerializeRule(':lang(*-1997)') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
    409 PASS parseThenSerializeRule(':lang(*-1997-*)') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
     399PASS parseThenSerializeRule(':lang(*-1997) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
     400PASS parseThenSerializeRule(':lang(*-1997-*) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
    410401PASS parseThenSerializeRule(':lang(*a*) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
    411402PASS parseThenSerializeRule(':lang(*a) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
    … …  
    418409PASS parseThenSerializeRule(':lang(*-a, br fr) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
    419410PASS parseThenSerializeRule(':lang(*-a, br fr en *) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
     411PASS parseThenSerializeRule(':lang(*-1996, *-1997) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
     412PASS parseThenSerializeRule(':lang(*-1996, *-1997 ) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
     413PASS parseThenSerializeRule(':lang(   *-1996   ,  *-1997   ) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
     414PASS parseThenSerializeRule(':lang(   *-1996   ,*-1997   ) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
     415PASS parseThenSerializeRule(':lang(   *-1996,*-1997   ) { }') threw exception TypeError: undefined is not an object (evaluating 'styleElement.sheet.cssRules[0].cssText').
    420416
    421417PASS parseThenSerializeRule(':role(a) { }') is ':role(a) { }'

  • trunk/LayoutTests/fast/css/css-selector-text.html

    r176902 r176983  
    8181debug('');
    8282
    83 testSelectorRoundTrip(":lang(*-)");
    84 testSelectorRoundTrip(":lang(*--)");
    85 testSelectorRoundTrip(":lang(*---)");
    86 testSelectorRoundTrip(":lang(*----)");
    87 
    88 debug('');
    89 
    9083testSelectorRoundTrip(":lang(*-ab)");
    9184testSelectorRoundTrip(":lang(*-ab-)");
    92 testSelectorRoundTrip(":lang(*-1996)");
    9385testSelectorRoundTrip(":lang(*-DE-1996)");
    9486
    … …  
    418410
    419411shouldBe("parseThenSerializeRule(':lang(\\\\*    ) { }')", "':lang(*) { }'");
    420 shouldBe("parseThenSerializeRule(':lang(*-    ) { }')", "':lang(*-) { }'");
    421412shouldBe("parseThenSerializeRule(':lang(*-en    ) { }')", "':lang(*-en) { }'");
    422413shouldBe("parseThenSerializeRule(':lang(*-en-\\\\*    ) { }')", "':lang(*-en-*) { }'");
    … …  
    459450shouldBe("parseThenSerializeRule(':lang(   \\\\*  ,  id-\\\\*-sumatra  ) { }')", "':lang(*, id-*-sumatra) { }'");
    460451shouldBe("parseThenSerializeRule(':lang(   \\\\*   ,    id-\\\\*-sumatra  ) { }')", "':lang(*, id-*-sumatra) { }'");
    461 
    462 debug('');
    463 
    464 shouldBe("parseThenSerializeRule(':lang(*-1996) { }')", "':lang(*-1996) { }'");
    465 shouldBe("parseThenSerializeRule(':lang(*-1996, *-1997) { }')", "':lang(*-1996, *-1997) { }'");
    466 shouldBe("parseThenSerializeRule(':lang(*-1996, *-1997 ) { }')", "':lang(*-1996, *-1997) { }'");
    467 shouldBe("parseThenSerializeRule(':lang(   *-1996   ,  *-1997   ) { }')", "':lang(*-1996, *-1997) { }'");
    468 shouldBe("parseThenSerializeRule(':lang(   *-1996   ,*-1997   ) { }')", "':lang(*-1996, *-1997) { }'");
    469 shouldBe("parseThenSerializeRule(':lang(   *-1996,*-1997   ) { }')", "':lang(*-1996, *-1997) { }'");
    470452
    471453debug('');
    … …  
    503485shouldThrow("parseThenSerializeRule(':lang(@media screen {}) { }')");
    504486
    505 shouldThrow("parseThenSerializeRule(':lang(*)')");
    506 shouldThrow("parseThenSerializeRule(':lang(**)')");
    507 shouldThrow("parseThenSerializeRule(':lang(-*-)')");
     487shouldThrow("parseThenSerializeRule(':lang(*-) { }')");
     488shouldThrow("parseThenSerializeRule(':lang(*-    ) { }')");
     489shouldThrow("parseThenSerializeRule(':lang(*--) { }')");
     490shouldThrow("parseThenSerializeRule(':lang(*---) { }')");
     491shouldThrow("parseThenSerializeRule(':lang(*----) { }')");
     492
     493shouldThrow("parseThenSerializeRule(':lang(*)' { })");
     494shouldThrow("parseThenSerializeRule(':lang(**) { }')");
     495shouldThrow("parseThenSerializeRule(':lang(-*-) { }')");
    508496shouldThrow("parseThenSerializeRule(':lang(*-*) { }')");
    509497shouldThrow("parseThenSerializeRule(':lang(de-*)')");
    … …  
    512500shouldThrow("parseThenSerializeRule(':lang(*-en-*fr) { }')");
    513501shouldThrow("parseThenSerializeRule(':lang(*-*en-fr) { }')");
    514 shouldThrow("parseThenSerializeRule(':lang(*-1997)')");
    515 shouldThrow("parseThenSerializeRule(':lang(*-1997-*)')");
     502shouldThrow("parseThenSerializeRule(':lang(*-1997) { }')");
     503shouldThrow("parseThenSerializeRule(':lang(*-1997-*) { }')");
    516504shouldThrow("parseThenSerializeRule(':lang(*a*) { }')");
    517505shouldThrow("parseThenSerializeRule(':lang(*a) { }')");
    … …  
    525513shouldThrow("parseThenSerializeRule(':lang(*-a, br fr en *) { }')");
    526514
     515shouldThrow("parseThenSerializeRule(':lang(*-1996, *-1997) { }')");
     516shouldThrow("parseThenSerializeRule(':lang(*-1996, *-1997 ) { }')");
     517shouldThrow("parseThenSerializeRule(':lang(   *-1996   ,  *-1997   ) { }')");
     518shouldThrow("parseThenSerializeRule(':lang(   *-1996   ,*-1997   ) { }')");
     519shouldThrow("parseThenSerializeRule(':lang(   *-1996,*-1997   ) { }')");
     520
    527521debug('');
    528522

  • trunk/LayoutTests/fast/css/parsing-css-lang-expected.txt

    r176902 r176983  
    125125PASS document.getElementById('style-container').sheet.cssRules.length is 1
    126126PASS document.getElementById('style-container').sheet.cssRules[0].selectorText is ":lang(rm-CH)"
    127 PASS document.querySelector(":lang(*-)") did not throw exception.
    128 PASS document.getElementById('style-container').sheet.cssRules.length is 1
    129 PASS document.getElementById('style-container').sheet.cssRules[0].selectorText is ":lang(*-)"
    130 PASS document.querySelector(":lang(*-    )") did not throw exception.
    131 PASS document.getElementById('style-container').sheet.cssRules.length is 1
    132 PASS document.getElementById('style-container').sheet.cssRules[0].selectorText is ":lang(*-)"
    133 PASS document.querySelector(":lang(*--)") did not throw exception.
    134 PASS document.getElementById('style-container').sheet.cssRules.length is 1
    135 PASS document.getElementById('style-container').sheet.cssRules[0].selectorText is ":lang(*--)"
    136 PASS document.querySelector(":lang(*--    )") did not throw exception.
    137 PASS document.getElementById('style-container').sheet.cssRules.length is 1
    138 PASS document.getElementById('style-container').sheet.cssRules[0].selectorText is ":lang(*--)"
    139 PASS document.querySelector(":lang(*---)") did not throw exception.
    140 PASS document.getElementById('style-container').sheet.cssRules.length is 1
    141 PASS document.getElementById('style-container').sheet.cssRules[0].selectorText is ":lang(*---)"
    142 PASS document.querySelector(":lang(*---    )") did not throw exception.
    143 PASS document.getElementById('style-container').sheet.cssRules.length is 1
    144 PASS document.getElementById('style-container').sheet.cssRules[0].selectorText is ":lang(*---)"
    145 PASS document.querySelector(":lang(*----)") did not throw exception.
    146 PASS document.getElementById('style-container').sheet.cssRules.length is 1
    147 PASS document.getElementById('style-container').sheet.cssRules[0].selectorText is ":lang(*----)"
    148 PASS document.querySelector(":lang(*----    )") did not throw exception.
    149 PASS document.getElementById('style-container').sheet.cssRules.length is 1
    150 PASS document.getElementById('style-container').sheet.cssRules[0].selectorText is ":lang(*----)"
    151127PASS document.querySelector(":lang(*-CH)") did not throw exception.
    152128PASS document.getElementById('style-container').sheet.cssRules.length is 1
    … …  
    161137PASS document.getElementById('style-container').sheet.cssRules.length is 1
    162138PASS document.getElementById('style-container').sheet.cssRules[0].selectorText is ":lang(*-DE-1996)"
    163 PASS document.querySelector(":lang(*-1996)") did not throw exception.
    164 PASS document.getElementById('style-container').sheet.cssRules.length is 1
    165 PASS document.getElementById('style-container').sheet.cssRules[0].selectorText is ":lang(*-1996)"
    166 PASS document.querySelector(":lang(*-1996    )") did not throw exception.
    167 PASS document.getElementById('style-container').sheet.cssRules.length is 1
    168 PASS document.getElementById('style-container').sheet.cssRules[0].selectorText is ":lang(*-1996)"
    169139PASS document.querySelector(":lang(*-br-zh)") did not throw exception.
    170140PASS document.getElementById('style-container').sheet.cssRules.length is 1

  • trunk/LayoutTests/fast/css/parsing-css-lang.html

    r176902 r176983  
    4747    "rm-CH",
    4848
    49     "*-",
    50     "*--",
    51     "*---",
    52     "*----",
    53 
    5449    "*-CH",
    5550    "*-DE-1996",
    56     "*-1996",
    5751    "*-br-zh",
    5852    "id-\\*-sumatra",

  • trunk/Source/WebCore/ChangeLog

    r176981 r176983  
     12014-12-08  Benjamin Poulain  <benjamin@webkit.org>
     2
     3        Fix the parsing of advanced :lang() after r176902
     4        https://bugs.webkit.org/show_bug.cgi?id=139379
     5
     6        Reviewed by Andreas Kling.
     7
     8        There were two mistakes that were only caught in debug:
     9
     10        The lexer was not calling isIdentifierStart() before parseIdentifier().
     11        Some identifier we were parsing should have been invalid.
     12        This was caught with an assertion in parseIdentifier().
     13
     14        The other issue is that we were accumulating pointer to freed memory.
     15        The tokenizer for LANGRANGE was creating a new string with a StringBuilder.
     16        The problem is that CSSParserString does not keep the source string alive.
     17        Consequently, the list of language range was accumulating pointers to dead
     18        StringImpls.
     19
     20        The fix there is to simply extend the token to take the original asterisk character
     21        from the input. That is not elegant but that's efficient and we know
     22        the buffer lifetime.
     23
     24        * css/CSSParser.cpp:
     25        (WebCore::CSSParser::realLex):
     26        * css/CSSGrammar.y.in: Fix the indentation of a language range rule.
     27
    1282014-12-08  Anders Carlsson  <andersca@apple.com>
    229

  • trunk/Source/WebCore/css/CSSGrammar.y.in

    r176902 r176983  
    13961396        $$ = nullptr;
    13971397        if ($4) {
    1398           auto selector = std::make_unique<CSSParserSelector>();
    1399           selector->setMatch(CSSSelector::PseudoClass);
    1400           selector->setArgumentList(*std::unique_ptr<Vector<CSSParserString>>($4));
    1401           selector->setPseudoClassValue($2);
    1402           if (selector->pseudoClassType() == CSSSelector::PseudoClassLang)
    1403               $$ = selector.release();
     1398            auto selector = std::make_unique<CSSParserSelector>();
     1399            selector->setMatch(CSSSelector::PseudoClass);
     1400            selector->setArgumentList(*std::unique_ptr<Vector<CSSParserString>>($4));
     1401            selector->setPseudoClassValue($2);
     1402            if (selector->pseudoClassType() == CSSSelector::PseudoClassLang)
     1403                $$ = selector.release();
    14041404        }
    14051405    }

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

    r176907 r176983  
    1148811488            m_token = CONTAINS;
    1148911489#if ENABLE(CSS_SELECTORS_LEVEL4)
    11490         } else if (*currentCharacter<SrcCharacterType>() == '-') {
     11490        } else if (*currentCharacter<SrcCharacterType>() == '-' && isIdentifierStart<SrcCharacterType>()) {
    1149111491            result = currentCharacter<SrcCharacterType>();
    1149211492
    … …  
    1149411494            parseIdentifier(result, parsedIdentifier, hasEscape);
    1149511495
    11496             StringBuilder parsedLangRange;
    11497             parsedLangRange.append('*');
    11498             parsedLangRange.append(parsedIdentifier);
    11499 
    11500             m_token = LANGRANGE;
    11501             yylval->string.init(parsedLangRange.toString());
     11496            if (parsedIdentifier.length()) {
     11497                m_token = LANGRANGE;
     11498                yylval->string.init(tokenStart<SrcCharacterType>(), parsedIdentifier.length() + 1);
     11499            }
    1150211500#endif
    1150311501        }
Note: See TracChangeset for help on using the changeset viewer.