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

Issue 208933006: Only :host and :ancestor should ever match the host in a ShadowRoot (Closed)

Created:
6 years, 9 months ago by esprehn
Modified:
5 years, 11 months ago
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, dglazkov+blink, ed+blinkwatch_opera.com, rwlbuis, rune+blink
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Only :host and :ancestor should ever match the host in a ShadowRoot We were blocking tag, id and class from matching when put before :host, but were not correctly stopping things like :first-child:host or *:host. Instead of trying to black list selectors we need to whitelist :host and :ancestor as being special. It turns out we did have test coverage for this, but the expected results were all wrong so it printed PASS even though we were failing what the test was supposed to be testing. BUG=355149, 355158 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169799

Patch Set 1 #

Patch Set 2 : Fix * #

Patch Set 3 : rebaseline tests #

Patch Set 4 : wrong type of selector #

Patch Set 5 : remove printf #

Patch Set 6 : Fix tests #

Patch Set 7 : Add a test for :first-child too #

Unified diffs Side-by-side diffs Delta from patch set Stats (+48 lines, -16 lines) Patch
M LayoutTests/fast/dom/shadow/ancestor-pseudo-class.html View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/dom/shadow/ancestor-pseudo-class-expected.txt View 1 2 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/dom/shadow/host-pseudo-class.html View 1 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/dom/shadow/host-pseudo-class-expected.txt View 1 2 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/fast/dom/shadow/shadow-tree-styles-select-host.html View 1 2 3 4 5 6 2 chunks +17 lines, -2 lines 0 comments Download
M LayoutTests/fast/dom/shadow/shadow-tree-styles-select-host-expected.txt View 1 2 3 4 5 6 1 chunk +4 lines, -2 lines 0 comments Download
M Source/core/css/CSSParserValues.h View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSParserValues.cpp View 1 2 3 1 chunk +10 lines, -0 lines 0 comments Download
M Source/core/css/CSSSelector.h View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/SelectorChecker.cpp View 1 2 3 4 5 6 1 chunk +7 lines, -5 lines 0 comments Download
M Source/core/css/parser/BisonCSSParser-in.cpp View 1 2 3 4 5 6 1 chunk +4 lines, -2 lines 0 comments Download

Messages

Total messages: 7 (1 generated)
esprehn
6 years, 9 months ago (2014-03-22 01:56:29 UTC) #1
esprehn
Okay this is ready for review.
6 years, 9 months ago (2014-03-22 02:52:18 UTC) #2
dglazkov
lgtm
6 years, 9 months ago (2014-03-22 15:22:26 UTC) #3
dglazkov
The CQ bit was checked by dglazkov@chromium.org
6 years, 9 months ago (2014-03-22 15:22:31 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/esprehn@chromium.org/208933006/110001
6 years, 9 months ago (2014-03-22 15:22:37 UTC) #5
commit-bot: I haz the power
6 years, 9 months ago (2014-03-22 20:57:09 UTC) #6
Message was sent while issue was closed.
Change committed as 169799

Powered by Google App Engine
This is Rietveld 408576698