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 WICG/webcomponents#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
xeonchen · Apr 27, 2020 · 1373412 · 1373412
1 parent 4b27205
commit 1373412
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 13 deletions.
diff --git a/dom/base/IDTracker.cpp b/dom/base/IDTracker.cpp
@@ -7,20 +7,39 @@
 #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();
   }
 
@@ -49,7 +68,6 @@ void IDTracker::ResetToURIFragmentID(nsIContent* aFromContent, nsIURI* aURI,
 
   // Get the thing to observe changes to.
   Document* doc = aFromContent->OwnerDoc();
-  DocumentOrShadowRoot* docOrShadow = DocOrShadowFromContent(*aFromContent);
   auto encoding = doc->GetDocumentCharacterSet();
 
   nsAutoString ref;
@@ -75,6 +93,7 @@ void IDTracker::ResetToURIFragmentID(nsIContent* aFromContent, nsIURI* aURI,
 
   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,
@@ -92,6 +111,8 @@ void IDTracker::ResetToURIFragmentID(nsIContent* aFromContent, nsIURI* aURI,
       load->AddObserver(observer);
       // Keep going so we set up our watching stuff a bit
     }
+  } else {
+    docOrShadow = FindTreeToWatch(*aFromContent, ref, aReferenceImage);
   }
 
   if (aWatch) {
@@ -111,8 +132,10 @@ void IDTracker::ResetWithID(Element& aFrom, nsAtom* aID, bool aWatch) {
 
   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,
@@ -131,9 +154,7 @@ void IDTracker::HaveNewDocumentOrShadowRoot(DocumentOrShadowRoot* aDocOrShadow,
     return;
   }
 
-  Element* e = mReferencingImage ? aDocOrShadow->LookupImageElement(aRef)
-                                 : aDocOrShadow->GetElementById(aRef);
-  if (e) {
+  if (Element* e = LookupElement(*aDocOrShadow, aRef, mReferencingImage)) {
     mElement = e;
   }
 }
@@ -189,7 +210,7 @@ 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);

diff --git a/layout/reftests/svg/fragid-shadow-7.html b/layout/reftests/svg/fragid-shadow-7.html
@@ -13,11 +13,11 @@
     <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>

diff --git a/layout/reftests/svg/fragid-shadow-8.html b/layout/reftests/svg/fragid-shadow-8.html
@@ -2,11 +2,11 @@
 <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>