Skip to content
Permalink
Browse files
[@Property] Fallback that does not match syntax should make var() inv…
…alid

https://bugs.webkit.org/show_bug.cgi?id=249981
rdar://103799338

Reviewed by Sam Weinig.

The fallback value must match the syntax definition of the custom property being referenced,
otherwise the declaration is invalid at computed-value time.

https://drafts.css-houdini.org/css-properties-values-api/#fallbacks-in-var-references

* LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/var-reference-registered-properties-expected.txt:
* Source/WebCore/css/CSSVariableReferenceValue.cpp:
(WebCore::CSSVariableReferenceValue::resolveVariableFallback const):

Check the syntax of the fallback.

(WebCore::CSSVariableReferenceValue::resolveVariableReference const):
(WebCore::CSSVariableReferenceValue::resolveTokenRange const):
(WebCore::CSSVariableReferenceValue::resolveVariableReferences const):
(WebCore::CSSVariableReferenceValue::resolveVariableFallback): Deleted.
(WebCore::CSSVariableReferenceValue::resolveVariableReference): Deleted.
(WebCore::CSSVariableReferenceValue::resolveTokenRange): Deleted.

Make these non-static (so parser context is available) and did some return value cleanup.

* Source/WebCore/css/CSSVariableReferenceValue.h:
* Source/WebCore/css/parser/CSSPropertyParser.cpp:
(WebCore::CSSPropertyParser::isValidCustomPropertyValueForSyntax):
* Source/WebCore/css/parser/CSSPropertyParser.h:

Canonical link: https://commits.webkit.org/258373@main
  • Loading branch information
anttijk committed Jan 1, 2023
1 parent 992e7fe commit 6dabb8f934a3f64c639eb12d8c24ddea1c16101e
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 24 deletions.
@@ -10,7 +10,7 @@ FAIL Values are absolutized when substituting into properties with universal syn
PASS Valid fallback does not invalidate var()-reference [<length>, 10px]
PASS Valid fallback does not invalidate var()-reference [<length> | <color>, red]
PASS Valid fallback does not invalidate var()-reference [<length> | none, none]
FAIL Invalid fallback invalidates var()-reference [<length>, red] assert_equals: expected "" but got "40px"
FAIL Invalid fallback invalidates var()-reference [<length> | none, nolength] assert_equals: expected "" but got "40px"
FAIL Invalid fallback invalidates var()-reference [<length>, var(--novar)] assert_equals: expected "" but got "40px"
PASS Invalid fallback invalidates var()-reference [<length>, red]
PASS Invalid fallback invalidates var()-reference [<length> | none, nolength]
PASS Invalid fallback invalidates var()-reference [<length>, var(--novar)]

@@ -30,6 +30,7 @@
#include "config.h"
#include "CSSVariableReferenceValue.h"

#include "CSSPropertyParser.h"
#include "CSSRegisteredCustomProperty.h"
#include "CSSVariableData.h"
#include "ConstantPropertyMap.h"
@@ -69,38 +70,55 @@ String CSSVariableReferenceValue::customCSSText() const
return m_stringValue;
}

bool CSSVariableReferenceValue::resolveVariableFallback(CSSParserTokenRange range, Vector<CSSParserToken>& tokens, Style::BuilderState& builderState)
auto CSSVariableReferenceValue::resolveVariableFallback(const AtomString& variableName, CSSParserTokenRange range, Style::BuilderState& builderState) const -> std::pair<FallbackResult, Vector<CSSParserToken>>
{
ASSERT(range.atEnd() || range.peek().type() == CommaToken);

if (range.atEnd())
return false;
ASSERT(range.peek().type() == CommaToken);
return { FallbackResult::None, { } };

range.consumeIncludingWhitespace();
return resolveTokenRange(range, tokens, builderState);

auto tokens = resolveTokenRange(range, builderState);

auto* registered = builderState.document().customPropertyRegistry().get(variableName);
if (!registered || registered->syntax.isUniversal()) {
if (!tokens)
return { FallbackResult::None, { } };

return { FallbackResult::Valid, WTFMove(*tokens) };
}

// https://drafts.css-houdini.org/css-properties-values-api/#fallbacks-in-var-references
// The fallback value must match the syntax definition of the custom property being referenced,
// otherwise the declaration is invalid at computed-value time
if (!tokens || !CSSPropertyParser::isValidCustomPropertyValueForSyntax(registered->syntax, *tokens, m_context))
return { FallbackResult::Invalid, { } };

return { FallbackResult::Valid, WTFMove(*tokens) };
}

