Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(8)

Issue 2177163002: Allow <link rel=stylesheet> in a connected shadow tree (Closed)

Created:
2 years, 3 months ago by hayato
Modified:
2 years, 3 months ago
Reviewers:
tkent, esprehn, kochi, rune
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-dom_chromium.org, blink-reviews-html_chromium.org, blink-reviews-style_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, gavinp+prerender_chromium.org, rwlbuis, sof, Yoav Weiss
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Allow <link rel=stylesheet> in a connected shadow tree See https://github.com/w3c/webcomponents/issues/530. We have relaxed the condition. "Web-Facing Change PSA" mail in blink-dev is: https://groups.google.com/a/chromium.org/d/msg/blink-dev/b7g62d2yqWo/2itpAFafBgAJ Regarding a browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. See also https://github.com/w3c/webcomponents/issues/535. "title= attribute" is ignored completely for <link rel=stylesheet>. BUG=630141 Committed: https://crrev.com/f0cecf3ac0c7adf7a7c78eeb02cc75d380a9ce4d Cr-Commit-Position: refs/heads/master@{#409728}

Patch Set 1 #

Patch Set 2 : wip #

Total comments: 6

Patch Set 3 : reparent #

Patch Set 4 : fix #

Total comments: 4

Patch Set 5 : update #

Total comments: 3

Patch Set 6 : updated #

Total comments: 2

Patch Set 7 : split tests #

Messages

