Skip to content

GitLab

T
tangled
Commit a3079e2f authored by alancutter's avatar alancutter Committed by Commit bot
Browse files
Options

Ensure original parser context is used when parsing resolved var() references 

When resolving var() references at style building time we were not
using the CSSParserContext specific to where the var() came from.
This resulted in relative url()s not being able to resolve to an appropriate
absolute URL based on the source base URL.

This change attaches CSSParserContexts to var() references so they can be
used at style resolution time and appropriately resolve relative URLs.

BUG=700445

Review-Url: https://codereview.chromium.org/2873943002
Cr-Commit-Position: refs/heads/master@{#471205}
parent fb001947
Hide whitespace changes
Inline Side-by-side
Showing with 127 additions and 14 deletions
third_party/WebKit/LayoutTests/fast/css/resources/image-url-var.css 0 → 100644
View file @ a3079e2f
.background.image-set.var {
--test: url('test.png');
background: -webkit-image-set(var(--test) 1x);
}
.background.image-set.inline {
background: -webkit-image-set(url('test.png') 1x);
}
.background.url.var {
--test: url('test.png');
background: var(--test);
}
.background.url.inline {
background: url('test.png');
}
.background-image.image-set.var {
--test: url('test.png');
background-image: -webkit-image-set(var(--test) 1x);
}
.background-image.image-set.inline {
background-image: -webkit-image-set(url('test.png') 1x);
}
.background-image.url.var {
--test: url('test.png');
background-image: var(--test);
}
.background-image.url.inline {
background-image: url('test.png');
}
third_party/WebKit/LayoutTests/fast/css/variables/computed-image-url.html 0 → 100644
View file @ a3079e2f
<!DOCTYPE html>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<div id="target"></div>
<script>
function testImageVar(property, value) {
test(() => {
target.style.setProperty('--test', value);
target.style[property] = 'var(--test)';
var actual = getComputedStyle(target)[property];
target.style[property] = value;
assert_not_equals(target.style[property], '', value + ' must be valid for ' + property);
var expected = getComputedStyle(target)[property];
assert_equals(actual, expected);
}, property + ' should resolve ' + value + ' the same whether via var() or not.');
}
testImageVar('background-image', 'url("image.png")');
testImageVar('background-image', '-webkit-image-set(url("image.png") 1x)');
testImageVar('background', 'url("image.png")');
testImageVar('background', '-webkit-image-set(url("image.png") 1x)');
</script>
third_party/WebKit/LayoutTests/fast/css/variables/computed-stylesheet-image-url.html 0 → 100644
View file @ a3079e2f
<!DOCTYPE html>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<!-- The linked stylesheet must not be in the same directory as this test file. -->
<!-- This is intended to test that relative URLs in stylesheets remain relative -->
<!-- to the stylesheet's directory rather than the page that uses it. -->
<link href="../resources/image-url-var.css" rel="stylesheet">
<div id="target"></div>
<script>
for (var property of ['background', 'background-image']) {
for (var value of ['image-set', 'url']) {
test(() => {
target.classList.remove(...target.classList);
assert_equals(target.classList.length, 0);
var initial = getComputedStyle(target)[property];
target.classList.add(property);
target.classList.add(value);
target.classList.add('var');
var actual = getComputedStyle(target)[property];
target.classList.remove('var');
target.classList.add('inline');
var expected = getComputedStyle(target)[property];
assert_not_equals(expected, initial);
assert_equals(actual, expected);
}, property + ' should resolve ' + value + ' the same whether via var() or not.');
}
}
</script>
third_party/WebKit/Source/core/css/CSSSyntaxDescriptor.cpp
View file @ a3079e2f
......@@ -203,7 +203,7 @@ const CSSValue* CSSSyntaxDescriptor::Parse(CSSParserTokenRange range,
bool is_animation_tainted) const {
if (IsTokenStream()) {
return CSSVariableParser::ParseRegisteredPropertyValue(
range, false, is_animation_tainted);
range, *context, false, is_animation_tainted);
}
range.ConsumeWhitespace();
for (const CSSSyntaxComponent& component : syntax_components_) {
......@@ -211,7 +211,7 @@ const CSSValue* CSSSyntaxDescriptor::Parse(CSSParserTokenRange range,
ConsumeSyntaxComponent(component, range, context))
return result;
}
return CSSVariableParser::ParseRegisteredPropertyValue(range, true,
return CSSVariableParser::ParseRegisteredPropertyValue(range, *context, true,
is_animation_tainted);
}
......
third_party/WebKit/Source/core/css/CSSVariableReferenceValue.cpp
View file @ a3079e2f
......@@ -8,6 +8,7 @@ namespace blink {
DEFINE_TRACE_AFTER_DISPATCH(CSSVariableReferenceValue) {
CSSValue::TraceAfterDispatch(visitor);
visitor->Trace(parser_context_);
}
String CSSVariableReferenceValue::CustomCSSText() const {
......
third_party/WebKit/Source/core/css/CSSVariableReferenceValue.h
View file @ a3079e2f
......@@ -7,17 +7,22 @@
#include "core/css/CSSValue.h"
#include "core/css/CSSVariableData.h"
#include "core/css/parser/CSSParserContext.h"
#include "platform/wtf/RefPtr.h"
namespace blink {
class CSSVariableReferenceValue : public CSSValue {
public:
static CSSVariableReferenceValue* Create(PassRefPtr<CSSVariableData> data) {
return new CSSVariableReferenceValue(std::move(data));
static CSSVariableReferenceValue* Create(PassRefPtr<CSSVariableData> data,
const CSSParserContext& context) {
return new CSSVariableReferenceValue(std::move(data), context);
}
CSSVariableData* VariableDataValue() const { return data_.Get(); }
const CSSParserContext* ParserContext() const {
return parser_context_.Get();
}
bool Equals(const CSSVariableReferenceValue& other) const {
return data_ == other.data_;
......@@ -27,10 +32,16 @@ class CSSVariableReferenceValue : public CSSValue {
DECLARE_TRACE_AFTER_DISPATCH();
private:
CSSVariableReferenceValue(PassRefPtr<CSSVariableData> data)
: CSSValue(kVariableReferenceClass), data_(std::move(data)) {}
CSSVariableReferenceValue(PassRefPtr<CSSVariableData> data,
const CSSParserContext& context)
: CSSValue(kVariableReferenceClass),
data_(std::move(data)),
parser_context_(context) {
DCHECK(parser_context_);
}
RefPtr<CSSVariableData> data_;
Member<const CSSParserContext> parser_context_;
};
DEFINE_CSS_VALUE_TYPE_CASTS(CSSVariableReferenceValue,
......
third_party/WebKit/Source/core/css/cssom/CSSUnparsedValue.cpp
View file @ a3079e2f
......@@ -87,9 +87,13 @@ CSSValue* CSSUnparsedValue::ToCSSValue() const {
}
CSSTokenizer tokenizer(tokens.ToString());
return CSSVariableReferenceValue::Create(CSSVariableData::Create(
tokenizer.TokenRange(), false /* isAnimationTainted */,
true /* needsVariableResolution */));
// TODO(alancutter): This should be using a real parser context instead of
// StrictCSSParserContext.
return CSSVariableReferenceValue::Create(
CSSVariableData::Create(tokenizer.TokenRange(),
false /* isAnimationTainted */,
true /* needsVariableResolution */),
*StrictCSSParserContext());
}
} // namespace blink
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
View file @ a3079e2f
......@@ -172,7 +172,8 @@ bool CSSPropertyParser::ParseValueStart(CSSPropertyID unresolved_property,
if (CSSVariableParser::ContainsValidVariableReferences(original_range)) {
bool is_animation_tainted = false;
CSSVariableReferenceValue* variable = CSSVariableReferenceValue::Create(
CSSVariableData::Create(original_range, is_animation_tainted, true));
CSSVariableData::Create(original_range, is_animation_tainted, true),
*context_);
if (is_shorthand) {
const CSSPendingSubstitutionValue& pending_value =
......
third_party/WebKit/Source/core/css/parser/CSSVariableParser.cpp
View file @ a3079e2f
......@@ -154,6 +154,7 @@ CSSCustomPropertyDeclaration* CSSVariableParser::ParseDeclarationValue(
CSSVariableReferenceValue* CSSVariableParser::ParseRegisteredPropertyValue(
CSSParserTokenRange range,
const CSSParserContext& context,
bool require_var_reference,
bool is_animation_tainted) {
if (range.AtEnd())
......@@ -170,7 +171,8 @@ CSSVariableReferenceValue* CSSVariableParser::ParseRegisteredPropertyValue(
return nullptr;
// TODO(timloh): Should this be hasReferences || hasAtApplyRule?
return CSSVariableReferenceValue::Create(
CSSVariableData::Create(range, is_animation_tainted, has_references));
CSSVariableData::Create(range, is_animation_tainted, has_references),
context);
}
} // namespace blink
third_party/WebKit/Source/core/css/parser/CSSVariableParser.h
View file @ a3079e2f
......@@ -14,6 +14,7 @@
namespace blink {
class CSSCustomPropertyDeclaration;
class CSSParserContext;
class CSSVariableReferenceValue;
class CORE_EXPORT CSSVariableParser {
......@@ -26,6 +27,7 @@ class CORE_EXPORT CSSVariableParser {
bool is_animation_tainted);
static CSSVariableReferenceValue* ParseRegisteredPropertyValue(
CSSParserTokenRange,
const CSSParserContext&,
bool require_var_reference,
bool is_animation_tainted);
......
third_party/WebKit/Source/core/css/resolver/CSSVariableResolver.cpp
View file @ a3079e2f
......@@ -229,7 +229,7 @@ const CSSValue* CSSVariableResolver::ResolveVariableReferences(
is_animation_tainted))
return CSSUnsetValue::Create();
const CSSValue* result =
CSSPropertyParser::ParseSingleValue(id, tokens, StrictCSSParserContext());
CSSPropertyParser::ParseSingleValue(id, tokens, value.ParserContext());
if (!result)
return CSSUnsetValue::Create();
return result;
......@@ -258,13 +258,13 @@ const CSSValue* CSSVariableResolver::ResolvePendingSubstitutions(
if (resolver.ResolveTokenRange(
shorthand_value->VariableDataValue()->Tokens(),
disallow_animation_tainted, tokens, is_animation_tainted)) {
CSSParserContext* context = CSSParserContext::Create(kHTMLStandardMode);
HeapVector<CSSProperty, 256> parsed_properties;
if (CSSPropertyParser::ParseValue(
shorthand_property_id, false, CSSParserTokenRange(tokens),
context, parsed_properties, StyleRule::RuleType::kStyle)) {
shorthand_value->ParserContext(), parsed_properties,
StyleRule::RuleType::kStyle)) {
unsigned parsed_properties_count = parsed_properties.size();
for (unsigned i = 0; i < parsed_properties_count; ++i) {
property_cache.Set(parsed_properties[i].Id(),
......
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