Changeset 217844 in webkit

Timestamp:

Jun 6, 2017, 11:27:53 AM (8 years ago)

Author:

commit-queue@webkit.org

Message:

[css-conditional] The one-string version of CSS.supports should be wrapped in implied parentheses.
​https://bugs.webkit.org/show_bug.cgi?id=172906

Patch by Emilio Cobos Álvarez <​ecobos@igalia.com> on 2017-06-06
Reviewed by Darin Adler.

Source/WebCore:

From ​https://drafts.csswg.org/css-conditional/#the-css-interface:

When invoked with a single conditionText argument, it must return
true if conditionText, when either parsed and evaluated as a
supports_condition or parsed as a declaration, wrapped in implied
parentheses, and evaluated as a supports_condition, would return
true.

Note the "wrapped in implied parentheses" bit.

Gecko is fixing it in ​https://bugzil.la/1338486, and Blink in
​https://crbug.com/729403.

Tests: css3/supports-dom-api.html

imported/w3c/web-platform-tests/cssom/CSS.html

• css/parser/CSSParser.cpp:

(WebCore::CSSParser::parseSupportsCondition):

• css/parser/CSSParserImpl.cpp:

(WebCore::CSSParserImpl::consumeSupportsRule):

• css/parser/CSSSupportsParser.cpp:

(WebCore::CSSSupportsParser::supportsCondition):

• css/parser/CSSSupportsParser.h:

LayoutTests:

