Closed Bug 929885 Opened 7 years ago Closed 7 years ago

Implement web components ShadowRoot style sheet behavior.

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: wchen, Assigned: wchen)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file)

Comment on attachment 820828 [details] [diff] [review]
Implement web components ShadowRoot style sheet behavior.

Review of attachment 820828 [details] [diff] [review]:
-----------------------------------------------------------------

This looks really good. Just a couple of small comments.

::: content/base/src/ShadowRoot.cpp
@@ +139,5 @@
> +
> +  mProtoBinding->FlushSkinSheets();
> +
> +  nsIPresShell* shell = OwnerDoc()->GetShell();
> +  if (shell) {

I think it's worth making a helper function for this (RestyleShadowRoot()) and sharing it between Insert/RemoveSheet.

::: content/html/content/src/HTMLStyleElement.cpp
@@ +161,2 @@
>    nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent);
> +  UpdateStyleSheetInternal(oldDoc, oldShadow);

Nit: no need for single-use |oldShadow|.

::: content/svg/content/src/SVGStyleElement.cpp
@@ +86,2 @@
>    SVGStyleElementBase::UnbindFromTree(aDeep, aNullParent);
> +  UpdateStyleSheetInternal(oldDoc, nullptr);

I asked you about this on IRC and you said that it's probably OK to pass null for the oldShadowRoot. Please add a comment why or simply pass in the shadow root as you said that'd also work and should be harmless.
Attachment #820828 - Flags: review?(mrbkap) → review+
https://hg.mozilla.org/mozilla-central/rev/1ac0576fa66f
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.