Bug 353575 - Allow IDTracker to look up elements in <svg:use> shadow trees. r=smaug
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 27 Apr 2020 21:15:18 +0000
changeset 526338 8efff752175e177108adceddb73a60f98f8cdd27
parent 526337 7690541a2c14907acc3103d66a96d0d618a64c87
child 526339 2b2c615bbe262851bfad84cd6e6bb2d46967e867
push id114211
push userealvarez@mozilla.com
push dateMon, 27 Apr 2020 21:17:10 +0000
treeherderautoland@8efff752175e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs353575
milestone77.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 353575 - Allow IDTracker to look up elements in <svg:use> shadow trees. r=smaug Other UAs allow this, and it seems in the general consensus of https://github.com/w3c/webcomponents/issues/179. This matches WebKit's behavior. Blink, for some reason shows red on the test-case, probably because they're not doing quite this, but they manage to render masks inside the display: none symbol element or such. Differential Revision: https://phabricator.services.mozilla.com/D72610
dom/base/IDTracker.cpp
layout/reftests/svg/fragid-shadow-7.html
layout/reftests/svg/fragid-shadow-8.html
--- a/dom/base/IDTracker.cpp
+++ b/dom/base/IDTracker.cpp
@@ -2,30 +2,49 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "IDTracker.h"
 
 #include "mozilla/Encoding.h"
+#include "mozilla/dom/DocumentOrShadowRoot.h"
+#include "nsAtom.h"
 #include "nsContentUtils.h"
 #include "nsIURI.h"
 #include "nsIReferrerInfo.h"
 #include "nsEscape.h"
 #include "nsCycleCollectionParticipant.h"
