Closed Bug 1025724 Opened 10 years ago Closed 7 years ago

reprocess <style> elements in shadow trees of elements just bound to the document

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: heycam, Assigned: heycam)

References

Details

Attachments

(2 files)

Currently if a <style> element is inserted into a ShadowRoot before the ShadowRoot's host is inserted into the document, we won't create a style sheet for the <style> element.
This is because HTMLStyleElement::BindToTree calls nsStyleLinkElement::UpdateStyleSheetInternal, but that only creates a style sheet if we are in a document.
Attached patch patchSplinter Review
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #8440477 - Flags: review?(bzbarsky)
Hrm. Do we not rebind (in the BindToTree sense) the anonymous content when the shadow host's parent chain changes? That's likely to break other things too, no? Things like iframes in the shadow tree come to mind.
Flags: needinfo?(wchen)
Let me ask another question. If a <style> is inserted into a ShadowRoot _after_ the host is in the document, what makes _that_ work?
(In reply to Boris Zbarsky [:bz] from comment #3) > Hrm. Do we not rebind (in the BindToTree sense) the anonymous content when > the shadow host's parent chain changes? That's likely to break other things > too, no? Things like iframes in the shadow tree come to mind. No, we don't rebind anonymous content in the shadow tree right now. Elements in the shadow tree behave similar to as if appended into a document fragment, BindToTree and UnbindFromTree only get called when inserted/removed from the shadow tree. I guess how we want to handle this depends on whether BindToTree/UnbindFromTree also means BindToComposedTree/UnbindFromComposedTree. It's a little strange for the Shadow DOM to behave like the latter compared to XBL because when the host is unbound, nodes in the shadow tree are unbound from the composed tree but they are not unbound from the shadow tree. In XBL when a host is unbound, node in anonymous content are unbound from the composed tree and due to tear-down they are also unbound from whatever tree they were in. (In reply to Boris Zbarsky [:bz] from comment #4) > Let me ask another question. If a <style> is inserted into a ShadowRoot > _after_ the host is in the document, what makes _that_ work? http://dxr.mozilla.org/mozilla-central/source/content/base/src/ShadowRoot.cpp#170
Flags: needinfo?(wchen)
> whether BindToTree/UnbindFromTree also means BindToComposedTree/UnbindFromComposedTree Right now, for normal content and XBL, BindToTree means binding to composed tree if aDocument is not null. UnbindFromTree always means unbinding from composed tree. I think we should generally preserve those semantics, ideally. If we do, can we remove the special-casing of style elements in shadow trees we have right now? Seems like at that point the normal style loading/application codepath will do the right thing, modulo the need to scope correctly.
Comment on attachment 8440477 [details] [diff] [review] patch I'd like to understand what our general plan, if any, is for propagating BindToTree to shadow trees. Seems like this is a specific instance of that general plan...
Attachment #8440477 - Flags: feedback?(wchen)
Attachment #8440477 - Flags: feedback?(bugs)
I think the plan now is to have the BindToTree/UnbindFromTree calls propagate into the Shadow DOM. This is being tracked by bug 1062578.
OK. Would that just fix this bug automagically?
Depends on: 1062578
Comment on attachment 8440477 [details] [diff] [review] patch Yeah, I don't think we should add this kind of specific hacks, but just make BindToTree deal with shadow dom
Attachment #8440477 - Flags: feedback?(bugs)
Attachment #8440477 - Flags: review?(bzbarsky)
Comment on attachment 8440477 [details] [diff] [review] patch (In reply to Boris Zbarsky [:bz] from comment #9) > OK. Would that just fix this bug automagically? I ran the reftests in this patch against the fix in the other bug and it is automagically fixed.
Attachment #8440477 - Flags: feedback?(wchen)
Blocks: 1056783
Cool. Should we land the reftests in this bug, or are they already covered by bug 1062578?
We should land the reftests in this bug.
Flags: needinfo?(cam)
Emilio, these may be redundant with other tests you've added elsewhere (or we've got from WPT). If so, let me know, and we can drop them.
Flags: needinfo?(cam)
Attachment #8965233 - Flags: review?(bzbarsky) → review+
(Meant to direct the review to Emilio, hence my comment 16.)
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #17) > Comment on attachment 8965233 [details] > Bug 1025724 - Add some shadow tree style tests. > > https://reviewboard.mozilla.org/r/233944/#review239940 Yeah, there are tests for this kind of stuff in the WPT directories css/css-scoping and shadow-dom. But I guess landing these doesn't hurt either.
Pushed by cam@mcc.id.au: https://hg.mozilla.org/integration/autoland/rev/fd8b1659a4ab Add some shadow tree style tests. r=bz
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: