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 809425 d7b2d2d11109e2cd17e2f68960260631560d03f3
parent 809424 e27f16329c725d807150863588ed572b2990c60c
child 809426 1bd5ff2542762b7ef8036e4ee1d8f3090c9c1f9b
child 809519 ba0e8cc012f74ea167dc14caaf32386f5290b209
child 809559 6b6f3f6ecf142908b3e437d3bc3fac75540a9bcb
push id113676
push userbmo:emilio@crisal.io
push dateFri, 22 Jun 2018 03:46:37 +0000
reviewersheycam
bugs1410578
milestone62.0a1
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>