Bug 1378306 - Force update in grid-inspector's reflow handler when nodes have been removed; r=gl draft
authorPatrick Brosset <pbrosset@mozilla.com>
Thu, 13 Jul 2017 14:29:53 +0200
changeset 610591 a18c47c41761bff4b74382128a9a0d86edead32a
parent 610590 e82dc917bee91bbef5e12dd333c94806599dbe3f
child 637908 c3cc2e21555aade582bb40c814fdbc5588273d77
push id68946
push userbmo:pbrosset@mozilla.com
push dateTue, 18 Jul 2017 14:41:59 +0000
reviewersgl
bugs1378306
milestone56.0a1
Bug 1378306 - Force update in grid-inspector's reflow handler when nodes have been removed; r=gl MozReview-Commit-ID: 45fHGmuKhkD
devtools/client/inspector/grids/grid-inspector.js
devtools/client/inspector/grids/test/browser.ini
devtools/client/inspector/grids/test/browser_grids_grid-list-on-iframe-reloaded.js
devtools/client/inspector/grids/test/doc_iframe_reloaded.html
--- a/devtools/client/inspector/grids/grid-inspector.js
+++ b/devtools/client/inspector/grids/grid-inspector.js
@@ -295,16 +295,17 @@ GridInspector.prototype = {
         }
       }
 
       let fallbackColor = GRID_COLORS[i % GRID_COLORS.length];
       let color = this.getInitialGridColor(nodeFront, fallbackColor);
 
       grids.push({
         id: i,
+        actorID: grid.actorID,
         color,
         gridFragments: grid.gridFragments,
         highlighted: nodeFront == this.highlighters.gridHighlighterShown,
         nodeFront,
       });
     }
 
     this.store.dispatch(updateGrids(grids));
