Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1232)

Issue 1322753006: Add inline style in the element's scope. (Closed)

Created:
5 years, 3 months ago by rune
Modified:
5 years, 3 months ago
Reviewers:
hayato, kochi
CC:
blink-reviews, blink-reviews-style_chromium.org, blink-reviews-css, dglazkov+blink, apavlov+blink_chromium.org, darktears, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

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

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -30 lines) Patch
M LayoutTests/fast/css/deep-cascade-order.html View 1 chunk +0 lines, -5 lines 0 comments Download
M LayoutTests/fast/css/deep-cascade-order-expected.txt View 1 chunk +2 lines, -4 lines 0 comments Download
A LayoutTests/fast/css/style-and-stylesheet-important.html View 1 chunk +11 lines, -0 lines 0 comments Download
A LayoutTests/fast/css/style-and-stylesheet-important-expected.txt View 1 chunk +10 lines, -0 lines 0 comments Download
M Source/core/css/ElementRuleCollector.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/resolver/StyleResolver.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/resolver/StyleResolver.cpp View 5 chunks +17 lines, -20 lines 0 comments Download

Messages

Total messages: 12 (3 generated)
rune
PTAL
5 years, 3 months ago (2015-09-01 15:57:17 UTC) #1
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1322753006/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1322753006/1
5 years, 3 months ago (2015-09-01 15:57:32 UTC) #3
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 3 months ago (2015-09-01 18:56:31 UTC) #5
kochi
lgtm Thanks for the quick fix! nit: in the CL description, style property's {} is ...
5 years, 3 months ago (2015-09-02 06:08:11 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1322753006/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1322753006/1
5 years, 3 months ago (2015-09-02 06:43:33 UTC) #8
rune
On 2015/09/02 06:08:11, Takayoshi Kochi wrote: > nit: in the CL description, style property's {} ...
5 years, 3 months ago (2015-09-02 06:43:43 UTC) #9
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://src.chromium.org/viewvc/blink?view=rev&revision=201620
5 years, 3 months ago (2015-09-02 06:48:41 UTC) #10
kochi
This regressed, see https://code.google.com/p/chromium/issues/detail?id=528370
5 years, 3 months ago (2015-09-05 04:00:38 UTC) #11
kochi
5 years, 3 months ago (2015-09-05 04:02:03 UTC) #12
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1329013002/ by kochi@chromium.org.

The reason for reverting is: This caused regression.
https://code.google.com/p/chromium/issues/detail?id=528370
.

Powered by Google App Engine
This is Rietveld 408576698