Commit 78e05d79 authored by Anders Hartvoll Ruud's avatar Anders Hartvoll Ruud Committed by Commit Bot

Don't require whitespace between and/or and <supports-in-parens>

The spec does not say that a whitespace is required here, yet Blink
requires it.

This aligns us with Firefox, Safari and spec.

Bug: 1053910
Change-Id: I7b1d00e6b6c5003039601765eaf3a738e59f52dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2107487Reviewed-by: default avatarXiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751628}
parent b809a5af
......@@ -13,27 +13,6 @@ namespace blink {
namespace {
// TODO(crbug.com/1053910): This is not correct, but it's here to make it
// possible to make web-facing changes separately.
//
// According to our (non-WPT) tests, the following is not a valid @supports
// rule:
//
// @supports (left:0)and (top:0) {}
// ^^
// The ")and" part of this is considered illegal by our tests, since there is
// no whitespace between ")" and "and". However, it's actually perfectly legal,
// since ")and" will tokenize into: <)-token> followed by <ident-token>. There
// is no reason why this wouldn't match the <supports-condition> production
// just because whitespace is missing.
//
// See also https://drafts.csswg.org/css-values-3/#component-whitespace
bool ConsumeRequiredWhitespaceOrEOF(CSSParserTokenRange& range) {
if (range.AtEnd())
return true;
return range.ConsumeIncludingWhitespace().GetType() == kWhitespaceToken;
}
// The result kUnknown must be converted to 'false' if passed to a context
// which requires a boolean value.
// TODO(crbug.com/1052274): This is supposed to happen at the top-level,
......@@ -141,8 +120,7 @@ CSSSupportsParser::Result CSSSupportsParser::ConsumeSupportsInParens(
if (range.Peek().GetType() == kLeftParenthesisToken) {
auto block = range.ConsumeBlock();
block.ConsumeWhitespace();
if (!ConsumeRequiredWhitespaceOrEOF(range))
return Result::kParseFailure;
range.ConsumeWhitespace();
Result result = ConsumeSupportsCondition(block);
if (result != Result::kParseFailure)
return result;
......@@ -193,8 +171,7 @@ CSSSupportsParser::Result CSSSupportsParser::ConsumeSupportsSelectorFn(
return Result::kParseFailure;
auto block = range.ConsumeBlock();
block.ConsumeWhitespace();
if (!ConsumeRequiredWhitespaceOrEOF(range))
return Result::kParseFailure;
range.ConsumeWhitespace();
if (CSSSelectorParser::SupportsComplexSelector(block, parser_.GetContext()))
return Result::kSupported;
return Result::kUnsupported;
......@@ -207,8 +184,7 @@ CSSSupportsParser::Result CSSSupportsParser::ConsumeSupportsDecl(
return Result::kParseFailure;
auto block = range.ConsumeBlock();
block.ConsumeWhitespace();
if (!ConsumeRequiredWhitespaceOrEOF(range))
return Result::kParseFailure;
range.ConsumeWhitespace();
if (block.Peek().GetType() != kIdentToken)
return Result::kParseFailure;
if (parser_.SupportsDeclaration(block))
......
......@@ -180,18 +180,15 @@ TEST_F(CSSSupportsParserTest, ConsumeSupportsInParens) {
// <general-enclosed>
EXPECT_EQ(Result::kUnsupported, ConsumeSupportsInParens("asdf(1)"));
// TODO(crbug.com/1053910): This is supposed to be kSupported, but there's
// currently a bug in Blink. This is here to hit those code paths until the
// bug is fixed.
EXPECT_EQ(Result::kParseFailure,
EXPECT_EQ(Result::kSupported,
ConsumeSupportsInParens("(color:red)and (color:green)"));
EXPECT_EQ(Result::kParseFailure,
EXPECT_EQ(Result::kSupported,
ConsumeSupportsInParens("(color:red)or (color:green)"));
{
ScopedCSSSupportsSelectorForTest css_supports_selector(true);
EXPECT_EQ(Result::kUnsupported,
EXPECT_EQ(Result::kSupported,
ConsumeSupportsInParens("selector(div)or (color:green)"));
EXPECT_EQ(Result::kUnsupported,
EXPECT_EQ(Result::kSupported,
ConsumeSupportsInParens("selector(div)and (color:green)"));
}
}
......
......@@ -33,7 +33,7 @@ PASS CSS.supports("(not (border: 1px 1px 1px 1px 1px solid #000)) and (display:
PASS CSS.supports("(display: block !important) and ((display: inline) or (display: deadbeef))") is true
PASS CSS.supports("not ((not (display: block)) or ((display: none) and (deadbeef: 1px)))") is true
PASS CSS.supports("not( display: deadbeef)") is false
PASS CSS.supports("(display: none)and ( -webkit-transition: all 1s )") is false
PASS CSS.supports("(display: none)and ( -webkit-transition: all 1s )") is true
PASS CSS.supports("(display: none)or(-webkit-transition: all 1s)") is false
PASS CSS.supports("(display: none) or(-webkit-transition: all 1s )") is false
PASS CSS.supports("(((((((display: none)))))))") is true
......
......@@ -51,7 +51,7 @@
// Whitespace/Syntax.
shouldBeFalse('CSS.supports("not( display: deadbeef)")');
shouldBeFalse('CSS.supports("(display: none)and ( -webkit-transition: all 1s )")');
shouldBeTrue('CSS.supports("(display: none)and ( -webkit-transition: all 1s )")');
shouldBeFalse('CSS.supports("(display: none)or(-webkit-transition: all 1s)")');
shouldBeFalse('CSS.supports("(display: none) or(-webkit-transition: all 1s )")');
shouldBeTrue('CSS.supports("(((((((display: none)))))))")');
......
......@@ -25,7 +25,7 @@ PASS getComputedStyle(document.getElementById('t19')).content is "\"APPLIED\""
PASS getComputedStyle(document.getElementById('t20')).content is "\"APPLIED\""
PASS getComputedStyle(document.getElementById('t21')).content is "\"APPLIED\""
PASS getComputedStyle(document.getElementById('t22')).content is "\"UNTOUCHED\""
PASS getComputedStyle(document.getElementById('t23')).content is "\"UNTOUCHED\""
PASS getComputedStyle(document.getElementById('t23')).content is "\"APPLIED\""
PASS getComputedStyle(document.getElementById('t24')).content is "\"UNTOUCHED\""
PASS getComputedStyle(document.getElementById('t25')).content is "\"UNTOUCHED\""
PASS getComputedStyle(document.getElementById('t26')).content is "\"APPLIED\""
......
......@@ -90,7 +90,7 @@
}
@supports (display: none)and ( -webkit-transition: all 1s ) {
#t23 { content: "FAIL" }
#t23 { content: "APPLIED" }
}
@supports (display: none)or(-webkit-transition: all 1s) {
......@@ -243,7 +243,7 @@
<script>
description("Test the @supports rule.");
var numTests = 48;
var untouchedTests = [1, 3, 5, 8, 12, 13, 14, 18, 22, 23, 24, 25, 28, 29, 34, 36, 43, 44, 45, 46]; // Tests whose content shouldn't change from the UNTOUCHED default.
var untouchedTests = [1, 3, 5, 8, 12, 13, 14, 18, 22, 24, 25, 28, 29, 34, 36, 43, 44, 45, 46]; // Tests whose content shouldn't change from the UNTOUCHED default.
var container = document.getElementById("test_container");
for (var i=0; i < numTests; i++) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment