- Revision
- 217844
- Author
- [email protected]
- Date
- 2017-06-06 11:27:53 -0700 (Tue, 06 Jun 2017)
Log 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 <[email protected]> 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
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (217843 => 217844)
--- trunk/LayoutTests/ChangeLog 2017-06-06 18:15:31 UTC (rev 217843)
+++ trunk/LayoutTests/ChangeLog 2017-06-06 18:27:53 UTC (rev 217844)
@@ -1,3 +1,13 @@
+2017-06-06 Emilio Cobos Álvarez <[email protected]>
+
+ [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 <[email protected]>
Unreviewed rollout r217807. Caused a test to crash.
Modified: trunk/LayoutTests/css3/supports-dom-api-expected.txt (217843 => 217844)
--- trunk/LayoutTests/css3/supports-dom-api-expected.txt 2017-06-06 18:15:31 UTC (rev 217843)
+++ trunk/LayoutTests/css3/supports-dom-api-expected.txt 2017-06-06 18:27:53 UTC (rev 217844)
@@ -3,8 +3,20 @@
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)))))))") 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
Modified: trunk/LayoutTests/css3/supports-dom-api.html (217843 => 217844)
--- trunk/LayoutTests/css3/supports-dom-api.html 2017-06-06 18:15:31 UTC (rev 217843)
+++ trunk/LayoutTests/css3/supports-dom-api.html 2017-06-06 18:27:53 UTC (rev 217844)
@@ -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.
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/cssom/CSS-expected.txt (217843 => 217844)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/cssom/CSS-expected.txt 2017-06-06 18:15:31 UTC (rev 217843)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/cssom/CSS-expected.txt 2017-06-06 18:27:53 UTC (rev 217844)
@@ -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
Modified: trunk/Source/WebCore/ChangeLog (217843 => 217844)
--- trunk/Source/WebCore/ChangeLog 2017-06-06 18:15:31 UTC (rev 217843)
+++ trunk/Source/WebCore/ChangeLog 2017-06-06 18:27:53 UTC (rev 217844)
@@ -1,3 +1,34 @@
+2017-06-06 Emilio Cobos Álvarez <[email protected]>
+
+ [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 <[email protected]>
Move Resource Timing / User Timing from experimental features into main preferences
Modified: trunk/Source/WebCore/css/parser/CSSParser.cpp (217843 => 217844)
--- trunk/Source/WebCore/css/parser/CSSParser.cpp 2017-06-06 18:15:31 UTC (rev 217843)
+++ trunk/Source/WebCore/css/parser/CSSParser.cpp 2017-06-06 18:27:53 UTC (rev 217844)
@@ -146,7 +146,7 @@
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)
Modified: trunk/Source/WebCore/css/parser/CSSParserImpl.cpp (217843 => 217844)
--- trunk/Source/WebCore/css/parser/CSSParserImpl.cpp 2017-06-06 18:15:31 UTC (rev 217843)
+++ trunk/Source/WebCore/css/parser/CSSParserImpl.cpp 2017-06-06 18:27:53 UTC (rev 217844)
@@ -562,7 +562,7 @@
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
Modified: trunk/Source/WebCore/css/parser/CSSSupportsParser.cpp (217843 => 217844)
--- trunk/Source/WebCore/css/parser/CSSSupportsParser.cpp 2017-06-06 18:15:31 UTC (rev 217843)
+++ trunk/Source/WebCore/css/parser/CSSSupportsParser.cpp 2017-06-06 18:27:53 UTC (rev 217844)
@@ -34,12 +34,19 @@
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 @@
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 @@
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
Modified: trunk/Source/WebCore/css/parser/CSSSupportsParser.h (217843 => 217844)
--- trunk/Source/WebCore/css/parser/CSSSupportsParser.h 2017-06-06 18:15:31 UTC (rev 217843)
+++ trunk/Source/WebCore/css/parser/CSSSupportsParser.h 2017-06-06 18:27:53 UTC (rev 217844)
@@ -44,8 +44,13 @@
Invalid
};
- static SupportsResult supportsCondition(CSSParserTokenRange, CSSParserImpl&);
+ enum SupportsParsingMode {
+ ForAtRule,
+ ForWindowCSS,
+ };
+ static SupportsResult supportsCondition(CSSParserTokenRange, CSSParserImpl&, SupportsParsingMode);
+
private:
CSSSupportsParser(CSSParserImpl& parser)
: m_parser(parser) { }
@@ -52,6 +57,7 @@
SupportsResult consumeCondition(CSSParserTokenRange);
SupportsResult consumeNegation(CSSParserTokenRange);
+ SupportsResult consumeDeclarationConditionOrGeneralEnclosed(CSSParserTokenRange&);
SupportsResult consumeConditionInParenthesis(CSSParserTokenRange&, CSSParserTokenType);
