Skip to content

GitLab

T
tangled
Commit 98ee5fa1 authored by rune@opera.com's avatar rune@opera.com
Browse files
Options

Add inline style in the element's scope. 

Style attribute declarations were added in a separate scope after all
other author origins. Instead add style attribute declarations right after
collecting matching rules from the element's scope. This means that we can
override values set on the style attribute from outer scopes, like we can
with values from inner scope's stylesheet.

Without this fix, you would get green on orange below:

<style>html /deep/ span { color: green; background-color: lime }</style>
<host>
  <host:root>
    <style>span { color:red }</style>
    <span style="background:orange">Green on orange?</span>
  </host:root>
</host>

With this change, we will get green on lime.

The regression (issue 526634) was not relying on this, but it would be
more complex code to fix the regression without fixing the style attribute
cascading order.

R=kochi@chromium.org,hayato@chromium.org
BUG=526634

Review URL: https://codereview.chromium.org/1322753006

git-svn-id: svn://svn.chromium.org/blink/trunk@201620 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent db849a3b
Hide whitespace changes
Inline Side-by-side
Showing with 42 additions and 30 deletions
third_party/WebKit/LayoutTests/fast/css/deep-cascade-order-expected.txt
View file @ 98ee5fa1
...@@ -5,11 +5,9 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE ...@@ -5,11 +5,9 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
PASS getComputedStyle(root1.querySelector('div')).color is green PASS getComputedStyle(root1.querySelector('div')).color is green
TODO(rune@opera.com): Currently fails because style attributes are added to the end, not in its element's scope's cascade order PASS getComputedStyle(root1.querySelector('div + div')).color is green
FAIL getComputedStyle(root1.querySelector('div + div')).color should be rgb(0, 128, 0). Was rgb(255, 0, 0).
PASS getComputedStyle(root2.querySelector('div')).color is green PASS getComputedStyle(root2.querySelector('div')).color is green
TODO(rune@opera.com): Currently fails because style attributes are added to the end, not in its element's scope's cascade order PASS getComputedStyle(root2.querySelector('div + div')).color is green
FAIL getComputedStyle(root2.querySelector('div + div')).color should be rgb(0, 128, 0). Was rgb(255, 0, 0).
PASS successfullyParsed is true PASS successfullyParsed is true
TEST COMPLETE TEST COMPLETE
......
third_party/WebKit/LayoutTests/fast/css/deep-cascade-order.html
View file @ 98ee5fa1
...@@ -18,12 +18,7 @@ ...@@ -18,12 +18,7 @@
var green = "rgb(0, 128, 0)"; var green = "rgb(0, 128, 0)";
shouldBe("getComputedStyle(root1.querySelector('div')).color", "green"); shouldBe("getComputedStyle(root1.querySelector('div')).color", "green");
debug("TODO(rune@opera.com): Currently fails because style attributes are added to the end, not in its element's scope's cascade order");
shouldBe("getComputedStyle(root1.querySelector('div + div')).color", "green"); shouldBe("getComputedStyle(root1.querySelector('div + div')).color", "green");
shouldBe("getComputedStyle(root2.querySelector('div')).color", "green"); shouldBe("getComputedStyle(root2.querySelector('div')).color", "green");
debug("TODO(rune@opera.com): Currently fails because style attributes are added to the end, not in its element's scope's cascade order");
shouldBe("getComputedStyle(root2.querySelector('div + div')).color", "green"); shouldBe("getComputedStyle(root2.querySelector('div + div')).color", "green");
</script> </script>
third_party/WebKit/LayoutTests/fast/css/style-and-stylesheet-important-expected.txt 0 → 100644
View file @ 98ee5fa1
Style attribute !important wins over stylesheet !important
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS getComputedStyle(p).color is "rgb(0, 128, 0)"
PASS successfullyParsed is true
TEST COMPLETE
This text should be green.
third_party/WebKit/LayoutTests/fast/css/style-and-stylesheet-important.html 0 → 100644
View file @ 98ee5fa1
<!DOCTYPE html>
<script src="../../resources/js-test.js"></script>
<style>
#p { color: red !important; }
</style>
<p id="p" style="color: green !important;">This text should be green.</p>
<script>
description("Style attribute !important wins over stylesheet !important");
shouldBeEqualToString("getComputedStyle(p).color", "rgb(0, 128, 0)");
</script>
third_party/WebKit/Source/core/css/ElementRuleCollector.h
View file @ 98ee5fa1
...@@ -124,6 +124,7 @@ public: ...@@ -124,6 +124,7 @@ public:
void addElementStyleProperties(const StylePropertySet*, bool isCacheable = true); void addElementStyleProperties(const StylePropertySet*, bool isCacheable = true);
void finishAddingUARules() { m_result.finishAddingUARules(); } void finishAddingUARules() { m_result.finishAddingUARules(); }
void finishAddingAuthorRulesForTreeScope() { m_result.finishAddingAuthorRulesForTreeScope(); } void finishAddingAuthorRulesForTreeScope() { m_result.finishAddingAuthorRulesForTreeScope(); }
bool isCollectingForPseudoElement() const { return m_pseudoStyleRequest.pseudoId != NOPSEUDO; }
private: private:
template<typename RuleDataListType> template<typename RuleDataListType>
......
third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp
View file @ 98ee5fa1
...@@ -379,12 +379,21 @@ void StyleResolver::matchHostRules(const Element& element, ElementRuleCollector& ...@@ -379,12 +379,21 @@ void StyleResolver::matchHostRules(const Element& element, ElementRuleCollector&
} }
} }
void StyleResolver::matchElementScopeRules(ScopedStyleResolver& elementScopeResolver, ElementRuleCollector& collector, bool includeEmptyRules) void StyleResolver::matchElementScopeRules(const Element& element, ScopedStyleResolver* elementScopeResolver, ElementRuleCollector& collector, bool includeEmptyRules)
{ {
collector.clearMatchedRules(); if (elementScopeResolver) {
elementScopeResolver.collectMatchingAuthorRules(collector, includeEmptyRules); collector.clearMatchedRules();
elementScopeResolver.collectMatchingTreeBoundaryCrossingRules(collector, includeEmptyRules); elementScopeResolver->collectMatchingAuthorRules(collector, includeEmptyRules);
collector.sortAndTransferMatchedRules(); elementScopeResolver->collectMatchingTreeBoundaryCrossingRules(collector, includeEmptyRules);
collector.sortAndTransferMatchedRules();
}
if (element.isStyledElement() && element.inlineStyle() && !collector.isCollectingForPseudoElement()) {
// Inline style is immutable as long as there is no CSSOM wrapper.
bool isInlineStyleCacheable = !element.inlineStyle()->isMutable();
collector.addElementStyleProperties(element.inlineStyle(), isInlineStyleCacheable);
}
collector.finishAddingAuthorRulesForTreeScope(); collector.finishAddingAuthorRulesForTreeScope();
} }
...@@ -397,7 +406,7 @@ void StyleResolver::matchScopedRules(const Element& element, ElementRuleCollecto ...@@ -397,7 +406,7 @@ void StyleResolver::matchScopedRules(const Element& element, ElementRuleCollecto
// scope, only tree-boundary-crossing rules may match. // scope, only tree-boundary-crossing rules may match.
ScopedStyleResolver* elementScopeResolver = scopedResolverFor(element); ScopedStyleResolver* elementScopeResolver = scopedResolverFor(element);
bool matchElementScopeDone = !elementScopeResolver; bool matchElementScopeDone = !elementScopeResolver && !element.inlineStyle();
for (auto it = m_treeBoundaryCrossingScopes.rbegin(); it != m_treeBoundaryCrossingScopes.rend(); ++it) { for (auto it = m_treeBoundaryCrossingScopes.rbegin(); it != m_treeBoundaryCrossingScopes.rend(); ++it) {
const TreeScope& scope = (*it)->treeScope(); const TreeScope& scope = (*it)->treeScope();
...@@ -413,7 +422,7 @@ void StyleResolver::matchScopedRules(const Element& element, ElementRuleCollecto ...@@ -413,7 +422,7 @@ void StyleResolver::matchScopedRules(const Element& element, ElementRuleCollecto
// to a scope which appears before the element's scope in the tree-of-trees order. // to a scope which appears before the element's scope in the tree-of-trees order.
// Try to match all rules from the element's scope. // Try to match all rules from the element's scope.
matchElementScopeRules(*elementScopeResolver, collector, includeEmptyRules); matchElementScopeRules(element, elementScopeResolver, collector, includeEmptyRules);
if (resolver == elementScopeResolver) { if (resolver == elementScopeResolver) {
// Boundary-crossing rules already collected in matchElementScopeRules. // Boundary-crossing rules already collected in matchElementScopeRules.
continue; continue;
...@@ -427,7 +436,7 @@ void StyleResolver::matchScopedRules(const Element& element, ElementRuleCollecto ...@@ -427,7 +436,7 @@ void StyleResolver::matchScopedRules(const Element& element, ElementRuleCollecto
} }
if (!matchElementScopeDone) if (!matchElementScopeDone)
matchElementScopeRules(*elementScopeResolver, collector, includeEmptyRules); matchElementScopeRules(element, elementScopeResolver, collector, includeEmptyRules);
} }
void StyleResolver::matchAuthorRules(const Element& element, ElementRuleCollector& collector, bool includeEmptyRules) void StyleResolver::matchAuthorRules(const Element& element, ElementRuleCollector& collector, bool includeEmptyRules)
...@@ -489,18 +498,6 @@ void StyleResolver::matchAllRules(StyleResolverState& state, ElementRuleCollecto ...@@ -489,18 +498,6 @@ void StyleResolver::matchAllRules(StyleResolverState& state, ElementRuleCollecto
matchAuthorRules(*state.element(), collector, false); matchAuthorRules(*state.element(), collector, false);
if (state.element()->isStyledElement()) { if (state.element()->isStyledElement()) {
// TODO(rune@opera.com): Adding style attribute rules here is probably too late
// when you have shadow piercing combinators. When we don't have piercing combinators,
// the style attribute always belong to the outermost scope whose rules apply to
// the element. Thus, applying inline style here is correct. Fixing this for piercing
// combinators means moving the code below into matchElementScopeRules and _not_
// invoking it for pseudo style requests.
if (state.element()->inlineStyle()) {
// Inline style is immutable as long as there is no CSSOM wrapper.
bool isInlineStyleCacheable = !state.element()->inlineStyle()->isMutable();
collector.addElementStyleProperties(state.element()->inlineStyle(), isInlineStyleCacheable);
}
// Now check SMIL animation override style. // Now check SMIL animation override style.
if (includeSMILProperties && state.element()->isSVGElement()) if (includeSMILProperties && state.element()->isSVGElement())
collector.addElementStyleProperties(toSVGElement(state.element())->animatedSMILStyleProperties(), false /* isCacheable */); collector.addElementStyleProperties(toSVGElement(state.element())->animatedSMILStyleProperties(), false /* isCacheable */);
......
third_party/WebKit/Source/core/css/resolver/StyleResolver.h
View file @ 98ee5fa1
...@@ -200,7 +200,7 @@ private: ...@@ -200,7 +200,7 @@ private:
void matchUARules(ElementRuleCollector&); void matchUARules(ElementRuleCollector&);
void matchAuthorRules(const Element&, ElementRuleCollector&, bool includeEmptyRules); void matchAuthorRules(const Element&, ElementRuleCollector&, bool includeEmptyRules);
void matchHostRules(const Element&, ElementRuleCollector&, bool includeEmptyRules); void matchHostRules(const Element&, ElementRuleCollector&, bool includeEmptyRules);
void matchElementScopeRules(ScopedStyleResolver&, ElementRuleCollector&, bool includeEmptyRules); void matchElementScopeRules(const Element&, ScopedStyleResolver*, ElementRuleCollector&, bool includeEmptyRules);
void matchScopedRules(const Element&, ElementRuleCollector&, bool includeEmptyRules); void matchScopedRules(const Element&, ElementRuleCollector&, bool includeEmptyRules);
void matchAllRules(StyleResolverState&, ElementRuleCollector&, bool includeSMILProperties); void matchAllRules(StyleResolverState&, ElementRuleCollector&, bool includeSMILProperties);
void collectFeatures(); void collectFeatures();
......
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