bool CSSVariableReferenceValue::resolveVariableReference(CSSParserTokenRange range, CSSValueID functionId, Vector<CSSParserToken>& tokens, Style::BuilderState& builderState)
bool CSSVariableReferenceValue::resolveVariableReference(CSSParserTokenRange range, CSSValueID functionId, Vector<CSSParserToken>& tokens, Style::BuilderState& builderState) const
{
ASSERT(functionId == CSSValueVar || functionId == CSSValueEnv);

auto& style = builderState.style();

range.consumeWhitespace();
ASSERT(range.peek().type() == IdentToken);
auto variableName = range.consumeIncludingWhitespace().value().toAtomString();
ASSERT(range.atEnd() || (range.peek().type() == CommaToken));

// Apply this variable first, in case it is still unresolved
builderState.builder().applyCustomProperty(variableName);

// Apply fallback to detect cycles
Vector<CSSParserToken> fallbackTokens;
bool fallbackSuccess = resolveVariableFallback(CSSParserTokenRange(range), fallbackTokens, builderState);
// Fallback has to be resolved even when not used to detect cycles and invalid syntax.
auto [fallbackResult, fallbackTokens] = resolveVariableFallback(variableName, range, builderState);
if (fallbackResult == FallbackResult::Invalid)
return false;

auto* property = [&]() -> const CSSCustomPropertyValue* {
if (functionId == CSSValueEnv)
return builderState.document().constantProperties().values().get(variableName);

auto* customProperty = style.getCustomProperty(variableName);
auto* customProperty = builderState.style().getCustomProperty(variableName);
if (customProperty)
return customProperty;

@@ -112,7 +130,7 @@ bool CSSVariableReferenceValue::resolveVariableReference(CSSParserTokenRange ran
if (fallbackTokens.size() > maxSubstitutionTokens)
return false;

if (fallbackSuccess) {
if (fallbackResult == FallbackResult::Valid) {
tokens.appendVector(fallbackTokens);
return true;
}
@@ -127,8 +145,9 @@ bool CSSVariableReferenceValue::resolveVariableReference(CSSParserTokenRange ran
return true;
}

bool CSSVariableReferenceValue::resolveTokenRange(CSSParserTokenRange range, Vector<CSSParserToken>& tokens, Style::BuilderState& builderState)
std::optional<Vector<CSSParserToken>> CSSVariableReferenceValue::resolveTokenRange(CSSParserTokenRange range, Style::BuilderState& builderState) const
{
Vector<CSSParserToken> tokens;
bool success = true;
while (!range.atEnd()) {
auto functionId = range.peek().functionId();
@@ -139,16 +158,19 @@ bool CSSVariableReferenceValue::resolveTokenRange(CSSParserTokenRange range, Vec
}
tokens.append(range.consume());
}
return success;
if (!success)
return { };

return tokens;
}

RefPtr<CSSVariableData> CSSVariableReferenceValue::resolveVariableReferences(Style::BuilderState& builderState) const
{
Vector<CSSParserToken> resolvedTokens;
if (!resolveTokenRange(m_data->tokenRange(), resolvedTokens, builderState))
auto resolvedTokens = resolveTokenRange(m_data->tokenRange(), builderState);
if (!resolvedTokens)
return nullptr;

return CSSVariableData::create(resolvedTokens);
return CSSVariableData::create(*resolvedTokens);
}

} // namespace WebCore
@@ -61,9 +61,10 @@ class CSSVariableReferenceValue : public CSSValue {
private:
explicit CSSVariableReferenceValue(Ref<CSSVariableData>&&, const CSSParserContext&);

static bool resolveTokenRange(CSSParserTokenRange, Vector<CSSParserToken>&, Style::BuilderState&);
static bool resolveVariableReference(CSSParserTokenRange, CSSValueID, Vector<CSSParserToken>&, Style::BuilderState&);
static bool resolveVariableFallback(CSSParserTokenRange, Vector<CSSParserToken>&, Style::BuilderState&);
std::optional<Vector<CSSParserToken>> resolveTokenRange(CSSParserTokenRange, Style::BuilderState&) const;
bool resolveVariableReference(CSSParserTokenRange, CSSValueID, Vector<CSSParserToken>&, Style::BuilderState&) const;
enum class FallbackResult : uint8_t { None, Valid, Invalid };
std::pair<FallbackResult, Vector<CSSParserToken>> resolveVariableFallback(const AtomString& variableName, CSSParserTokenRange, Style::BuilderState&) const;

Ref<CSSVariableData> m_data;
mutable String m_stringValue;
@@ -285,6 +285,15 @@ void CSSPropertyParser::collectParsedCustomPropertyValueDependencies(const CSSCu
parser.collectParsedCustomPropertyValueDependencies(syntax, isRoot, dependencies);
}

bool CSSPropertyParser::isValidCustomPropertyValueForSyntax(const CSSCustomPropertySyntax& syntax, CSSParserTokenRange tokens, const CSSParserContext& context)
{
if (syntax.isUniversal())
return true;

CSSPropertyParser parser { tokens, context, nullptr };
return !!parser.consumeCustomPropertyValueWithSyntax(syntax).first;
}

bool CSSPropertyParser::parseValueStart(CSSPropertyID propertyID, bool important)
{
if (consumeCSSWideKeyword(propertyID, important))
@@ -53,6 +53,7 @@ class CSSPropertyParser {
static RefPtr<CSSCustomPropertyValue> parseTypedCustomPropertyInitialValue(const AtomString&, const CSSCustomPropertySyntax&, CSSParserTokenRange, Style::BuilderState&, const CSSParserContext&);
static RefPtr<CSSCustomPropertyValue> parseTypedCustomPropertyValue(const AtomString& name, const CSSCustomPropertySyntax&, const CSSParserTokenRange&, Style::BuilderState&, const CSSParserContext&);
static void collectParsedCustomPropertyValueDependencies(const CSSCustomPropertySyntax&, bool isRoot, HashSet<CSSPropertyID>& dependencies, const CSSParserTokenRange&, const CSSParserContext&);
static bool isValidCustomPropertyValueForSyntax(const CSSCustomPropertySyntax&, CSSParserTokenRange, const CSSParserContext&);

static RefPtr<CSSValue> parseCounterStyleDescriptor(CSSPropertyID, CSSParserTokenRange&, const CSSParserContext&);

0 comments on commit 6dabb8f

Please sign in to comment.