@@ -405,18 +406,28 @@ GridInspector.prototype = {
     try {
       newGridFronts = yield this.layoutInspector.getAllGrids(this.walker.rootNode);
     } catch (e) {
       // This call might fail if called asynchrously after the toolbox is finished
       // closing.
       return;
     }
 
-    // Compare the list of DOM nodes which define these grids.
+    // Get the node front(s) from the current grid(s) so we can compare them to them to
+    // node(s) of the new grids.
     const oldNodeFronts = grids.map(grid => grid.nodeFront.actorID);
+
+    // In some cases, the nodes for current grids may have been removed from the DOM in
+    // which case we need to update.
+    if (grids.length && grids.some(grid => !grid.nodeFront.actorID)) {
+      this.updateGridPanel(newGridFronts);
+      return;
+    }
+
+    // Otherwise, continue comparing with the new grids.
     const newNodeFronts = newGridFronts.filter(grid => grid.containerNodeFront)
                                        .map(grid => grid.containerNodeFront.actorID);
     if (grids.length === newGridFronts.length &&
         oldNodeFronts.sort().join(",") == newNodeFronts.sort().join(",")) {
       // Same list of containers, but let's check if the geometry of the current grid has
       // changed, if it hasn't we can safely abort.
       if (!this.highlighters.gridHighlighterShown ||
           (this.highlighters.gridHighlighterShown &&
--- a/devtools/client/inspector/grids/test/browser.ini
+++ b/devtools/client/inspector/grids/test/browser.ini
@@ -1,12 +1,13 @@
 [DEFAULT]
 tags = devtools
 subsuite = devtools
 support-files =
+  doc_iframe_reloaded.html
   head.js
   !/devtools/client/commandline/test/helpers.js
   !/devtools/client/framework/test/shared-head.js
   !/devtools/client/inspector/test/head.js
   !/devtools/client/inspector/test/shared-head.js
   !/devtools/client/shared/test/test-actor.js
   !/devtools/client/shared/test/test-actor-registry.js
   !/devtools/client/framework/test/shared-redux-head.js
@@ -15,16 +16,17 @@ support-files =
 [browser_grids_color-in-rules-grid-toggle.js]
 [browser_grids_display-setting-extend-grid-lines.js]
 [browser_grids_display-setting-show-grid-line-numbers.js]
 [browser_grids_display-setting-show-grid-areas.js]
 [browser_grids_grid-list-color-picker-on-ESC.js]
 [browser_grids_grid-list-color-picker-on-RETURN.js]
 [browser_grids_grid-list-element-rep.js]
 [browser_grids_grid-list-no-grids.js]
+[browser_grids_grid-list-on-iframe-reloaded.js]
 [browser_grids_grid-list-on-mutation-element-added.js]
 [browser_grids_grid-list-on-mutation-element-removed.js]
 [browser_grids_grid-list-toggle-multiple-grids.js]
 [browser_grids_grid-list-toggle-single-grid.js]
 [browser_grids_grid-outline-cannot-show-outline.js]
 [browser_grids_grid-outline-highlight-area.js]
 [browser_grids_grid-outline-highlight-cell.js]
 [browser_grids_grid-outline-selected-grid.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/grids/test/browser_grids_grid-list-on-iframe-reloaded.js
@@ -0,0 +1,59 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Tests that the list of grids does refresh when an iframe containing a grid is removed
+// and re-created.
+// See bug 1378306 where this happened with jsfiddle.
+
+const TEST_URI = URL_ROOT + "doc_iframe_reloaded.html";
+
+add_task(function* () {
+  yield addTab(TEST_URI);
+
+  const { inspector, gridInspector, testActor } = yield openLayoutView();
+  const { document: doc } = gridInspector;
+  const { store, highlighters } = inspector;
+  const gridList = doc.querySelector("#grid-list");
+
+  info("Clicking on the first checkbox to highlight the grid");
+  yield enableTheFirstGrid(doc, inspector);
+
+  is(gridList.childNodes.length, 1, "There's one grid in the list");
+  let checkbox = gridList.querySelector("input");
+  ok(checkbox.checked, "The checkbox is checked");
+  ok(highlighters.gridHighlighterShown, "There's a highlighter shown");
+
+  info("Reload the iframe in content and expect the grid list to update");
+  const oldGrid = store.getState().grids[0];
+  const onNewListUnchecked = waitUntilState(store, state =>
+    state.grids.length == 1 &&
+    state.grids[0].actorID !== oldGrid.actorID &&
+    !state.grids[0].highlighted);
+  const onHighlighterHidden = highlighters.once("grid-highlighter-hidden");
+  testActor.eval("window.wrappedJSObject.reloadIFrame();");
+  yield onNewListUnchecked;
+  yield onHighlighterHidden;
+
+  is(gridList.childNodes.length, 1, "There's still one grid in the list");
+
+  info("Highlight the first grid again to make sure this still works");
+  yield enableTheFirstGrid(doc, inspector);
+
+  is(gridList.childNodes.length, 1, "There's again one grid in the list");
+  ok(highlighters.gridHighlighterShown, "There's a highlighter shown");
+});
+
+function* enableTheFirstGrid(doc, { highlighters, store }) {
+  const checkbox = doc.querySelector("#grid-list input");
+
+  const onHighlighterShown = highlighters.once("grid-highlighter-shown");
+  const onCheckboxChange = waitUntilState(store, state =>
+    state.grids.length == 1 && state.grids[0].highlighted);
+
+  checkbox.click();
+
+  yield onHighlighterShown;
+  yield onCheckboxChange;
+}
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/grids/test/doc_iframe_reloaded.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<iframe srcdoc="<style>.grid{display:grid;}</style><div class='grid'><span>a</span><span>b</span></div>"></iframe>
+<script>
+"use strict";
+function reloadIFrame() { // eslint-disable-line no-unused-vars
+  const iFrame = document.querySelector("iframe");
+  iFrame.setAttribute("srcdoc", iFrame.getAttribute("srcdoc"));
+}
+</script>