Skip to content
This repository has been archived by the owner on Nov 7, 2018. It is now read-only.

Commit

Permalink
Bug 1490791 - Make ShadowRoot stylesheet list work only when the shad…
Browse files Browse the repository at this point in the history
…ow root is connected. r=smaug

This avoids the issue of triggering stylesheet loads for disconnected stuff and
such, and is more consistent with Chrome and WebKit.

This is per the recent CSSWG resolution at
w3c/csswg-drafts#3096.

Differential Revision: https://phabricator.services.mozilla.com/D5699
  • Loading branch information
emilio committed Sep 12, 2018
1 parent 5e8eb3d commit 4a447b2
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 13 deletions.
8 changes: 1 addition & 7 deletions dom/base/nsStyleLinkElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -303,11 +303,6 @@ nsStyleLinkElement::DoUpdateStyleSheet(nsIDocument* aOldDocument,
// unload the stylesheet. We want to do this even if updates are
// disabled, since otherwise a sheet with a stale linking element pointer
// will be hanging around -- not good!
//
// TODO(emilio): We can reach this code with aOldShadowRoot ==
// thisContent->GetContainingShadowRoot(), when moving the shadow host
// around. We probably could optimize some of this stuff out, is it worth
// it?
if (aOldShadowRoot) {
aOldShadowRoot->RemoveSheet(mStyleSheet);
} else {
Expand All @@ -317,8 +312,7 @@ nsStyleLinkElement::DoUpdateStyleSheet(nsIDocument* aOldDocument,
SetStyleSheet(nullptr);
}

nsIDocument* doc = thisContent->IsInShadowTree()
? thisContent->OwnerDoc() : thisContent->GetUncomposedDoc();
nsIDocument* doc = thisContent->GetComposedDoc();

// Loader could be null during unlink, see bug 1425866.
if (!doc || !doc->CSSLoader() || !doc->CSSLoader()->GetEnabled()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,10 @@
var host = document.createElement('div');
var shadowRoot = host.attachShadow({'mode': mode});

assert_equals(shadowRoot.styleSheets.length, 0, 'shadowRoot.styleSheets must be empty when the shadow root does not contain any stylesheets');
shadowRoot.innerHTML = '<span></span><style> a.rule {} </style><style> b.rule {} </style>';
assert_equals(shadowRoot.styleSheets.length, 0, 'shadowRoot.styleSheets must be empty when the shadow root is not connected');

document.body.appendChild(host);
assert_equals(shadowRoot.styleSheets.length, 2, 'shadowRoot.styleSheets must contain two items when the shadow root has two style elements');
var styles = shadowRoot.querySelectorAll('style');
assert_equals(shadowRoot.styleSheets[0], styles[0].sheet, 'shadowRoot.styleSheets[0] must be the first style element in the shadow root');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@
assert_equals(s.styleSheets.length, 0, 'There should be no style sheets');
}), 'A_06_00_03_T02');

//TODO Now this tests produces an error on Chromium because styleSheets.length
//returns 0 when the shadow root is orphaned.
//Tracking bug: http://crbug.com/392771
test(unit(function (ctx) {
var d = newRenderedHTMLDocument(ctx);
var host = d.createElement('div');
Expand All @@ -60,8 +57,7 @@
style.textContent = 'div {width: 50%;}';
s.appendChild(style);

// The following line fixes the issue on Chromium, http://crbug.com/392771
// d.body.appendChild(host);
d.body.appendChild(host);
assert_equals(s.styleSheets.length, 1, 'Style sheet is not accessible via styleSheets');
}), 'A_06_00_03_T03');
</script>
Expand Down

0 comments on commit 4a447b2

Please sign in to comment.