+#include "nsStringFwd.h"
 
 namespace mozilla {
 namespace dom {
 
-static DocumentOrShadowRoot* DocOrShadowFromContent(nsIContent& aContent) {
+static Element* LookupElement(DocumentOrShadowRoot& aDocOrShadow,
+                              const nsAString& aRef, bool aReferenceImage) {
+  if (aReferenceImage) {
+    return aDocOrShadow.LookupImageElement(aRef);
+  }
+  return aDocOrShadow.GetElementById(aRef);
+}
+
+static DocumentOrShadowRoot* FindTreeToWatch(nsIContent& aContent,
+                                             const nsAString& aID,
+                                             bool aReferenceImage) {
   ShadowRoot* shadow = aContent.GetContainingShadow();
 
-  // We never look in <svg:use> shadow trees, for backwards compat.
+  // We allow looking outside an <svg:use> shadow tree for backwards compat.
   while (shadow && shadow->Host()->IsSVGElement(nsGkAtoms::use)) {
+    // <svg:use> shadow trees are immutable, so we can just early-out if we find
+    // our relevant element instead of having to support watching multiple
+    // trees.
+    if (LookupElement(*shadow, aID, aReferenceImage)) {
+      return shadow;
+    }
     shadow = shadow->Host()->GetContainingShadow();
   }
 
   if (shadow) {
     return shadow;
   }
 
   return aContent.OwnerDoc();
@@ -44,17 +63,16 @@ void IDTracker::ResetToURIFragmentID(nsI
   nsAutoCString refPart;
   aURI->GetRef(refPart);
   // Unescape %-escapes in the reference. The result will be in the
   // document charset, hopefully...
   NS_UnescapeURL(refPart);
 
   // Get the thing to observe changes to.
   Document* doc = aFromContent->OwnerDoc();
-  DocumentOrShadowRoot* docOrShadow = DocOrShadowFromContent(*aFromContent);
   auto encoding = doc->GetDocumentCharacterSet();
 
   nsAutoString ref;
   nsresult rv = encoding->DecodeWithoutBOMHandling(refPart, ref);
   if (NS_FAILED(rv) || ref.IsEmpty()) {
     return;
   }
 
@@ -70,16 +88,17 @@ void IDTracker::ResetToURIFragmentID(nsI
       // We don't have watching working yet for anonymous content, so bail out
       // here.
       return;
     }
   }
 
   bool isEqualExceptRef;
   rv = aURI->EqualsExceptRef(doc->GetDocumentURI(), &isEqualExceptRef);
+  DocumentOrShadowRoot* docOrShadow;
   if (NS_FAILED(rv) || !isEqualExceptRef) {
     RefPtr<Document::ExternalResourceLoad> load;
     doc = doc->RequestExternalResource(aURI, aReferrerInfo, aFromContent,
                                        getter_AddRefs(load));
     docOrShadow = doc;
     if (!doc) {
       if (!load || !aWatch) {
         // Nothing will ever happen here
@@ -87,16 +106,18 @@ void IDTracker::ResetToURIFragmentID(nsI
       }
 
       DocumentLoadNotification* observer =
           new DocumentLoadNotification(this, ref);
       mPendingNotification = observer;
       load->AddObserver(observer);
       // Keep going so we set up our watching stuff a bit
     }
+  } else {
+    docOrShadow = FindTreeToWatch(*aFromContent, ref, aReferenceImage);
   }
 
   if (aWatch) {
     mWatchID = NS_Atomize(ref);
   }
 
   mReferencingImage = aReferenceImage;
   HaveNewDocumentOrShadowRoot(docOrShadow, aWatch, ref);
@@ -106,18 +127,20 @@ void IDTracker::ResetWithID(Element& aFr
   MOZ_ASSERT(aID);
 
   if (aWatch) {
     mWatchID = aID;
   }
 
   mReferencingImage = false;
 
-  DocumentOrShadowRoot* docOrShadow = DocOrShadowFromContent(aFrom);
-  HaveNewDocumentOrShadowRoot(docOrShadow, aWatch, nsDependentAtomString(aID));
+  nsDependentAtomString str(aID);
+  DocumentOrShadowRoot* docOrShadow =
+      FindTreeToWatch(aFrom, str, /* aReferenceImage = */ false);
+  HaveNewDocumentOrShadowRoot(docOrShadow, aWatch, str);
 }
 
 void IDTracker::HaveNewDocumentOrShadowRoot(DocumentOrShadowRoot* aDocOrShadow,
                                             bool aWatch, const nsString& aRef) {
   if (aWatch) {
     mWatchDocumentOrShadowRoot = nullptr;
     if (aDocOrShadow) {
       mWatchDocumentOrShadowRoot = &aDocOrShadow->AsNode();
@@ -126,19 +149,17 @@ void IDTracker::HaveNewDocumentOrShadowR
     }
     return;
   }
 
   if (!aDocOrShadow) {
     return;
   }
 
-  Element* e = mReferencingImage ? aDocOrShadow->LookupImageElement(aRef)
-                                 : aDocOrShadow->GetElementById(aRef);
-  if (e) {
+  if (Element* e = LookupElement(*aDocOrShadow, aRef, mReferencingImage)) {
     mElement = e;
   }
 }
 
 void IDTracker::Traverse(nsCycleCollectionTraversalCallback* aCB) {
   NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(*aCB, "mWatchDocumentOrShadowRoot");
   aCB->NoteXPCOMChild(mWatchDocumentOrShadowRoot);
   NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(*aCB, "mElement");
@@ -184,17 +205,17 @@ bool IDTracker::Observe(Element* aOldEle
 
 NS_IMPL_ISUPPORTS_INHERITED0(IDTracker::ChangeNotification, mozilla::Runnable)
 NS_IMPL_ISUPPORTS(IDTracker::DocumentLoadNotification, nsIObserver)
 
 NS_IMETHODIMP
 IDTracker::DocumentLoadNotification::Observe(nsISupports* aSubject,
                                              const char* aTopic,
                                              const char16_t* aData) {
-  NS_ASSERTION(PL_strcmp(aTopic, "external-resource-document-created") == 0,
+  NS_ASSERTION(!strcmp(aTopic, "external-resource-document-created"),
                "Unexpected topic");
   if (mTarget) {
     nsCOMPtr<Document> doc = do_QueryInterface(aSubject);
     mTarget->mPendingNotification = nullptr;
     NS_ASSERTION(!mTarget->mElement, "Why do we have content here?");
     // If we got here, that means we had Reset*() called with
     // aWatch == true.  So keep watching if IsPersistent().
     mTarget->HaveNewDocumentOrShadowRoot(doc, mTarget->IsPersistent(), mRef);
--- a/layout/reftests/svg/fragid-shadow-7.html
+++ b/layout/reftests/svg/fragid-shadow-7.html
@@ -8,21 +8,21 @@
 </svg>
 <div id="host"></div>
 <script>
   // Test references from a <svg:use> subtree.
   host.attachShadow({ mode: "open" }).innerHTML = `
     <svg width="100" height="100">
       <defs>
         <pattern id="rect" width="100" height="100">
-          <rect fill="lime" width="100" height="100" />
+          <rect fill="red" width="100" height="100" />
         </pattern>
         <symbol id="useme">
           <pattern id="rect" width="100" height="100">
-            <rect fill="red" width="100" height="100" />
+            <rect fill="lime" width="100" height="100" />
           </pattern>
           <rect fill="url(#rect)" width="100" height="100" />
         </symbol>
       </defs>
       <use href="#useme" />
     </svg>
   `;
 </script>
--- a/layout/reftests/svg/fragid-shadow-8.html
+++ b/layout/reftests/svg/fragid-shadow-8.html
@@ -1,15 +1,15 @@
 <!doctype html>
 <svg width="100" height="100">
   <defs>
     <pattern id="rect" width="100" height="100">
-      <rect fill="lime" width="100" height="100" />
+      <rect fill="red" width="100" height="100" />
     </pattern>
     <symbol id="useme">
       <pattern id="rect" width="100" height="100">
-        <rect fill="red" width="100" height="100" />
+        <rect fill="lime" width="100" height="100" />
       </pattern>
       <rect fill="url(#rect)" width="100" height="100" />
     </symbol>
   </defs>
   <use href="#useme" />
 </svg>