Bug 1548848 - Moving assigned nodes caused by slot removal should properly invalidate layout; r=emilio
authorEdgar Chen <echen@mozilla.com>
Wed, 08 May 2019 10:39:40 +0000
changeset 531858 d6bc3bde65d6e4f99c5c5cd6be6898801bfd0549
parent 531857 cee398776b835ecb955e0ff7af1a457898808637
child 531859 b771a88efe40227ba5cce8f1880d755018c1871e
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1548848
milestone68.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 1548848 - Moving assigned nodes caused by slot removal should properly invalidate layout; r=emilio Differential Revision: https://phabricator.services.mozilla.com/D30194
dom/base/ShadowRoot.cpp
testing/web-platform/tests/css/css-scoping/shadow-reassign-dynamic-004.html
--- a/dom/base/ShadowRoot.cpp
+++ b/dom/base/ShadowRoot.cpp
@@ -262,38 +262,39 @@ void ShadowRoot::RemoveSlot(HTMLSlotElem
       aSlot->EnqueueSlotChangeEvent();
     }
 
     return;
   }
 
   const bool wasFirstSlot = currentSlots->ElementAt(0) == aSlot;
   currentSlots.RemoveElement(*aSlot);
-
-  // Move assigned nodes from removed slot to the next slot in
-  // tree order with the same name.
   if (!wasFirstSlot) {
     return;
   }
 
+  // Move assigned nodes from removed slot to the next slot in
+  // tree order with the same name.
   InvalidateStyleAndLayoutOnSubtree(aSlot);
   HTMLSlotElement* replacementSlot = currentSlots->ElementAt(0);
   const nsTArray<RefPtr<nsINode>>& assignedNodes = aSlot->AssignedNodes();
-  bool slottedNodesChanged = !assignedNodes.IsEmpty();
+  if (assignedNodes.IsEmpty()) {
+    return;
+  }
+
+  InvalidateStyleAndLayoutOnSubtree(replacementSlot);
   while (!assignedNodes.IsEmpty()) {
     nsINode* assignedNode = assignedNodes[0];
 
     aSlot->RemoveAssignedNode(assignedNode);
     replacementSlot->AppendAssignedNode(assignedNode);
   }
 
-  if (slottedNodesChanged) {
-    aSlot->EnqueueSlotChangeEvent();
-    replacementSlot->EnqueueSlotChangeEvent();
-  }
+  aSlot->EnqueueSlotChangeEvent();
+  replacementSlot->EnqueueSlotChangeEvent();
 }
 
 // FIXME(emilio): There's a bit of code duplication between this and the
 // equivalent ServoStyleSet methods, it'd be nice to not duplicate it...
 void ShadowRoot::RuleAdded(StyleSheet& aSheet, css::Rule& aRule) {
   if (!aSheet.IsApplicable()) {
     return;
   }
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-scoping/shadow-reassign-dynamic-004.html
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS Scoping: Dynamic reassignment of a slot.</title>
+<link rel="author" title="Edgar Chen" href="mailto:echen@mozilla.com">
+<link rel="help" href="https://drafts.csswg.org/css-scoping/#selectors-data-model">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1548848">
+<link rel="match" href="reference/green-box.html"/>
+<div id="host">
+  <div id="green" style="background: green"></div>
+<div>
+<script>
+  let root = host.attachShadow({mode: 'open'});
+  root.innerHTML = `
+    <style>::slotted(div),div { width: 100px; height: 100px }</style>
+    <p>Test passes if you see a single 100px by 100px green box below.</p>
+    <slot id="slot"></slot>
+    <slot>
+      <div style="background: red"></div>
+    </slot>
+  `;
+
+  onload = function () {
+    root.offsetTop; // Update layout
+    root.getElementById('slot').remove();
+  };
+</script>
\ No newline at end of file