Commit a44ebf4c authored by timloh's avatar timloh Committed by Commit bot

CSS Properties and Values API: Use initial value where appropriate for var()

This patch fixes var() references to registered properties to return
initial values where appropriate. When a registered property is not
explicitly set, var() references to it should result in that property's
initial value. We now store the initial value as a token stream as well
to accommodate this.

Any unregistered property in a var() cycle will continue to compute to
their initial value (i.e. the invalid value). Any registered property
in a var() cycle similar compute to their initial value (I don't think
this is explicitly mentioned in the spec, but for consistency this makes
more sense than the unset value). Valid references to properties in a
var() cycle will resolve to the referenced property'd initial value.

BUG=641877

Review-Url: https://codereview.chromium.org/2358203003
Cr-Commit-Position: refs/heads/master@{#420579}
parent 96a67c55
<!DOCTYPE HTML>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<style>
#test1 {
--registered-1-a: var(--registered-1-b, 10px);
--registered-1-b: var(--registered-1-a, 20px);
--registered-1-c: var(--registered-1-b, 30px);
--registered-1-d: var(--registered-1-b);
--unregistered-1-a:var(--registered-1-a,40px);
--unregistered-1-a:var(--registered-1-a);
left: var(--registered-1-a, 50px);
top: var(--registered-1-b, 60px);
}
</style>
<div id=test1></div>
<script>
test(function() {
CSS.registerProperty({name: '--registered-1-a', syntax: '<length>', initialValue: '1px'});
CSS.registerProperty({name: '--registered-1-b', syntax: '<length>', initialValue: '2px'});
CSS.registerProperty({name: '--registered-1-c', syntax: '<length>', initialValue: '3px'});
CSS.registerProperty({name: '--registered-1-d', syntax: '<length>', initialValue: '4px'});
computedStyle = getComputedStyle(test1);
assert_equals(computedStyle.getPropertyValue('--registered-1-a'), '1px');
assert_equals(computedStyle.getPropertyValue('--registered-1-b'), '2px');
assert_equals(computedStyle.getPropertyValue('--registered-1-c'), '2px');
assert_equals(computedStyle.getPropertyValue('--registered-1-d'), '2px');
assert_equals(computedStyle.getPropertyValue('--unregistered-1-a'), '1px');
assert_equals(computedStyle.left, '1px');
assert_equals(computedStyle.top, '2px');
}, "A var() cycle between two registered properties is handled correctly.");
</script>
<style>
#test2 {
--registered-2-a: var(--unregistered-2-a, 10px);
--unregistered-2-a:var(--registered-2-a,20px);
--registered-2-b: var(--registered-2-a, 30px);
--registered-2-c: var(--registered-2-a);
--registered-2-d: var(--unregistered-2-a, 40px);
--registered-2-e: var(--unregistered-2-a);
--unregistered-2-b:var(--registered-2-a,50px);
--unregistered-2-c:var(--registered-2-a);
--unregistered-2-d:var(--unregistered-2-a,60px);
--unregistered-2-e:var(--unregistered-2-a);
left: var(--registered-2-a, 70px);
top: var(--unregistered-2-a, 80px);
}
</style>
<div id=test2></div>
<script>
test(function() {
CSS.registerProperty({name: '--registered-2-a', syntax: '<length>', initialValue: '1px'});
CSS.registerProperty({name: '--registered-2-b', syntax: '<length>', initialValue: '2px'});
CSS.registerProperty({name: '--registered-2-c', syntax: '<length>', initialValue: '3px'});
CSS.registerProperty({name: '--registered-2-d', syntax: '<length>', initialValue: '4px'});
CSS.registerProperty({name: '--registered-2-e', syntax: '<length>', initialValue: '5px'});
computedStyle = getComputedStyle(test2);
assert_equals(computedStyle.getPropertyValue('--registered-2-a'), '1px');
assert_equals(computedStyle.getPropertyValue('--unregistered-2-a'), '');
assert_equals(computedStyle.getPropertyValue('--registered-2-b'), '1px');
assert_equals(computedStyle.getPropertyValue('--registered-2-c'), '1px');
assert_equals(computedStyle.getPropertyValue('--registered-2-d'), '40px');
assert_equals(computedStyle.getPropertyValue('--registered-2-e'), '5px');
assert_equals(computedStyle.getPropertyValue('--unregistered-2-b'), '1px');
assert_equals(computedStyle.getPropertyValue('--unregistered-2-c'), '1px');
assert_equals(computedStyle.getPropertyValue('--unregistered-2-d'), '60px');
assert_equals(computedStyle.getPropertyValue('--unregistered-2-e'), '');
assert_equals(computedStyle.left, '1px');
assert_equals(computedStyle.top, '80px');
}, "A var() cycle between a registered properties and an unregistered property is handled correctly.");
</script>
<style>
#test3 {
--unregistered-3-a:var(--unregistered-3-b,10px);
--unregistered-3-b:var(--unregistered-3-a,20px);
--registered-3-a: var(--unregistered-3-a, 30px);
--registered-3-b: var(--unregistered-3-a);
--registered-3-c: var(--unregistered-3-b, 40px);
--registered-3-d: var(--registered-3-c, 50px);
left: var(--registered-3-d, 60px);
top: var(--registered-3-b, 70px);
}
</style>
<div id=test3></div>
<script>
test(function() {
CSS.registerProperty({name: '--registered-3-a', syntax: '<length>', initialValue: '1px'});
CSS.registerProperty({name: '--registered-3-b', syntax: '<length>', initialValue: '2px'});
CSS.registerProperty({name: '--registered-3-c', syntax: '<length>', initialValue: '3px'});
CSS.registerProperty({name: '--registered-3-d', syntax: '<length>', initialValue: '4px'});
computedStyle = getComputedStyle(test3);
assert_equals(computedStyle.getPropertyValue('--unregistered-3-a'), '');
assert_equals(computedStyle.getPropertyValue('--unregistered-3-b'), '');
assert_equals(computedStyle.getPropertyValue('--registered-3-a'), '30px');
assert_equals(computedStyle.getPropertyValue('--registered-3-b'), '2px');
assert_equals(computedStyle.getPropertyValue('--registered-3-c'), '40px');
assert_equals(computedStyle.getPropertyValue('--registered-3-d'), '40px');
assert_equals(computedStyle.left, '40px');
assert_equals(computedStyle.top, '2px');
}, "A var() cycle between a two unregistered properties is handled correctly.");
</script>
<style>
#test4 {
--registered-4-a:var(--unregistered-4-a,hello);
--unregistered-4-a:var(--registered-4-a,world);
--registered-4-b:var(--unregistered-4-a,meow);
--registered-4-c:var(--unregistered-4-a);
--unregistered-4-b:var(--unregistered-4-a,woof);
--unregistered-4-c:var(--unregistered-4-a);
transition-property: var(--registered-4-a, water);
}
</style>
<div id=test4></div>
<script>
test(function() {
CSS.registerProperty({name: '--registered-4-a', syntax: '*'});
CSS.registerProperty({name: '--registered-4-b', syntax: '*', initialValue: 'moo'});
CSS.registerProperty({name: '--registered-4-c', syntax: '*', initialValue: 'circle'});
computedStyle = getComputedStyle(test4);
assert_equals(computedStyle.getPropertyValue('--registered-4-a'), '');
assert_equals(computedStyle.getPropertyValue('--unregistered-4-a'), '');
assert_equals(computedStyle.getPropertyValue('--registered-4-b'), 'meow');
assert_equals(computedStyle.getPropertyValue('--registered-4-c'), 'circle');
assert_equals(computedStyle.getPropertyValue('--unregistered-4-b'), 'woof');
assert_equals(computedStyle.getPropertyValue('--unregistered-4-c'), '');
assert_equals(computedStyle.transitionProperty, 'water');
}, "A var() cycle between a syntax:'*' property and an unregistered property is handled correctly.");
</script>
...@@ -9,24 +9,36 @@ div { ...@@ -9,24 +9,36 @@ div {
--registered-length-4: calc(var(--length-1) + 40px); --registered-length-4: calc(var(--length-1) + 40px);
--registered-length-5: var(--invalid, 70px); --registered-length-5: var(--invalid, 70px);
--registered-length-6: calc(var(--registered-length-3)*4); --registered-length-6: calc(var(--registered-length-3)*4);
--registered-length-7: var(--123px, 6px);
--length-1: 20px; --length-1: 20px;
--length-2: var(--registered-length-1); --length-2: var(--registered-length-1);
--length-3: calc(var(--123px, 6px) + var(--123px));
--percentage: 10%; --percentage: 10%;
--registered-length-invalid: var(--percentage); --registered-length-invalid: var(--percentage);
--registered-token-stream-1:var(--invalid);
--registered-token-stream-2:var(--invalid,fallback);
--token-stream-1:var(--registered-token-stream-1,moo);
} }
</style> </style>
<div id=element></div> <div id=element></div>
<script> <script>
CSS.registerProperty({name: '--123px', syntax: '<length>', initialValue: '123px'});
CSS.registerProperty({name: '--registered-length-1', syntax: '<length>', initialValue: '0px'}); CSS.registerProperty({name: '--registered-length-1', syntax: '<length>', initialValue: '0px'});
CSS.registerProperty({name: '--registered-length-2', syntax: '<length>', initialValue: '0px'}); CSS.registerProperty({name: '--registered-length-2', syntax: '<length>', initialValue: '0px'});
CSS.registerProperty({name: '--registered-length-3', syntax: '<length>', initialValue: '0px'}); CSS.registerProperty({name: '--registered-length-3', syntax: '<length>', initialValue: '0px'});
CSS.registerProperty({name: '--registered-length-4', syntax: '<length>', initialValue: '0px'}); CSS.registerProperty({name: '--registered-length-4', syntax: '<length>', initialValue: '0px'});
CSS.registerProperty({name: '--registered-length-5', syntax: '<length>', initialValue: '0px'}); CSS.registerProperty({name: '--registered-length-5', syntax: '<length>', initialValue: '0px'});
CSS.registerProperty({name: '--registered-length-6', syntax: '<length>', initialValue: '0px'}); CSS.registerProperty({name: '--registered-length-6', syntax: '<length>', initialValue: '0px'});
CSS.registerProperty({name: '--registered-length-7', syntax: '<length>', initialValue: '0px'});
CSS.registerProperty({name: '--registered-length-invalid', syntax: '<length>', initialValue: '15px'}); CSS.registerProperty({name: '--registered-length-invalid', syntax: '<length>', initialValue: '15px'});
CSS.registerProperty({name: '--registered-token-stream-1', syntax: '*'});
CSS.registerProperty({name: '--registered-token-stream-2', syntax: '*'});
test(function() { test(function() {
computedStyle = getComputedStyle(element); computedStyle = getComputedStyle(element);
assert_equals(computedStyle.getPropertyValue('--registered-length-1'), '10px'); assert_equals(computedStyle.getPropertyValue('--registered-length-1'), '10px');
...@@ -35,8 +47,14 @@ test(function() { ...@@ -35,8 +47,14 @@ test(function() {
assert_equals(computedStyle.getPropertyValue('--registered-length-4'), '60px'); assert_equals(computedStyle.getPropertyValue('--registered-length-4'), '60px');
assert_equals(computedStyle.getPropertyValue('--registered-length-5'), '70px'); assert_equals(computedStyle.getPropertyValue('--registered-length-5'), '70px');
assert_equals(computedStyle.getPropertyValue('--registered-length-6'), '80px'); assert_equals(computedStyle.getPropertyValue('--registered-length-6'), '80px');
assert_equals(computedStyle.getPropertyValue('--registered-length-7'), '123px');
assert_equals(computedStyle.getPropertyValue('--length-1'), ' 20px'); assert_equals(computedStyle.getPropertyValue('--length-1'), ' 20px');
assert_equals(computedStyle.getPropertyValue('--length-2'), ' 10px'); assert_equals(computedStyle.getPropertyValue('--length-2'), ' 10px');
assert_equals(computedStyle.getPropertyValue('--length-3'), ' calc(123px + 123px)');
assert_equals(computedStyle.getPropertyValue('--registered-length-invalid'), '15px'); assert_equals(computedStyle.getPropertyValue('--registered-length-invalid'), '15px');
assert_equals(computedStyle.getPropertyValue('--registered-token-stream-1'), '');
assert_equals(computedStyle.getPropertyValue('--registered-token-stream-2'), 'fallback');
assert_equals(computedStyle.getPropertyValue('--token-stream-1'), 'moo');
}, "var() references work with registered properties"); }, "var() references work with registered properties");
</script> </script>
...@@ -10,7 +10,6 @@ ...@@ -10,7 +10,6 @@
#include "core/css/CSSVariableReferenceValue.h" #include "core/css/CSSVariableReferenceValue.h"
#include "core/css/parser/CSSParserIdioms.h" #include "core/css/parser/CSSParserIdioms.h"
#include "core/css/parser/CSSPropertyParserHelpers.h" #include "core/css/parser/CSSPropertyParserHelpers.h"
#include "core/css/parser/CSSTokenizer.h"
#include "core/css/parser/CSSVariableParser.h" #include "core/css/parser/CSSVariableParser.h"
#include "core/html/parser/HTMLParserIdioms.h" #include "core/html/parser/HTMLParserIdioms.h"
...@@ -196,12 +195,6 @@ const CSSValue* consumeSyntaxComponent(const CSSSyntaxComponent& syntax, CSSPars ...@@ -196,12 +195,6 @@ const CSSValue* consumeSyntaxComponent(const CSSSyntaxComponent& syntax, CSSPars
return result; return result;
} }
const CSSValue* CSSSyntaxDescriptor::parse(const String& value) const
{
CSSTokenizer::Scope scope(value);
return parse(scope.tokenRange());
}
const CSSValue* CSSSyntaxDescriptor::parse(CSSParserTokenRange range) const const CSSValue* CSSSyntaxDescriptor::parse(CSSParserTokenRange range) const
{ {
if (isTokenStream()) if (isTokenStream())
......
...@@ -47,7 +47,6 @@ public: ...@@ -47,7 +47,6 @@ public:
CSSSyntaxDescriptor(String syntax); CSSSyntaxDescriptor(String syntax);
const CSSValue* parse(CSSParserTokenRange) const; const CSSValue* parse(CSSParserTokenRange) const;
const CSSValue* parse(const String&) const;
bool isValid() const { return !m_syntaxComponents.isEmpty(); } bool isValid() const { return !m_syntaxComponents.isEmpty(); }
bool isTokenStream() const { return m_syntaxComponents.size() == 1 && m_syntaxComponents[0].m_type == CSSSyntaxType::TokenStream; } bool isTokenStream() const { return m_syntaxComponents.size() == 1 && m_syntaxComponents[0].m_type == CSSSyntaxType::TokenStream; }
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "core/css/CSSVariableReferenceValue.h" #include "core/css/CSSVariableReferenceValue.h"
#include "core/css/PropertyDescriptor.h" #include "core/css/PropertyDescriptor.h"
#include "core/css/PropertyRegistry.h" #include "core/css/PropertyRegistry.h"
#include "core/css/parser/CSSTokenizer.h"
#include "core/css/parser/CSSVariableParser.h" #include "core/css/parser/CSSVariableParser.h"
#include "core/dom/Document.h" #include "core/dom/Document.h"
#include "core/dom/ExceptionCode.h" #include "core/dom/ExceptionCode.h"
...@@ -79,7 +80,8 @@ void PropertyRegistration::registerProperty(ExecutionContext* executionContext, ...@@ -79,7 +80,8 @@ void PropertyRegistration::registerProperty(ExecutionContext* executionContext,
} }
if (descriptor.hasInitialValue()) { if (descriptor.hasInitialValue()) {
const CSSValue* initial = syntaxDescriptor.parse(descriptor.initialValue()); CSSTokenizer::Scope scope(descriptor.initialValue());
const CSSValue* initial = syntaxDescriptor.parse(scope.tokenRange());
if (!initial) { if (!initial) {
exceptionState.throwDOMException(SyntaxError, "The initial value provided does not parse for the given syntax."); exceptionState.throwDOMException(SyntaxError, "The initial value provided does not parse for the given syntax.");
return; return;
...@@ -88,13 +90,14 @@ void PropertyRegistration::registerProperty(ExecutionContext* executionContext, ...@@ -88,13 +90,14 @@ void PropertyRegistration::registerProperty(ExecutionContext* executionContext,
exceptionState.throwDOMException(SyntaxError, "The initial value provided is not computationally independent."); exceptionState.throwDOMException(SyntaxError, "The initial value provided is not computationally independent.");
return; return;
} }
registry.registerProperty(atomicName, syntaxDescriptor, descriptor.inherits(), initial); RefPtr<CSSVariableData> initialVariableData = CSSVariableData::create(scope.tokenRange(), false);
registry.registerProperty(atomicName, syntaxDescriptor, descriptor.inherits(), initial, initialVariableData.release());
} else { } else {
if (!syntaxDescriptor.isTokenStream()) { if (!syntaxDescriptor.isTokenStream()) {
exceptionState.throwDOMException(SyntaxError, "An initial value must be provided if the syntax is not '*'"); exceptionState.throwDOMException(SyntaxError, "An initial value must be provided if the syntax is not '*'");
return; return;
} }
registry.registerProperty(atomicName, syntaxDescriptor, descriptor.inherits(), nullptr); registry.registerProperty(atomicName, syntaxDescriptor, descriptor.inherits(), nullptr, nullptr);
} }
// TODO(timloh): Invalidate only elements with this custom property set // TODO(timloh): Invalidate only elements with this custom property set
......
...@@ -6,12 +6,12 @@ ...@@ -6,12 +6,12 @@
namespace blink { namespace blink {
void PropertyRegistry::registerProperty(const AtomicString& name, const CSSSyntaxDescriptor& syntax, bool inherits, const CSSValue* initial) void PropertyRegistry::registerProperty(const AtomicString& name, const CSSSyntaxDescriptor& syntax, bool inherits, const CSSValue* initial, PassRefPtr<CSSVariableData> initialVariableData)
{ {
DCHECK(!registration(name)); DCHECK(!registration(name));
// TODO(timloh): We only support inherited properties for now. // TODO(timloh): We only support inherited properties for now.
inherits = true; inherits = true;
m_registrations.set(name, new Registration(syntax, inherits, initial)); m_registrations.set(name, new Registration(syntax, inherits, initial, initialVariableData));
} }
void PropertyRegistry::unregisterProperty(const AtomicString& name) void PropertyRegistry::unregisterProperty(const AtomicString& name)
......
...@@ -24,12 +24,17 @@ public: ...@@ -24,12 +24,17 @@ public:
class Registration : public GarbageCollectedFinalized<Registration> { class Registration : public GarbageCollectedFinalized<Registration> {
public: public:
Registration(const CSSSyntaxDescriptor& syntax, bool inherits, const CSSValue* initial) Registration(const CSSSyntaxDescriptor& syntax, bool inherits, const CSSValue* initial, PassRefPtr<CSSVariableData> initialVariableData)
: m_syntax(syntax), m_inherits(inherits), m_initial(initial) { } : m_syntax(syntax)
, m_inherits(inherits)
, m_initial(initial)
, m_initialVariableData(initialVariableData)
{ }
const CSSSyntaxDescriptor& syntax() const { return m_syntax; } const CSSSyntaxDescriptor& syntax() const { return m_syntax; }
bool inherits() const { return m_inherits; } bool inherits() const { return m_inherits; }
const CSSValue* initial() const { return m_initial; } const CSSValue* initial() const { return m_initial; }
CSSVariableData* initialVariableData() const { return m_initialVariableData.get(); }
DEFINE_INLINE_TRACE() { visitor->trace(m_initial); } DEFINE_INLINE_TRACE() { visitor->trace(m_initial); }
...@@ -37,9 +42,10 @@ public: ...@@ -37,9 +42,10 @@ public:
const CSSSyntaxDescriptor m_syntax; const CSSSyntaxDescriptor m_syntax;
const bool m_inherits; const bool m_inherits;
const Member<const CSSValue> m_initial; const Member<const CSSValue> m_initial;
const RefPtr<CSSVariableData> m_initialVariableData;
}; };
void registerProperty(const AtomicString&, const CSSSyntaxDescriptor&, bool inherits, const CSSValue* initial); void registerProperty(const AtomicString&, const CSSSyntaxDescriptor&, bool inherits, const CSSValue* initial, PassRefPtr<CSSVariableData> initialVariableData);
void unregisterProperty(const AtomicString&); void unregisterProperty(const AtomicString&);
const Registration* registration(const AtomicString&) const; const Registration* registration(const AtomicString&) const;
......
...@@ -35,38 +35,37 @@ bool CSSVariableResolver::resolveFallback(CSSParserTokenRange range, Vector<CSSP ...@@ -35,38 +35,37 @@ bool CSSVariableResolver::resolveFallback(CSSParserTokenRange range, Vector<CSSP
CSSVariableData* CSSVariableResolver::valueForCustomProperty(AtomicString name) CSSVariableData* CSSVariableResolver::valueForCustomProperty(AtomicString name)
{ {
// TODO(timloh): Registered properties shouldn't return nullptr in failure
// cases (aside from cycles?), but instead return the initial/inherited value.
if (m_variablesSeen.contains(name)) { if (m_variablesSeen.contains(name)) {
m_cycleStartPoints.add(name); m_cycleStartPoints.add(name);
return nullptr; return nullptr;
} }
if (!m_styleVariableData) DCHECK(m_registry || !RuntimeEnabledFeatures::cssVariables2Enabled());
return nullptr; const PropertyRegistry::Registration* registration = m_registry ? m_registry->registration(name) : nullptr;
CSSVariableData* variableData = m_styleVariableData->getVariable(name);
CSSVariableData* variableData = nullptr;
if (m_styleVariableData)
variableData = m_styleVariableData->getVariable(name);
if (!variableData) if (!variableData)
return nullptr; return registration ? registration->initialVariableData() : nullptr;
if (!variableData->needsVariableResolution()) if (!variableData->needsVariableResolution())
return variableData; return variableData;
RefPtr<CSSVariableData> newVariableData = resolveCustomProperty(name, *variableData);
DCHECK(m_registry || !RuntimeEnabledFeatures::cssVariables2Enabled()); RefPtr<CSSVariableData> newVariableData = resolveCustomProperty(name, *variableData);
if (m_registry) { if (registration) {
const PropertyRegistry::Registration* registration = m_registry->registration(name); const CSSValue* parsedValue = nullptr;
if (registration) { if (newVariableData) {
const CSSValue* parsedValue = nullptr; parsedValue = newVariableData->parseForSyntax(registration->syntax());
if (newVariableData) { if (parsedValue)
parsedValue = newVariableData->parseForSyntax(registration->syntax()); parsedValue = &StyleBuilderConverter::convertRegisteredPropertyValue(m_styleResolverState, *parsedValue);
if (parsedValue) else
parsedValue = &StyleBuilderConverter::convertRegisteredPropertyValue(m_styleResolverState, *parsedValue); newVariableData = nullptr;
else
newVariableData = nullptr;
}
m_styleVariableData->setVariable(name, newVariableData);
m_styleVariableData->setRegisteredInheritedProperty(name, parsedValue);
return newVariableData.get();
} }
m_styleVariableData->setVariable(name, newVariableData);
m_styleVariableData->setRegisteredInheritedProperty(name, parsedValue);
if (!newVariableData)
return registration->initialVariableData();
return newVariableData.get();
} }
m_styleVariableData->setVariable(name, newVariableData); m_styleVariableData->setVariable(name, newVariableData);
......
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