Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2177163002:
Allow <link rel=stylesheet> in a connected shadow tree (Closed)
|
Created:
4 years, 5 months ago by hayato Modified:
4 years, 4 months ago 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. |
DescriptionAllow <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)
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Allow <link re=stylesheet> in a connected shadow tree BUG= ========== to ========== Allow <link re=stylesheet> in a connected shadow tree See https://github.com/w3c/webcomponents/issues/530. We have relaxed the condition. - Regarding browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. - Media query is still valid only in a document tree. BUG=630141 ==========
Description was changed from ========== Allow <link re=stylesheet> in a connected shadow tree See https://github.com/w3c/webcomponents/issues/530. We have relaxed the condition. - Regarding browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. - Media query is still valid only in a document tree. BUG=630141 ========== to ========== Allow <link re=stylesheet> in a connected shadow tree See https://github.com/w3c/webcomponents/issues/530. We have relaxed the condition. - Regarding a browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. - Media query is still valid only in a document tree. BUG=630141 ==========
Description was changed from ========== Allow <link re=stylesheet> in a connected shadow tree See https://github.com/w3c/webcomponents/issues/530. We have relaxed the condition. - Regarding a browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. - Media query is still valid only in a document tree. BUG=630141 ========== to ========== Allow <link re=stylesheet> in a connected shadow tree See https://github.com/w3c/webcomponents/issues/530. We have relaxed the condition. - Regarding a browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. - Media query is still valid only in a document tree. - I will send a "Web-Facing Change PSA" mail to blink-dev. BUG=630141 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Allow <link re=stylesheet> in a connected shadow tree See https://github.com/w3c/webcomponents/issues/530. We have relaxed the condition. - Regarding a browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. - Media query is still valid only in a document tree. - I will send a "Web-Facing Change PSA" mail to blink-dev. BUG=630141 ========== to ========== Allow <link re=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/2itpAFaf... - Regarding a browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. - Media query is still valid only in a document tree. BUG=630141 ==========
wip
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
hayato@chromium.org changed reviewers: + kochi@chromium.org, tkent@chromium.org
PTAL
dglazkov@chromium.org changed reviewers: + esprehn@chromium.org, rune@opera.com
Nit: please update the subject and description (s/re=stylesheet/rel=stylesheet/). https://codereview.chromium.org/2177163002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/shadow-dom/link-element.html (right): https://codereview.chromium.org/2177163002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/shadow-dom/link-element.html:18: assert_equals(computedBackgroundColor(host), 'rgba(0, 0, 0, 0)', 'A host in a document tree should not be styled.'); Having a :host rule in linked css and check if that rule is applied to host? (to check if the linked css load invalidate the original host style) https://codereview.chromium.org/2177163002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLLinkElement.cpp (right): https://codereview.chromium.org/2177163002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLLinkElement.cpp:206: if (isConnected() && !shouldLoadLink()) { Does insertionPoint->isConnected() on line 203 guarantee that this node is also connected? Then isConnected() here is unnecessary. https://codereview.chromium.org/2177163002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLLinkElement.cpp:665: // MediaQuery is valid only in a document tree. Is this a TODO, or defined in any spec?
Description was changed from ========== Allow <link re=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/2itpAFaf... - Regarding a browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. - Media query is still valid only in a document tree. BUG=630141 ========== to ========== 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/2itpAFaf... - Regarding a browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. - Media query is still valid only in a document tree. BUG=630141 ==========
Description was changed from ========== Allow <link re=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/2itpAFaf... - Regarding a browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. - Media query is still valid only in a document tree. BUG=630141 ========== to ========== 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/2itpAFaf... - Regarding a browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. - Media query is still valid only in a document tree. BUG=630141 ==========
Thank you for the review. Let me upload a new patch later and ping you. https://codereview.chromium.org/2177163002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/shadow-dom/link-element.html (right): https://codereview.chromium.org/2177163002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/shadow-dom/link-element.html:18: assert_equals(computedBackgroundColor(host), 'rgba(0, 0, 0, 0)', 'A host in a document tree should not be styled.'); That sounds another concern. That should not be done in this basic test. https://codereview.chromium.org/2177163002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLLinkElement.cpp (right): https://codereview.chromium.org/2177163002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLLinkElement.cpp:206: if (isConnected() && !shouldLoadLink()) { Good point. I have added DCHECK()s around here. https://codereview.chromium.org/2177163002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLLinkElement.cpp:665: // MediaQuery is valid only in a document tree. Ops. That was my misunderstanding. I though this as if https://github.com/w3c/webcomponents/issues/391. My memory is limited. :) Let me update the patch.
Description was changed from ========== 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/2itpAFaf... - Regarding a browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. - Media query is still valid only in a document tree. BUG=630141 ========== to ========== 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/2itpAFaf... - Regarding a browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. BUG=630141 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
FYI. I have created another CL, https://codereview.chromium.org/2181263002, which should remove confusing code in media query.
reparent
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== 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/2itpAFaf... - Regarding a browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. BUG=630141 ========== to ========== 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/2itpAFaf... Regarding a browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. BUG=630141 ==========
I suspect this will incorrectly make title and rel=alternate on link elements inside shadow trees start affecting preferred and alternate style sheet sets.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
On 2016/07/26 at 09:56:16, rune wrote: > I suspect this will incorrectly make title and rel=alternate on link elements inside shadow trees start affecting preferred and alternate style sheet sets. Yeah, let me investigate how allowing a link in a shadow tree affects these.
I think no one has discussed well how <link rel=alternate> and <link title=> should work in a shadow tree in https://github.com/w3c/webcomponents/issues/530. At least, we have to decide how "rel=altername" should work in a shadow tree in the standard discussion. As we did ignore for <style>'s title attribute in a shadow tree in Blink, it might be safe to ignore <link>'s "rel=alternate" for a while. WDTY?
My understanding is: - We do not have to ignore <link rel="alternate stylesheet" title="xxx">. It is harmless, I think. - We have to ignore the title attribute of <link rel="stylesheet" title="xxx">, as we did in https://codereview.chromium.org/1717303002, because it may have a side effect for a document tree. Does this sound good?
On 2016/07/27 08:24:47, hayato wrote: > My understanding is: > > - We do not have to ignore <link rel="alternate stylesheet" title="xxx">. It is > harmless, I think. > - We have to ignore the title attribute of <link rel="stylesheet" title="xxx">, > as we did in https://codereview.chromium.org/1717303002, because it may have a > side effect for a document tree. > > Does this sound good? We had a discussion (closed without conclusion?): https://github.com/w3c/webcomponents/issues/391 In the code review, the following comment was removed: (in ShadowTreeStyleSheetCollection.cpp) 60 // FIXME: clarify how PREFERRED or ALTERNATE works in shadow trees. 61 // Should we set preferred/selected stylesheets name in shadow trees and 62 // use the name in document? 63 if (candidate.hasPreferrableName(engine.preferredStylesh I suppose this is not clear. If we allow <link rel="alternate stylesheet" title="xxx">, then if |document.selectedStylesheetSet="xxx"| will change the stylesheet for the shadow DOM?
> We had a discussion (closed without conclusion?): > https://github.com/w3c/webcomponents/issues/391 I am aware of this discussion, which is about <style> element. I have reflected the conclusion at https://github.com/w3c/webcomponents/commit/08793befae7ab36ba7ee8cfd7c7455fbb... Now we have a <link> element. The difference is that link has "rel=alternate". > If we allow <link rel="alternate stylesheet" title="xxx">, then if > |document.selectedStylesheetSet="xxx"| will change the stylesheet for the shadow DOM? Yes, that's what I am proposing: - We honor the title attribute of a <link rel="alternate stylesheet" title="xxx"> in a shadow tree. Thus, in default, this stylesheet is not applied, unless this "title" is selected by users somehow. - Regarding <link rel="sylesheet" title="xxx"> in a shadow tree, it should not have a side effect for a document tree (or other node trees). However, if users specify the "alternate", we should not apply this stylesheet.
On 2016/07/27 09:39:06, hayato wrote: > > We had a discussion (closed without conclusion?): > > https://github.com/w3c/webcomponents/issues/391 > > I am aware of this discussion, which is about <style> element. I have reflected > the conclusion at > https://github.com/w3c/webcomponents/commit/08793befae7ab36ba7ee8cfd7c7455fbb... Sorry, I missed the link at the bottom of the #391. > Now we have a <link> element. The difference is that link has "rel=alternate". > > > > If we allow <link rel="alternate stylesheet" title="xxx">, then if > > |document.selectedStylesheetSet="xxx"| will change the stylesheet for the > shadow DOM? > > Yes, that's what I am proposing: > > - We honor the title attribute of a <link rel="alternate stylesheet" > title="xxx"> in a shadow tree. Thus, in default, this stylesheet is not applied, > unless this "title" is selected by users somehow. > - Regarding <link rel="sylesheet" title="xxx"> in a shadow tree, it should not > have a side effect for a document tree (or other node trees). However, if users > specify the "alternate", we should not apply this stylesheet. I think that is consistent. Thanks for clarification.
fix
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
I have updated the patch so it reflects my proposal. See the test for details. Could you take a look again?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2177163002/diff/60001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/shadow-dom/link.html (right): https://codereview.chromium.org/2177163002/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/shadow-dom/link.html:18: test.step(() => { nit: instead of "(e) => { test.step(() => {... test.done(); })", you can use "test.step_func_done((e) => { ... })" which creates a function object which calls test.done() at the end of the function. https://github.com/w3c/testharness.js/blob/master/docs/api.md https://codereview.chromium.org/2177163002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/StyleSheetCandidate.cpp (right): https://codereview.chromium.org/2177163002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/StyleSheetCandidate.cpp:92: if (isCSSStyle() || (isHTMLLink() && !isImport() && !isAlternate())) I'm not sure !isImport() here makes sense. My understanding is that if isHTMLLink() is true, it is already <link rel="stylesheet" ...> so does |&& !isImport()| check mean <link rel="stylesheet import" ...> ?
update
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thank you for the review. https://codereview.chromium.org/2177163002/diff/60001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/shadow-dom/link.html (right): https://codereview.chromium.org/2177163002/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/shadow-dom/link.html:18: test.step(() => { Done https://codereview.chromium.org/2177163002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/StyleSheetCandidate.cpp (right): https://codereview.chromium.org/2177163002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/StyleSheetCandidate.cpp:92: if (isCSSStyle() || (isHTMLLink() && !isImport() && !isAlternate())) I think we should check, as far as I see: - https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Style... - https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Style...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
rune@, could you have a chance to take a look again?
https://codereview.chromium.org/2177163002/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/shadow-dom/link-title.html (right): https://codereview.chromium.org/2177163002/diff/80001/third_party/WebKit/Layo... 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 disabled by default.'); A test for <link rel="alternate stylesheet" title="preferred1"> and <link rel="stylesheet" title="preferred1"> inside the shadow tree should be key tests here. https://codereview.chromium.org/2177163002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/StyleSheetCandidate.cpp (right): https://codereview.chromium.org/2177163002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/StyleSheetCandidate.cpp:94: } I think I would be leaning towards a behavior where the title attribute is ignored, effectively making: <link rel="stylesheet" title="x"> <link rel="alternate stylesheet" title="x"> behave as: <link rel="stylesheet"> // always enabled <link rel="alternate stylesheet"> // never enabled With the current patch, the former behaves as: <link rel="alternate stylesheet" title="x"> <link rel="alternate stylesheet" title="x"> Have you opened a spec issue?
This CL's intention is: 1) <link rel="stylesheet" title="x"> 2) <link rel="alternate stylesheet" title="x"> behave as: 1) <link rel="stylesheet"> // always enabled 2) <link rel="alternate stylesheet" title="x"> // disabled by default, unless users select alternate stylesheet of "x". But I do not have a strong opinion either. I am okay as long as 1) is enabled by default and 2) is disabled by default so that it matches users' intention (by default case). So, we have the same opinion in this sense. > > Have you opened a spec issue? Yeah, let me file a spec issue before proceeding.
On 2016/08/01 09:38:01, hayato wrote: > This CL's intention is: > > 1) <link rel="stylesheet" title="x"> > 2) <link rel="alternate stylesheet" title="x"> > > behave as: > > 1) <link rel="stylesheet"> // always enabled > 2) <link rel="alternate stylesheet" title="x"> // disabled by default, unless > users select alternate stylesheet of "x". Ah, yes, I misread the code. > But I do not have a strong opinion either. I am okay as long as 1) is enabled by > default and 2) is disabled by default so that it matches users' intention (by > default case). > So, we have the same opinion in this sense. My rationale is that we have other means of theming web-components than using alternate stylesheets and if the main document uses alternate stylesheets there might be name clashes if anyone use alternate stylesheets in shadow trees. Also, for the sake of speccing, it's cleaner and more straightforward to say that the title attribute has no effect in style and link elements rather than having to have the exception that the title attribute do matter if the stylesheet is alternate.
Thanks! I have filed a spec issue: https://github.com/w3c/webcomponents/issues/535
updated
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== 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/2itpAFaf... Regarding a browsing context, this CL does not change any behavior. That should be another concern and should be investigated later. BUG=630141 ========== to ========== 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/2itpAFaf... 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 ==========
It looks we can reach rough consensus in https://github.com/w3c/webcomponents/issues/535. I have reflected the conclusion in the new patch set. rune@, could you take a look again? https://codereview.chromium.org/2177163002/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/shadow-dom/link-title.html (right): https://codereview.chromium.org/2177163002/diff/80001/third_party/WebKit/Layo... 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 disabled by default.'); Done. I have updated the test.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_...)
lgtm https://codereview.chromium.org/2177163002/diff/100001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/shadow-dom/link-title.html (right): https://codereview.chromium.org/2177163002/diff/100001/third_party/WebKit/Lay... 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 be disabled in a shadow tree.'); Nit: these are really separate test()s. testharness stops a test() on a failing assert as well, so it's more debugging-friendly to wrap each test in a test().
split tests
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thank you for the review. https://codereview.chromium.org/2177163002/diff/100001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/shadow-dom/link-title.html (right): https://codereview.chromium.org/2177163002/diff/100001/third_party/WebKit/Lay... 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 be disabled in a shadow tree.'); On 2016/08/03 at 08:55:29, rune wrote: > Nit: these are really separate test()s. testharness stops a test() on a failing assert as well, so it's more debugging-friendly to wrap each test in a test(). Done. I have split this into 3 test()s.
The CQ bit was unchecked by hayato@chromium.org
The CQ bit was checked by hayato@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rune@opera.com Link to the patchset: https://codereview.chromium.org/2177163002/#ps120001 (title: "split tests")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
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_...)
The CQ bit was checked by hayato@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #7 (id:120001)
Message was sent while issue was closed.
Description was changed from ========== 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/2itpAFaf... 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 ========== to ========== 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/2itpAFaf... 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} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/f0cecf3ac0c7adf7a7c78eeb02cc75d380a9ce4d Cr-Commit-Position: refs/heads/master@{#409728} |