• css3/supports-dom-api-expected.txt:
• css3/supports-dom-api.html: Added test

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/css3/supports-dom-api-expected.txt (modified) (2 diffs)
• LayoutTests/css3/supports-dom-api.html (modified) (2 diffs)
• LayoutTests/imported/w3c/web-platform-tests/cssom/CSS-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/parser/CSSParser.cpp (modified) (1 diff)
• Source/WebCore/css/parser/CSSParserImpl.cpp (modified) (1 diff)
• Source/WebCore/css/parser/CSSSupportsParser.cpp (modified) (3 diffs)
• Source/WebCore/css/parser/CSSSupportsParser.h (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-06-06  Emilio Cobos Álvarez  <ecobos@igalia.com>
+
+        [css-conditional] The one-string version of CSS.supports should be wrapped in implied parentheses.
+        https://bugs.webkit.org/show_bug.cgi?id=172906
+
+        Reviewed by Darin Adler.
+
+        * css3/supports-dom-api-expected.txt:
+        * css3/supports-dom-api.html: Added test
+
 2017-06-06  Joseph Pecoraro  <pecoraro@apple.com>
 

trunk/LayoutTests/css3/supports-dom-api-expected.txt

@@ -4 +4 @@
 
 
+PASS CSS.supports("display: none") is true
 PASS CSS.supports("(display: none)") is true
+PASS CSS.supports("  display: none ") is true
 PASS CSS.supports("(display: deadbeef)") is false
+PASS CSS.supports("display: deadbeef") is false
+PASS CSS.supports("(display: none) and ((display: block) or (display: inline))") is true
+PASS CSS.supports("(not (display: deadbeef)) and (display: block)") is true
+PASS CSS.supports("top: -webkit-calc(80% - 20px)") is true
+PASS CSS.supports("background-color: rgb(0, 128, 0)") is true
+PASS CSS.supports("background: url('/blah')") is true
+PASS CSS.supports("background: invalid('/blah')") is false
+PASS CSS.supports("display: none;") is false
+PASS CSS.supports("display: none; garbage") is false
+PASS CSS.supports("  display: none ; garbage ") is false
 PASS CSS.supports("not (display: deadbeef)") is true
 PASS CSS.supports("not (display: none)") is false
@@ -28 +40 @@
 PASS CSS.supports("(((((((display: none)))))))") is true
 PASS CSS.supports("(!important)") is false
+PASS CSS.supports("!important") is false
 PASS CSS.supports("not not not not (display: none)") is false
 PASS CSS.supports("(top: -webkit-calc(80% - 20px))") is true

trunk/LayoutTests/css3/supports-dom-api.html

@@ -8 +8 @@
     description("Test window.CSS.supports()");
 
+    shouldBeTrue('CSS.supports("display: none")');
     shouldBeTrue('CSS.supports("(display: none)")');
+    shouldBeTrue('CSS.supports("  display: none ")');
     shouldBeFalse('CSS.supports("(display: deadbeef)")');
+
+    shouldBeFalse('CSS.supports("display: deadbeef")');
+    shouldBeTrue('CSS.supports("(display: none) and ((display: block) or (display: inline))")');
+    shouldBeTrue('CSS.supports("(not (display: deadbeef)) and (display: block)")');
+    shouldBeTrue('CSS.supports("top: -webkit-calc(80% - 20px)")');
+    shouldBeTrue('CSS.supports("background-color: rgb(0, 128, 0)")');
+    shouldBeTrue('CSS.supports("background: url(\'/blah\')")');
+    shouldBeFalse('CSS.supports("background: invalid(\'/blah\')")');
+    shouldBeFalse('CSS.supports("display: none;")');
+    shouldBeFalse('CSS.supports("display: none; garbage")');
+    shouldBeFalse('CSS.supports("  display: none ; garbage ")');
 
     // Negation
@@ -44 +57 @@
     shouldBeTrue('CSS.supports("(((((((display: none)))))))")');
     shouldBeFalse('CSS.supports("(!important)")');
+    shouldBeFalse('CSS.supports("!important")');
     shouldBeFalse('CSS.supports("not not not not (display: none)")');
 

trunk/LayoutTests/imported/w3c/web-platform-tests/cssom/CSS-expected.txt

@@ -1 +1 @@
 
 PASS CSS.escape 
-FAIL CSS.supports, one argument form assert_equals: CSS.supports: Single-argument form allows for declarations without enclosing parentheses expected true but got false
+PASS CSS.supports, one argument form 
 FAIL CSS.supports, two argument form assert_equals: CSS.supports: two argument form succeeds for custom property expected true but got false
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-06-06  Emilio Cobos Álvarez  <ecobos@igalia.com>
+
+        [css-conditional] The one-string version of CSS.supports should be wrapped in implied parentheses.
+        https://bugs.webkit.org/show_bug.cgi?id=172906
+
+        Reviewed by Darin Adler.
+
+        From https://drafts.csswg.org/css-conditional/#the-css-interface:
+
+        > When invoked with a single conditionText argument, it must return
+        > true if conditionText, when either parsed and evaluated as a
+        > supports_condition or parsed as a declaration, wrapped in implied
+        > parentheses, and evaluated as a supports_condition, would return
+        > true.
+
+        Note the "wrapped in implied parentheses" bit.
+
+        Gecko is fixing it in https://bugzil.la/1338486, and Blink in
+        https://crbug.com/729403.
+
+        Tests: css3/supports-dom-api.html
+               imported/w3c/web-platform-tests/cssom/CSS.html
+
+        * css/parser/CSSParser.cpp:
+        (WebCore::CSSParser::parseSupportsCondition):
+        * css/parser/CSSParserImpl.cpp:
+        (WebCore::CSSParserImpl::consumeSupportsRule):
+        * css/parser/CSSSupportsParser.cpp:
+        (WebCore::CSSSupportsParser::supportsCondition):
+        * css/parser/CSSSupportsParser.h:
+
 2017-06-06  Joseph Pecoraro  <pecoraro@apple.com>
 

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

@@ -147 +147 @@
 {
     CSSParserImpl parser(m_context, condition);
-    return CSSSupportsParser::supportsCondition(parser.tokenizer()->tokenRange(), parser) == CSSSupportsParser::Supported;
+    return CSSSupportsParser::supportsCondition(parser.tokenizer()->tokenRange(), parser, CSSSupportsParser::ForWindowCSS) == CSSSupportsParser::Supported;
 }
 

trunk/Source/WebCore/css/parser/CSSParserImpl.cpp

@@ -563 +563 @@
 RefPtr<StyleRuleSupports> CSSParserImpl::consumeSupportsRule(CSSParserTokenRange prelude, CSSParserTokenRange block)
 {
-    CSSSupportsParser::SupportsResult supported = CSSSupportsParser::supportsCondition(prelude, *this);
+    CSSSupportsParser::SupportsResult supported = CSSSupportsParser::supportsCondition(prelude, *this, CSSSupportsParser::ForAtRule);
     if (supported == CSSSupportsParser::Invalid)
         return nullptr; // Parse error, invalid @supports condition

trunk/Source/WebCore/css/parser/CSSSupportsParser.cpp

@@ -35 +35 @@
 namespace WebCore {
 
-CSSSupportsParser::SupportsResult CSSSupportsParser::supportsCondition(CSSParserTokenRange range, CSSParserImpl& parser)
+CSSSupportsParser::SupportsResult CSSSupportsParser::supportsCondition(CSSParserTokenRange range, CSSParserImpl& parser, SupportsParsingMode mode)
 {
     // FIXME: The spec allows leading whitespace in @supports but not CSS.supports,
     // but major browser vendors allow it in CSS.supports also.
     range.consumeWhitespace();
-    return CSSSupportsParser(parser).consumeCondition(range);
+    CSSSupportsParser supportsParser(parser);
+    auto result = supportsParser.consumeCondition(range);
+    if (mode != ForWindowCSS || result != Invalid)
+        return result;
+    // window.CSS.supports requires parsing as-if the condition was wrapped in
+    // parenthesis. The only productions that wouldn't have parsed above are the
+    // declaration condition or the general enclosed productions.
+    return supportsParser.consumeDeclarationConditionOrGeneralEnclosed(range);
 }
 
@@ -106 +113 @@
 }
 
+CSSSupportsParser::SupportsResult CSSSupportsParser::consumeDeclarationConditionOrGeneralEnclosed(CSSParserTokenRange& range)
+{
+    if (range.peek().type() == FunctionToken) {
+        range.consumeComponentValue();
+        return Unsupported;
+    }
+
+    return range.peek().type() == IdentToken && m_parser.supportsDeclaration(range) ? Supported : Unsupported;
+}
+
 CSSSupportsParser::SupportsResult CSSSupportsParser::consumeConditionInParenthesis(CSSParserTokenRange& range, CSSParserTokenType startTokenType)
 {
@@ -116 +133 @@
     if (result != Invalid)
         return result;
-    
-    if (innerRange.peek().type() == FunctionToken) {
-        innerRange.consumeComponentValue();
-        return Unsupported;
-    }
-
-    return innerRange.peek().type() == IdentToken && m_parser.supportsDeclaration(innerRange) ? Supported : Unsupported;
+    return consumeDeclarationConditionOrGeneralEnclosed(innerRange);
 }
 

trunk/Source/WebCore/css/parser/CSSSupportsParser.h

@@ -45 +45 @@
     };
 
-    static SupportsResult supportsCondition(CSSParserTokenRange, CSSParserImpl&);
+    enum SupportsParsingMode {
+        ForAtRule,
+        ForWindowCSS,
+    };
+
+    static SupportsResult supportsCondition(CSSParserTokenRange, CSSParserImpl&, SupportsParsingMode);
 
 private:
@@ -53 +58 @@
     SupportsResult consumeCondition(CSSParserTokenRange);
     SupportsResult consumeNegation(CSSParserTokenRange);
+    SupportsResult consumeDeclarationConditionOrGeneralEnclosed(CSSParserTokenRange&);
 
     SupportsResult consumeConditionInParenthesis(CSSParserTokenRange&, CSSParserTokenType);