Bug 1410578: Make <link rel="stylesheet"> work in shadow trees. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 22 Jun 2018 01:12:42 +0200
changeset 477531 d7b2d2d11109e2cd17e2f68960260631560d03f3
parent 477530 e27f16329c725d807150863588ed572b2990c60c
child 477560 6b6f3f6ecf142908b3e437d3bc3fac75540a9bcb
child 477589 ba0e8cc012f74ea167dc14caaf32386f5290b209
push id9385
push userdluca@mozilla.com
push dateFri, 22 Jun 2018 15:47:18 +0000
treeherdermozilla-beta@82a9a1027e2b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1410578
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1410578: Make <link rel="stylesheet"> work in shadow trees. r=heycam Summary: Somewhat straight-forward, mostly removing special-casing. Differential Revision: https://phabricator.services.mozilla.com/D1761 MozReview-Commit-ID: 6f8duD4pGrl
dom/base/nsStyleLinkElement.cpp
dom/html/HTMLLinkElement.cpp
dom/tests/mochitest/webcomponents/test_shadowroot_inert_element.html
layout/style/Loader.cpp
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/css/css-scoping/shadow-link-rel-stylesheet-no-style-leak.html
testing/web-platform/tests/css/css-scoping/shadow-link-rel-stylesheet.html
--- a/dom/base/nsStyleLinkElement.cpp
+++ b/dom/base/nsStyleLinkElement.cpp
@@ -287,53 +287,50 @@ nsStyleLinkElement::DoUpdateStyleSheet(n
 
   if (thisContent->IsInAnonymousSubtree() &&
       thisContent->IsAnonymousContentInSVGUseSubtree()) {
     // Stylesheets in <use>-cloned subtrees are disabled until we figure out
     // how they should behave.
     return Update { };
   }
 
-  // Check for a ShadowRoot because link elements are inert in a
-  // ShadowRoot.
-  ShadowRoot* containingShadow = thisContent->GetContainingShadow();
-  if (thisContent->IsHTMLElement(nsGkAtoms::link) &&
-      (aOldShadowRoot || containingShadow)) {
-    return Update { };
-  }
-
   if (mStyleSheet && (aOldDocument || aOldShadowRoot)) {
     MOZ_ASSERT(!(aOldDocument && aOldShadowRoot),
                "ShadowRoot content is never in document, thus "
                "there should not be a old document and old "
                "ShadowRoot simultaneously.");
 
     // We're removing the link element from the document or shadow tree,
     // 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 {
       aOldDocument->RemoveStyleSheet(mStyleSheet);
     }
 
-    nsStyleLinkElement::SetStyleSheet(nullptr);
+    SetStyleSheet(nullptr);
+  }
+
+  nsIDocument* doc = thisContent->IsInShadowTree()
+    ? thisContent->OwnerDoc() : thisContent->GetUncomposedDoc();
+
+  // Loader could be null during unlink, see bug 1425866.
+  if (!doc || !doc->CSSLoader() || !doc->CSSLoader()->GetEnabled()) {
+    return Update { };
   }
 
   // When static documents are created, stylesheets are cloned manually.
-  if (mDontLoadStyle || !mUpdatesEnabled ||
-      thisContent->OwnerDoc()->IsStaticDocument()) {
-    return Update { };
-  }
-
-  nsCOMPtr<nsIDocument> doc = thisContent->IsInShadowTree() ?
-    thisContent->OwnerDoc() : thisContent->GetUncomposedDoc();
-  // Loader could be null during unlink, see bug 1425866.
-  if (!doc || !doc->CSSLoader() || !doc->CSSLoader()->GetEnabled()) {
+  if (mDontLoadStyle || !mUpdatesEnabled || doc->IsStaticDocument()) {
     return Update { };
   }
 
   Maybe<SheetInfo> info = GetStyleSheetInfo();
   if (aForceUpdate == ForceUpdate::No &&
       mStyleSheet &&
       info &&
       !info->mIsInline &&
@@ -389,25 +386,25 @@ nsStyleLinkElement::DoUpdateStyleSheet(n
         return Err(rv);
       }
       return Update { };
     }
 
     // Parse the style sheet.
     return doc->CSSLoader()->LoadInlineStyle(*info, text, mLineNumber, aObserver);
   }
-  nsAutoString integrity;
   if (thisContent->IsElement()) {
+    nsAutoString integrity;
     thisContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::integrity,
                                       integrity);
