Skip to content

GitLab

T
tangled
Commit 11ad0175 authored by kochi@chromium.org's avatar kochi@chromium.org
Browse files
Options

Revert of Add inline style in the element's scope. (patchset #1 id:1 of... 

Revert of Add inline style in the element's scope. (patchset #1 id:1 of https://codereview.chromium.org/1322753006/ )

Reason for revert:
This caused regression.
https://code.google.com/p/chromium/issues/detail?id=528370


Original issue's description:
> 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
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201620

TBR=hayato@chromium.org,rune@opera.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=526634

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

git-svn-id: svn://svn.chromium.org/blink/trunk@201842 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent d0389c94
Hide whitespace changes
Inline Side-by-side
Showing with 30 additions and 42 deletions
third_party/WebKit/LayoutTests/fast/css/deep-cascade-order-expected.txt
View file @ 11ad0175
...@@ -5,9 +5,11 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE ...@@ -5,9 +5,11 @@ 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
PASS getComputedStyle(root1.querySelector('div + 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
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
PASS getComputedStyle(root2.querySelector('div + 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
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 @ 11ad0175
...@@ -18,7 +18,12 @@ ...@@ -18,7 +18,12 @@
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 deleted 100644 → 0
View file @ d0389c94
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 deleted 100644 → 0
View file @ d0389c94
<!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 @ 11ad0175
...@@ -124,7 +124,6 @@ public: ...@@ -124,7 +124,6 @@ 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 @ 11ad0175
...@@ -379,21 +379,12 @@ void StyleResolver::matchHostRules(const Element& element, ElementRuleCollector& ...@@ -379,21 +379,12 @@ void StyleResolver::matchHostRules(const Element& element, ElementRuleCollector&
} }
} }
void StyleResolver::matchElementScopeRules(const Element& element, ScopedStyleResolver* elementScopeResolver, ElementRuleCollector& collector, bool includeEmptyRules) void StyleResolver::matchElementScopeRules(ScopedStyleResolver& elementScopeResolver, ElementRuleCollector& collector, bool includeEmptyRules)
{ {
if (elementScopeResolver) { collector.clearMatchedRules();
collector.clearMatchedRules(); elementScopeResolver.collectMatchingAuthorRules(collector, includeEmptyRules);
elementScopeResolver->collectMatchingAuthorRules(collector, includeEmptyRules); elementScopeResolver.collectMatchingTreeBoundaryCrossingRules(collector, includeEmptyRules);
elementScopeResolver->collectMatchingTreeBoundaryCrossingRules(collector, includeEmptyRules); collector.sortAndTransferMatchedRules();
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();
} }
...@@ -406,7 +397,7 @@ void StyleResolver::matchScopedRules(const Element& element, ElementRuleCollecto ...@@ -406,7 +397,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 && !element.inlineStyle(); bool matchElementScopeDone = !elementScopeResolver;
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();
...@@ -422,7 +413,7 @@ void StyleResolver::matchScopedRules(const Element& element, ElementRuleCollecto ...@@ -422,7 +413,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(element, elementScopeResolver, collector, includeEmptyRules); matchElementScopeRules(*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;
...@@ -436,7 +427,7 @@ void StyleResolver::matchScopedRules(const Element& element, ElementRuleCollecto ...@@ -436,7 +427,7 @@ void StyleResolver::matchScopedRules(const Element& element, ElementRuleCollecto
} }
if (!matchElementScopeDone) if (!matchElementScopeDone)
matchElementScopeRules(element, elementScopeResolver, collector, includeEmptyRules); matchElementScopeRules(*elementScopeResolver, collector, includeEmptyRules);
} }
void StyleResolver::matchAuthorRules(const Element& element, ElementRuleCollector& collector, bool includeEmptyRules) void StyleResolver::matchAuthorRules(const Element& element, ElementRuleCollector& collector, bool includeEmptyRules)
...@@ -498,6 +489,18 @@ void StyleResolver::matchAllRules(StyleResolverState& state, ElementRuleCollecto ...@@ -498,6 +489,18 @@ 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 @ 11ad0175
...@@ -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(const Element&, ScopedStyleResolver*, ElementRuleCollector&, bool includeEmptyRules); void matchElementScopeRules(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