Bug 1475203: Fix id table handling in UnbindFromTree(). r=smaug
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 12 Jul 2018 08:23:38 +0200
changeset 426312 5654b62b0b11a68bc40c91a1e9764f6ef504563b
parent 426311 21f8f4c25486be1159fa051f90de9bd473d8529d
child 426313 a5e9e9a0ff04aaf15644c8d35c7e7c22251f1419
push id105197
push useremilio@crisal.io
push dateThu, 12 Jul 2018 12:12:46 +0000
treeherdermozilla-inbound@5654b62b0b11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1475203
milestone63.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 1475203: Fix id table handling in UnbindFromTree(). r=smaug Only remove from the table when we're actually disconnecting the element from the shadow root. Differential Revision: https://phabricator.services.mozilla.com/D2092 MozReview-Commit-ID: 1Ubdz57PNc0
dom/base/Element.cpp
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/shadow-dom/getElementById-dynamic-001.html
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -1876,25 +1876,42 @@ RemoveFromBindingManagerRunnable::Run()
   if (!mContent->IsInComposedDoc()) {
     mManager->RemovedFromDocumentInternal(mContent, mDoc,
                                           nsBindingManager::eRunDtor);
   }
 
   return NS_OK;
 }
 
+static bool
+ShouldRemoveFromIdTableOnUnbind(const Element& aElement, bool aNullParent)
+{
+  if (aElement.IsInUncomposedDoc()) {
+    return true;
+  }
+
+  if (!aElement.IsInShadowTree()) {
+    return false;
+  }
+
+  return aNullParent || !aElement.GetParent()->IsInShadowTree();
+}
 
 void
 Element::UnbindFromTree(bool aDeep, bool aNullParent)
 {
   MOZ_ASSERT(aDeep || (!GetUncomposedDoc() && !GetBindingParent()),
                   "Shallow unbind won't clear document and binding parent on "
                   "kids!");
 
-  RemoveFromIdTable();
+  // Make sure to only remove from the ID table if our subtree root is actually
+  // changing.
+  if (ShouldRemoveFromIdTableOnUnbind(*this, aNullParent)) {
+    RemoveFromIdTable();
+  }
 
   // Make sure to unbind this node before doing the kids
   nsIDocument* document = GetComposedDoc();
 
   if (HasPointerLock()) {
     nsIDocument::UnlockPointer();
   }
   if (mState.HasState(NS_EVENT_STATE_FULL_SCREEN)) {
@@ -1926,17 +1943,17 @@ Element::UnbindFromTree(bool aDeep, bool
         nsIContent* parent = GetParent();
         while (parent) {
           parent->ChangeEditableDescendantCount(editableDescendantChange);
           parent = parent->GetParent();
         }
       }
     }
 
-    if (this->IsRootOfNativeAnonymousSubtree()) {
+    if (IsRootOfNativeAnonymousSubtree()) {
       nsNodeUtils::NativeAnonymousChildListChange(this, true);
     }
 
     if (GetParent()) {
       RefPtr<nsINode> p;
       p.swap(mParent);
     } else {
       mParent = nullptr;
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -378592,16 +378592,22 @@
     ]
    ],
    "shadow-dom/form-control-form-attribute.html": [
     [
      "/shadow-dom/form-control-form-attribute.html",
      {}
     ]
    ],
+   "shadow-dom/getElementById-dynamic-001.html": [
+    [
+     "/shadow-dom/getElementById-dynamic-001.html",
+     {}
+    ]
+   ],
    "shadow-dom/historical.html": [
     [
      "/shadow-dom/historical.html",
      {}
     ]
    ],
    "shadow-dom/input-element-list.html": [
     [
@@ -617961,16 +617967,20 @@
   "shadow-dom/event-with-related-target.html": [
    "572ddb9624ba8871d93cb13fad830f1acc8d4cac",
    "testharness"
   ],
   "shadow-dom/form-control-form-attribute.html": [
    "7726f8fe9056d3d5c9fb7b963c4bc6e777a8256a",
    "testharness"
   ],
+  "shadow-dom/getElementById-dynamic-001.html": [
+   "a4b02cd6fc9a2e68a74af141566b863dd55c2b13",
+   "testharness"
+  ],
   "shadow-dom/historical.html": [
    "1469992db34a25397dc3d5a5e1eb600e8afcf71b",
    "testharness"
   ],
   "shadow-dom/input-element-list.html": [
    "fcfa6fee4ecc1391a457880bad3564778a76127a",
    "testharness"
   ],
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/shadow-dom/getElementById-dynamic-001.html
@@ -0,0 +1,23 @@
+<!doctype html>
+<title>Shadow DOM: ShadowRoot.getElementById in shadow trees keeps working after host is removed from tree</title>
+<link rel="help" href="https://dom.spec.whatwg.org/#dom-nonelementparentnode-getelementbyid">
+<link rel="help" href="https://bugzil.la/1475203">
+<link rel="author" name="Emilio Cobos Álvarez" href="emilio@crisal.io">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<div id="host"></div>
+<script>
+test(function() {
+  let host = document.getElementById("host");
+  host.attachShadow({ mode: "open" }).innerHTML = `<div id="test-id"></div>`;
+
+  let element = host.shadowRoot.getElementById("test-id");
+  assert_true(!!element);
+
+  host.remove();
+  assert_equals(host.shadowRoot.getElementById("test-id"), element);
+
+  element.remove();
+  assert_equals(host.shadowRoot.getElementById("test-id"), null);
+}, "ShadowRoot.getElementById keeps working after host has been removed");
+</script>