[css-conditional] The one-string version of CSS.supports should be wrapped in implied...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Jun 2017 18:27:53 +0000 (18:27 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Jun 2017 18:27:53 +0000 (18:27 +0000)
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

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@217844 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/css3/supports-dom-api-expected.txt
LayoutTests/css3/supports-dom-api.html
LayoutTests/imported/w3c/web-platform-tests/cssom/CSS-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/css/parser/CSSParser.cpp
Source/WebCore/css/parser/CSSParserImpl.cpp
Source/WebCore/css/parser/CSSSupportsParser.cpp
Source/WebCore/css/parser/CSSSupportsParser.h

index fca1251..c658238 100644 (file)
@@ -1,3 +1,13 @@
+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>
 
         Unreviewed rollout r217807. Caused a test to crash.
index 2370a9e..fc04e18 100644 (file)
@@ -3,8 +3,20 @@ Test window.CSS.supports()
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
+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
 PASS CSS.supports("not (not (display: none))") is true
@@ -27,6 +39,7 @@ PASS CSS.supports("(display: none)or(-webkit-transition: all 1s)") is true
 PASS CSS.supports("(display: none) or(-webkit-transition: all 1s    )") is true
 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
 PASS CSS.supports("(background-color: rgb(0, 128, 0))") is true
index 84e3f07..4324851 100644 (file)
@@ -7,9 +7,22 @@
 <script>
     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
     shouldBeTrue('CSS.supports("not (display: deadbeef)")');
     shouldBeFalse('CSS.supports("not (display: none)")');
@@ -43,6 +56,7 @@
     shouldBeTrue('CSS.supports("(display: none) or(-webkit-transition: all 1s    )")');
     shouldBeTrue('CSS.supports("(((((((display: none)))))))")');
     shouldBeFalse('CSS.supports("(!important)")');
+    shouldBeFalse('CSS.supports("!important")');
     shouldBeFalse('CSS.supports("not not not not (display: none)")');
 
     // Functions.
index 2493b13..3bafa7b 100644 (file)
@@ -1,5 +1,5 @@
 
 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
 
index f88250d..32fa248 100644 (file)
@@ -1,3 +1,34 @@
+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>
 
         Move Resource Timing / User Timing from experimental features into main preferences
index 69f9d95..09a72e4 100644 (file)
@@ -146,7 +146,7 @@ RefPtr<StyleRuleKeyframe> CSSParser::parseKeyframeRule(const String& string)
 bool CSSParser::parseSupportsCondition(const String& condition)
 {
     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;
 }
 
 Color CSSParser::parseColor(const String& string, bool strict)
index 8320cb7..e6d5032 100644 (file)
@@ -562,7 +562,7 @@ RefPtr<StyleRuleMedia> CSSParserImpl::consumeMediaRule(CSSParserTokenRange prelu
 
 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
 
index 0d71bec..a1d7bba 100644 (file)
 
 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);
 }
 
 enum ClauseType { Unresolved, Conjunction, Disjunction };
@@ -105,6 +112,16 @@ CSSSupportsParser::SupportsResult CSSSupportsParser::consumeNegation(CSSParserTo
     return result ? Unsupported : Supported;
 }
 
+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)
 {
     if (startTokenType == IdentToken && range.peek().type() != LeftParenthesisToken)
@@ -115,13 +132,7 @@ CSSSupportsParser::SupportsResult CSSSupportsParser::consumeConditionInParenthes
     SupportsResult result = consumeCondition(innerRange);
     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);
 }
 
 } // namespace WebCore
index ecf3c80..5b3940d 100644 (file)
@@ -44,7 +44,12 @@ public:
         Invalid
     };
 
-    static SupportsResult supportsCondition(CSSParserTokenRange, CSSParserImpl&);
+    enum SupportsParsingMode {
+        ForAtRule,
+        ForWindowCSS,
+    };
+
+    static SupportsResult supportsCondition(CSSParserTokenRange, CSSParserImpl&, SupportsParsingMode);
 
 private:
     CSSSupportsParser(CSSParserImpl& parser)
@@ -52,6 +57,7 @@ private:
 
     SupportsResult consumeCondition(CSSParserTokenRange);
     SupportsResult consumeNegation(CSSParserTokenRange);
+    SupportsResult consumeDeclarationConditionOrGeneralEnclosed(CSSParserTokenRange&);
 
     SupportsResult consumeConditionInParenthesis(CSSParserTokenRange&, CSSParserTokenType);