Commit 113cc2d5 authored by Becca Hughes's avatar Becca Hughes Committed by Commit Bot

[CSS Env Vars] Throw parse error when used with min/max

min() and max() are not supported and should throw a parse
error when used with env() and var().

BUG=864435

Change-Id: I7e89607cc6bb41910131b35a72dabb979f10b7dc
Reviewed-on: https://chromium-review.googlesource.com/1140746
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575871}
parent cf06d026
<!DOCTYPE html>
<html>
<head>
<link rel="help" href="https://drafts.csswg.org/css-env-1/">
<title>Test env() and var() throw syntax errors if used with invalid functions</title>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<style>
div {
--a: 0px;
margin-left: 10px;
}
</style>
</head>
<body>
<script>
// This value is expected if the syntax is valid.
const workingValue = "0px";
// This value is expected if the syntax is invalid.
const pageDefaultValue = "10px";
// This value is expected if the calc() syntax is valid.
const workingCalcValue = "20px";
const testCases = [
{ style: "", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: env(safe-area-inset-left)", expectedPropertyValue: workingValue },
// min and max() are not supported.
{ style: "margin-left: min(env(safe-area-inset-left))", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: min(env(safe-area-inset-left), 1px)", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: min(env(safe-area-inset-left, 1px))", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: min(env(safe-area-inset-left, 1px), 1px)", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: min(env(test))", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: min(env(test), 1px)", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: min(env(test, 1px))", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: min(env(test, 1px), 1px)", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: max(env(safe-area-inset-left))", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: max(env(safe-area-inset-left), 1px)", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: max(env(safe-area-inset-left, 1px))", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: max(env(safe-area-inset-left, 1px), 1px)", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: max(env(test))", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: max(env(test), 1px)", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: max(env(test, 1px))", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: max(env(test, 1px), 1px)", expectedPropertyValue: pageDefaultValue },
// calc() should work.
{ style: "margin-left: calc(env(safe-area-inset-left) + 20px)", expectedPropertyValue: workingCalcValue },
{ style: "margin-left: calc(env(safe-area-inset-left, 0px) + 20px)", expectedPropertyValue: workingCalcValue },
{ style: "margin-left: calc(env(safe-area-inset-left, 0) + 20px)", expectedPropertyValue: workingCalcValue },
{ style: "margin-left: calc(env(test) + 20px)", expectedPropertyValue: "0px" },
{ style: "margin-left: calc(env(test, 1px) + 20px)", expectedPropertyValue: "21px" },
// min and max() are not supported.
{ style: "margin-left: min(var(--a))", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: min(var(--a), 1px)", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: min(var(--a, 1px))", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: min(var(--a, 1px), 1px)", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: min(var(--b))", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: min(var(--b), 1px)", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: min(var(--b, 1px))", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: min(var(--b, 1px), 1px)", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: max(var(--a))", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: max(var(--a), 1px)", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: max(var(--a, 1px))", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: max(var(--a, 1px), 1px)", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: max(var(--b))", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: max(var(--b), 1px)", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: max(var(--b, 1px))", expectedPropertyValue: pageDefaultValue },
{ style: "margin-left: max(var(--b, 1px), 1px)", expectedPropertyValue: pageDefaultValue },
// calc() should work.
{ style: "margin-left: calc(var(--a) + 20px)", expectedPropertyValue: workingCalcValue },
{ style: "margin-left: calc(var(--a, 0px) + 20px)", expectedPropertyValue: workingCalcValue },
{ style: "margin-left: calc(var(--a, 0) + 20px)", expectedPropertyValue: workingCalcValue },
{ style: "margin-left: calc(var(--b) + 20px)", expectedPropertyValue: "0px" },
{ style: "margin-left: calc(var(--b, 1px) + 20px)", expectedPropertyValue: "21px" },
];
testCases.forEach((testcase) => {
test(() => {
const elem = document.createElement("div");
const style = window.getComputedStyle(elem);
document.body.appendChild(elem);
elem.style.cssText = testcase.style;
assert_equals(style.getPropertyValue("margin-left"), testcase.expectedPropertyValue);
}, testcase.style + " " + testcase.expectedPropertyValue);
});
</script>
</body>
</html>
......@@ -13,10 +13,12 @@ namespace blink {
namespace {
bool IsValidVariableReference(CSSParserTokenRange);
bool IsValidEnvVariableReference(CSSParserTokenRange);
bool IsValidVariableReference(CSSParserTokenRange, bool);
bool IsValidEnvVariableReference(CSSParserTokenRange, bool);
bool ClassifyBlock(CSSParserTokenRange range, bool& has_references) {
bool ClassifyBlock(CSSParserTokenRange range,
bool& has_references,
bool skip_variables) {
size_t block_stack_size = 0;
while (!range.AtEnd()) {
......@@ -29,14 +31,15 @@ bool ClassifyBlock(CSSParserTokenRange range, bool& has_references) {
// and used as fallbacks.
switch (token.FunctionId()) {
case CSSValueVar:
if (!IsValidVariableReference(range.ConsumeBlock()))
if (!IsValidVariableReference(range.ConsumeBlock(), skip_variables))
return false; // Invalid reference.
has_references = true;
continue;
case CSSValueEnv:
if (!RuntimeEnabledFeatures::CSSEnvironmentVariablesEnabled())
return false;
if (!IsValidEnvVariableReference(range.ConsumeBlock()))
if (!IsValidEnvVariableReference(range.ConsumeBlock(),
skip_variables))
return false; // Invalid reference.
has_references = true;
continue;
......@@ -47,6 +50,12 @@ bool ClassifyBlock(CSSParserTokenRange range, bool& has_references) {
const CSSParserToken& token = range.Consume();
if (token.GetBlockType() == CSSParserToken::kBlockStart) {
// If we are an invalid function then we should skip over any variables
// this function contains.
if (token.GetType() == CSSParserTokenType::kFunctionToken &&
token.FunctionId() == CSSValueInvalid) {
skip_variables = true;
}
++block_stack_size;
} else if (token.GetBlockType() == CSSParserToken::kBlockEnd) {
--block_stack_size;
......@@ -75,8 +84,10 @@ bool ClassifyBlock(CSSParserTokenRange range, bool& has_references) {
return true;
}
bool IsValidVariableReference(CSSParserTokenRange range) {
bool IsValidVariableReference(CSSParserTokenRange range, bool skip_variables) {
range.ConsumeWhitespace();
if (skip_variables)
return false;
if (!CSSVariableParser::IsValidVariableName(
range.ConsumeIncludingWhitespace()))
return false;
......@@ -89,11 +100,14 @@ bool IsValidVariableReference(CSSParserTokenRange range) {
return false;
bool has_references = false;
return ClassifyBlock(range, has_references);
return ClassifyBlock(range, has_references, skip_variables);
}
bool IsValidEnvVariableReference(CSSParserTokenRange range) {
bool IsValidEnvVariableReference(CSSParserTokenRange range,
bool skip_variables) {
range.ConsumeWhitespace();
if (skip_variables)
return false;
if (range.ConsumeIncludingWhitespace().GetType() !=
CSSParserTokenType::kIdentToken)
return false;
......@@ -106,7 +120,7 @@ bool IsValidEnvVariableReference(CSSParserTokenRange range) {
return false;
bool has_references = false;
return ClassifyBlock(range, has_references);
return ClassifyBlock(range, has_references, skip_variables);
}
CSSValueID ClassifyVariableRange(CSSParserTokenRange range,
......@@ -121,7 +135,7 @@ CSSValueID ClassifyVariableRange(CSSParserTokenRange range,
return id;
}
if (ClassifyBlock(range, has_references))
if (ClassifyBlock(range, has_references, false /* skip_variables */))
return CSSValueInternalVariableValue;
return CSSValueInvalid;
}
......
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