-  }
-  if (!integrity.IsEmpty()) {
-    MOZ_LOG(SRILogHelper::GetSriLog(), mozilla::LogLevel::Debug,
-            ("nsStyleLinkElement::DoUpdateStyleSheet, integrity=%s",
-             NS_ConvertUTF16toUTF8(integrity).get()));
+    if (!integrity.IsEmpty()) {
+      MOZ_LOG(SRILogHelper::GetSriLog(), mozilla::LogLevel::Debug,
+              ("nsStyleLinkElement::DoUpdateStyleSheet, integrity=%s",
+               NS_ConvertUTF16toUTF8(integrity).get()));
+    }
   }
   auto resultOrError = doc->CSSLoader()->LoadStyleLink(*info, aObserver);
   if (resultOrError.isErr()) {
     // Don't propagate LoadStyleLink() errors further than this, since some
     // consumers (e.g. nsXMLContentSink) will completely abort on innocuous
     // things like a stylesheet load being blocked by the security system.
     return Update { };
   }
--- a/dom/html/HTMLLinkElement.cpp
+++ b/dom/html/HTMLLinkElement.cpp
@@ -120,33 +120,30 @@ HTMLLinkElement::OnDNSPrefetchDeferred()
 
 bool
 HTMLLinkElement::HasDeferredDNSPrefetchRequest()
 {
   return HasFlag(HTML_LINK_DNS_PREFETCH_DEFERRED);
 }
 
 nsresult
-HTMLLinkElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent,
+HTMLLinkElement::BindToTree(nsIDocument* aDocument,
+                            nsIContent* aParent,
                             nsIContent* aBindingParent,
                             bool aCompileEventHandlers)
 {
   Link::ResetLinkState(false, Link::ElementHasHref());
 
   nsresult rv = nsGenericHTMLElement::BindToTree(aDocument, aParent,
                                                  aBindingParent,
                                                  aCompileEventHandlers);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // Link must be inert in ShadowRoot.
-  if (aDocument && !GetContainingShadow()) {
-    aDocument->RegisterPendingLinkUpdate(this);
-  }
-
-  if (IsInComposedDoc()) {
+  if (nsIDocument* doc = GetComposedDoc()) {
+    doc->RegisterPendingLinkUpdate(this);
     TryDNSPrefetchOrPreconnectOrPrefetchOrPreloadOrPrerender();
   }
 
   void (HTMLLinkElement::*update)() = &HTMLLinkElement::UpdateStyleSheetInternal;
   nsContentUtils::AddScriptRunner(
     NewRunnableMethod("dom::HTMLLinkElement::BindToTree", this, update));
 
   CreateAndDispatchEvent(aDocument, NS_LITERAL_STRING("DOMLinkAdded"));
@@ -177,22 +174,18 @@ HTMLLinkElement::UnbindFromTree(bool aDe
   CancelPrefetchOrPreload();
 
   // If this link is ever reinserted into a document, it might
   // be under a different xml:base, so forget the cached state now.
   Link::ResetLinkState(false, Link::ElementHasHref());
 
   // If this is reinserted back into the document it will not be
   // from the parser.
-  nsCOMPtr<nsIDocument> oldDoc = GetUncomposedDoc();
-
-  // Check for a ShadowRoot because link elements are inert in a
-  // ShadowRoot.
-  ShadowRoot* oldShadowRoot = GetBindingParent() ?
-    GetBindingParent()->GetShadowRoot() : nullptr;
+  nsIDocument* oldDoc = GetUncomposedDoc();
+  ShadowRoot* oldShadowRoot = GetContainingShadow();
 
   CreateAndDispatchEvent(oldDoc, NS_LITERAL_STRING("DOMLinkRemoved"));
   nsGenericHTMLElement::UnbindFromTree(aDeep, aNullParent);
 
   Unused << UpdateStyleSheetInternal(oldDoc, oldShadowRoot);
 }
 
 bool
--- a/dom/tests/mochitest/webcomponents/test_shadowroot_inert_element.html
+++ b/dom/tests/mochitest/webcomponents/test_shadowroot_inert_element.html
@@ -11,39 +11,20 @@ https://bugzilla.mozilla.org/show_bug.cg
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=806506">Bug 806506</a>
 <script>
 
 SimpleTest.waitForExplicitFinish();
 
 var content = '<div id="grabme"></div>';
-setShadowDOMPrefAndCreateIframe(content)
-  .then((aDocument) => {
-    function runChecks() {
-      isnot(aDocument.defaultView.getComputedStyle(shadowSpan, null).getPropertyValue("padding-top"), "10px", "Link element should be inert.");
-      is(aDocument.styleSheets.length, numStyleBeforeLoad, "Document style count should remain the same because the style should not be in the doucment.");
-      is(shadow.styleSheets.length, 0, "Inert link should not add style to ShadowRoot.");
-      // Remove link to make sure we don't get assertions.
-      shadow.removeChild(shadowStyle);
-      SimpleTest.finish();
-    };
-
-    var element = aDocument.getElementById("grabme");
-    var shadow = element.attachShadow({mode: "open"});
+setShadowDOMPrefAndCreateIframe(content).then(aDocument => {
+  var element = aDocument.getElementById("grabme");
+  var shadow = element.attachShadow({mode: "open"});
 
-    // Check that <base> is inert.
-    shadow.innerHTML = '<base href="http://www.example.org/" />';
-    isnot(aDocument.baseURI, "http://www.example.org/", "Base element should be inert in ShadowRoot.");
-
-    // Check that <link> is inert.
-    var numStyleBeforeLoad = aDocument.styleSheets.length;
-
-    shadow.innerHTML = '<link id="shadowlink" rel="stylesheet" type="text/css" href="inert_style.css" /><span id="shadowspan"></span>';
-    shadow.applyAuthorStyles = true;
-    var shadowSpan = shadow.getElementById("shadowspan");
-    var shadowStyle = shadow.getElementById("shadowlink");
-
-    runChecks();
-  });
+  // Check that <base> is inert.
+  shadow.innerHTML = '<base href="http://www.example.org/" />';
+  isnot(aDocument.baseURI, "http://www.example.org/", "Base element should be inert in ShadowRoot.");
+  SimpleTest.finish();
+});
 </script>
 </body>
 </html>
--- a/layout/style/Loader.cpp
+++ b/layout/style/Loader.cpp
@@ -822,19 +822,16 @@ SheetLoadData::VerifySheetReadyToParse(n
 Loader::IsAlternate
 Loader::IsAlternateSheet(const nsAString& aTitle, bool aHasAlternateRel)
 {
   // A sheet is alternate if it has a nonempty title that doesn't match the
   // currently selected style set.  But if there _is_ no currently selected
   // style set, the sheet wasn't marked as an alternate explicitly, and aTitle
   // is nonempty, we should select the style set corresponding to aTitle, since
   // that's a preferred sheet.
-  //
-  // FIXME(emilio): This should return false for Shadow DOM regardless of the
-  // document.
   if (aTitle.IsEmpty()) {
     return IsAlternate::No;
   }
 
   if (mDocument) {
     const nsString& currentSheetSet = mDocument->GetCurrentStyleSheetSet();
     if (!aHasAlternateRel && currentSheetSet.IsEmpty()) {
       // There's no preferred set yet, and we now have a sheet with a title.
@@ -1916,20 +1913,18 @@ Loader::LoadInlineStyle(const SheetInfo&
   NS_ASSERTION(state == eSheetNeedsParser,
                "Inline sheets should not be cached");
 
   LOG(("  Sheet is alternate: %d", static_cast<int>(isAlternate)));
 
   auto matched =
     PrepareSheet(sheet, aInfo.mTitle, aInfo.mMedia, nullptr, isAlternate);
 
-  if (aInfo.mContent->HasFlag(NODE_IS_IN_SHADOW_TREE)) {
-    ShadowRoot* containingShadow = aInfo.mContent->GetContainingShadow();
-    MOZ_ASSERT(containingShadow);
-    containingShadow->InsertSheet(sheet, aInfo.mContent);
+  if (aInfo.mContent->IsInShadowTree()) {
+    aInfo.mContent->GetContainingShadow()->InsertSheet(sheet, aInfo.mContent);
   } else {
     rv = InsertSheetInDoc(sheet, aInfo.mContent, mDocument);
     if (NS_FAILED(rv)) {
       return Err(rv);
     }
   }
 
   nsIPrincipal* principal = aInfo.mContent->NodePrincipal();
@@ -2035,20 +2030,23 @@ Loader::LoadStyleLink(const SheetInfo& a
     return Err(rv);
   }
 
   LOG(("  Sheet is alternate: %d", static_cast<int>(isAlternate)));
 
   auto matched =
     PrepareSheet(sheet, aInfo.mTitle, aInfo.mMedia, nullptr, isAlternate);
 
-  // FIXME(emilio, bug 1410578): Shadow DOM should be handled here too.
-  rv = InsertSheetInDoc(sheet, aInfo.mContent, mDocument);
-  if (NS_FAILED(rv)) {
-    return Err(rv);
+  if (aInfo.mContent && aInfo.mContent->IsInShadowTree()) {
+    aInfo.mContent->GetContainingShadow()->InsertSheet(sheet, aInfo.mContent);
+  } else {
+    rv = InsertSheetInDoc(sheet, aInfo.mContent, mDocument);
+    if (NS_FAILED(rv)) {
+      return Err(rv);
+    }
   }
 
   nsCOMPtr<nsIStyleSheetLinkingElement> owningElement(
     do_QueryInterface(aInfo.mContent));
 
   if (state == eSheetComplete) {
     LOG(("  Sheet already complete: 0x%p", sheet.get()));
     if (aObserver || !mObservers.IsEmpty() || owningElement) {
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -129446,16 +129446,40 @@
       [
        "/css/css-scoping/reference/green-box.html",
        "=="
       ]
      ],
      {}
     ]
    ],
+   "css/css-scoping/shadow-link-rel-stylesheet-no-style-leak.html": [
+    [
+     "/css/css-scoping/shadow-link-rel-stylesheet-no-style-leak.html",
+     [
+      [
+       "/css/css-scoping/reference/green-box.html",
+       "=="
+      ]
+     ],
+     {}
+    ]
+   ],
+   "css/css-scoping/shadow-link-rel-stylesheet.html": [
+    [
+     "/css/css-scoping/shadow-link-rel-stylesheet.html",
+     [
+      [
+       "/css/css-scoping/reference/green-box.html",
+       "=="
+      ]
+     ],
+     {}
+    ]
+   ],
    "css/css-scoping/shadow-reassign-dynamic-001.html": [
     [
      "/css/css-scoping/shadow-reassign-dynamic-001.html",
      [
       [
        "/css/css-scoping/reference/green-box.html",
        "=="
       ]
@@ -523079,16 +523103,24 @@
   "css/css-scoping/shadow-fallback-dynamic-005.html": [
    "ab3c3d205e59df800ba5b4217245b83685521c31",
    "reftest"
   ],
   "css/css-scoping/shadow-host-with-before-after.html": [
    "99af6e29e69b3131b59dbdc2b0eead52931123c2",
    "reftest"
   ],
+  "css/css-scoping/shadow-link-rel-stylesheet-no-style-leak.html": [
+   "76a54dabd8bd09f7155ab0331e3d17d1a0cae243",
+   "reftest"
+  ],
+  "css/css-scoping/shadow-link-rel-stylesheet.html": [
+   "07862ce7d2a954988bdbce882869a4c5f097089a",
+   "reftest"
+  ],
   "css/css-scoping/shadow-reassign-dynamic-001.html": [
    "11ed4da2e6ce88d8a2b98a8f1c814417ef7770dd",
    "reftest"
   ],
   "css/css-scoping/shadow-reassign-dynamic-002.html": [
    "2a90e0623a99cfb46430f0236ceea44f93a25131",
    "reftest"
   ],
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-scoping/shadow-link-rel-stylesheet-no-style-leak.html
@@ -0,0 +1,21 @@
+<!doctype html>
+<title>CSS Test: &lt;link rel="stylesheet"&gt; in Shadow DOM doesn't affect the normal DOM</title>
+<link rel="help" href="https://html.spec.whatwg.org/#link-type-stylesheet">
+<link rel="help" href="https://drafts.csswg.org/css-scoping/#selectors-data-model">
+<link rel="match" href="reference/green-box.html">
+<p>Test passes if you see a single 100px by 100px green box below.</p>
+<style>
+  #light-dom {
+    width: 100px;
+    height: 100px;
+    background: green;
+    color: green;
+  }
+</style>
+<div id="host">FAIL</div>
+<div id="light-dom"></div>
+<script>
+  host.attachShadow({ mode: "open" }).innerHTML = `
+    <link rel="stylesheet" href="data:text/css,div { background: red !important }">
+  `;
+</script>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-scoping/shadow-link-rel-stylesheet.html
@@ -0,0 +1,11 @@
+<!doctype html>
+<title>CSS Test: &lt;link rel="stylesheet"&gt; in Shadow DOM</title>
+<link rel="help" href="https://html.spec.whatwg.org/#link-type-stylesheet">
+<link rel="match" href="reference/green-box.html">
+<p>Test passes if you see a single 100px by 100px green box below.</p>
+<div id="host">FAIL</div>
+<script>
+  host.attachShadow({ mode: "open" }).innerHTML = `
+    <link rel="stylesheet" href="resources/host-green-box.css">
+  `;
+</script>