Total messages: 76 (43 generated)
hayato
wip
2 years, 3 months ago (2016-07-26 04:33:35 UTC) #9
hayato
PTAL
2 years, 3 months ago (2016-07-26 04:34:55 UTC) #13
kochi
Nit: please update the subject and description (s/re=stylesheet/rel=stylesheet/). https://codereview.chromium.org/2177163002/diff/20001/third_party/WebKit/LayoutTests/shadow-dom/link-element.html File third_party/WebKit/LayoutTests/shadow-dom/link-element.html (right): https://codereview.chromium.org/2177163002/diff/20001/third_party/WebKit/LayoutTests/shadow-dom/link-element.html#newcode18 third_party/WebKit/LayoutTests/shadow-dom/link-element.html:18: assert_equals(computedBackgroundColor(host), ...
2 years, 3 months ago (2016-07-26 05:35:06 UTC) #15
hayato
Thank you for the review. Let me upload a new patch later and ping you. ...
2 years, 3 months ago (2016-07-26 06:04:32 UTC) #18
hayato
FYI. I have created another CL, https://codereview.chromium.org/2181263002, which should remove confusing code in media query.
2 years, 3 months ago (2016-07-26 07:44:42 UTC) #22
hayato
reparent
2 years, 3 months ago (2016-07-26 07:53:05 UTC) #23
rune
I suspect this will incorrectly make title and rel=alternate on link elements inside shadow trees ...
2 years, 3 months ago (2016-07-26 09:56:16 UTC) #27
hayato
On 2016/07/26 at 09:56:16, rune wrote: > I suspect this will incorrectly make title and ...
2 years, 3 months ago (2016-07-27 06:52:37 UTC) #30
hayato
I think no one has discussed well how <link rel=alternate> and <link title=> should work ...
2 years, 3 months ago (2016-07-27 07:57:31 UTC) #31
hayato
My understanding is: - We do not have to ignore <link rel="alternate stylesheet" title="xxx">. It ...
2 years, 3 months ago (2016-07-27 08:24:47 UTC) #32
kochi
On 2016/07/27 08:24:47, hayato wrote: > My understanding is: > > - We do not ...
2 years, 3 months ago (2016-07-27 09:02:49 UTC) #33
hayato
> We had a discussion (closed without conclusion?): > https://github.com/w3c/webcomponents/issues/391 I am aware of this ...
2 years, 3 months ago (2016-07-27 09:39:06 UTC) #34
kochi
On 2016/07/27 09:39:06, hayato wrote: > > We had a discussion (closed without conclusion?): > ...
2 years, 3 months ago (2016-07-27 09:56:38 UTC) #35
hayato
fix
2 years, 3 months ago (2016-07-28 10:11:36 UTC) #36
hayato
I have updated the patch so it reflects my proposal. See the test for details. ...
2 years, 3 months ago (2016-07-28 10:13:31 UTC) #39
kochi
https://codereview.chromium.org/2177163002/diff/60001/third_party/WebKit/LayoutTests/shadow-dom/link.html File third_party/WebKit/LayoutTests/shadow-dom/link.html (right): https://codereview.chromium.org/2177163002/diff/60001/third_party/WebKit/LayoutTests/shadow-dom/link.html#newcode18 third_party/WebKit/LayoutTests/shadow-dom/link.html:18: test.step(() => { nit: instead of "(e) => { ...
2 years, 3 months ago (2016-07-29 07:10:12 UTC) #42
hayato
update
2 years, 3 months ago (2016-07-29 07:39:19 UTC) #43
hayato
Thank you for the review. https://codereview.chromium.org/2177163002/diff/60001/third_party/WebKit/LayoutTests/shadow-dom/link.html File third_party/WebKit/LayoutTests/shadow-dom/link.html (right): https://codereview.chromium.org/2177163002/diff/60001/third_party/WebKit/LayoutTests/shadow-dom/link.html#newcode18 third_party/WebKit/LayoutTests/shadow-dom/link.html:18: test.step(() => { Done ...
2 years, 3 months ago (2016-07-29 07:41:57 UTC) #46
hayato
rune@, could you have a chance to take a look again?
2 years, 3 months ago (2016-08-01 03:45:49 UTC) #49
rune
https://codereview.chromium.org/2177163002/diff/80001/third_party/WebKit/LayoutTests/shadow-dom/link-title.html File third_party/WebKit/LayoutTests/shadow-dom/link-title.html (right): https://codereview.chromium.org/2177163002/diff/80001/third_party/WebKit/LayoutTests/shadow-dom/link-title.html#newcode35 third_party/WebKit/LayoutTests/shadow-dom/link-title.html:35: assert_equals(colorFor(host.shadowRoot.querySelector('#shadowChild3')), 'rgb(0, 0, 0)', 'An alternate stylesheet should be ...
2 years, 3 months ago (2016-08-01 09:22:15 UTC) #50
hayato
This CL's intention is: 1) <link rel="stylesheet" title="x"> 2) <link rel="alternate stylesheet" title="x"> behave as: ...
2 years, 3 months ago (2016-08-01 09:38:01 UTC) #51
rune
On 2016/08/01 09:38:01, hayato wrote: > This CL's intention is: > > 1) <link rel="stylesheet" ...
2 years, 3 months ago (2016-08-01 10:03:31 UTC) #52
hayato
Thanks! I have filed a spec issue: https://github.com/w3c/webcomponents/issues/535
2 years, 3 months ago (2016-08-01 11:31:26 UTC) #53
hayato
updated
2 years, 3 months ago (2016-08-03 04:21:25 UTC) #54
hayato
It looks we can reach rough consensus in https://github.com/w3c/webcomponents/issues/535. I have reflected the conclusion in ...
2 years, 3 months ago (2016-08-03 04:24:19 UTC) #58
rune
lgtm https://codereview.chromium.org/2177163002/diff/100001/third_party/WebKit/LayoutTests/shadow-dom/link-title.html File third_party/WebKit/LayoutTests/shadow-dom/link-title.html (right): https://codereview.chromium.org/2177163002/diff/100001/third_party/WebKit/LayoutTests/shadow-dom/link-title.html#newcode36 third_party/WebKit/LayoutTests/shadow-dom/link-title.html:36: assert_equals(colorFor(host.shadowRoot.querySelector('#shadowChild4')), 'rgb(0, 0, 0)', 'An alternate stylesheet should ...
2 years, 3 months ago (2016-08-03 08:55:29 UTC) #61
hayato
split tests
2 years, 3 months ago (2016-08-04 03:56:46 UTC) #62
hayato
Thank you for the review. https://codereview.chromium.org/2177163002/diff/100001/third_party/WebKit/LayoutTests/shadow-dom/link-title.html File third_party/WebKit/LayoutTests/shadow-dom/link-title.html (right): https://codereview.chromium.org/2177163002/diff/100001/third_party/WebKit/LayoutTests/shadow-dom/link-title.html#newcode36 third_party/WebKit/LayoutTests/shadow-dom/link-title.html:36: assert_equals(colorFor(host.shadowRoot.querySelector('#shadowChild4')), 'rgb(0, 0, 0)', ...
2 years, 3 months ago (2016-08-04 03:58:15 UTC) #65
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2177163002/120001
2 years, 3 months ago (2016-08-04 03:58:51 UTC) #69
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_asan_rel_ng/builds/203893)
2 years, 3 months ago (2016-08-04 05:19:52 UTC) #71
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2177163002/120001
2 years, 3 months ago (2016-08-04 06:01:48 UTC) #73
commit-bot: I haz the power
Committed patchset #7 (id:120001)
2 years, 3 months ago (2016-08-04 06:34:48 UTC) #74
commit-bot: I haz the power
2 years, 3 months ago (2016-08-04 06:36:26 UTC) #76
Message was sent while issue was closed.
Patchset 7 (id:??) landed as
https://crrev.com/f0cecf3ac0c7adf7a7c78eeb02cc75d380a9ce4d
Cr-Commit-Position: refs/heads/master@{#409728}

Powered by Google App Engine
This is Rietveld 408576698