Bug 1506792 - Bail out when trying to get getComputedStyle for a non attached node; r=jdescottes
authorPatrick Brosset <pbrosset@mozilla.com>
Mon, 19 Nov 2018 10:55:45 +0000
changeset 503395 aed811658fae595beec6344b597ab8bca2ac9c83
parent 503394 209069890e62b4c402c64be007f9cc992b9ec363
child 503396 0ceae9db9ec0be18daa1a279511ad305723185d4
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdescottes
bugs1506792
milestone65.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 1506792 - Bail out when trying to get getComputedStyle for a non attached node; r=jdescottes Differential Revision: https://phabricator.services.mozilla.com/D12117
devtools/client/inspector/markup/test/browser.ini
devtools/client/inspector/markup/test/browser_markup_pseudo_on_reload.js
devtools/client/inspector/markup/test/doc_markup_pseudo.html
devtools/server/actors/inspector/css-logic.js
--- a/devtools/client/inspector/markup/test/browser.ini
+++ b/devtools/client/inspector/markup/test/browser.ini
@@ -27,16 +27,17 @@ support-files =
   doc_markup_image_and_canvas.html
   doc_markup_image_and_canvas_2.html
   doc_markup_links.html
   doc_markup_mutation.html
   doc_markup_navigation.html
   doc_markup_not_displayed.html
   doc_markup_pagesize_01.html
   doc_markup_pagesize_02.html
+  doc_markup_pseudo.html
   doc_markup_search.html
   doc_markup_svg_attributes.html
   doc_markup_toggle.html
   doc_markup_tooltip.png
   doc_markup_void_elements.html
   doc_markup_void_elements.xhtml
   doc_markup_whitespace.html
   doc_markup_xul.xul
@@ -162,16 +163,17 @@ skip-if = verify
 [browser_markup_mutation_02.js]
 [browser_markup_navigation.js]
 [browser_markup_node_names.js]
 [browser_markup_node_names_namespaced.js]
 [browser_markup_node_not_displayed_01.js]
 [browser_markup_node_not_displayed_02.js]
 [browser_markup_pagesize_01.js]
 [browser_markup_pagesize_02.js]
+[browser_markup_pseudo_on_reload.js]
 [browser_markup_remove_xul_attributes.js]
 [browser_markup_search_01.js]
 [browser_markup_shadowdom.js]
 [browser_markup_shadowdom_clickreveal.js]
 [browser_markup_shadowdom_clickreveal_scroll.js]
 [browser_markup_shadowdom_copy_paths.js]
 subsuite = clipboard
 [browser_markup_shadowdom_delete.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/markup/test/browser_markup_pseudo_on_reload.js
@@ -0,0 +1,39 @@
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test that reloading the page when an element with sibling pseudo elements is selected
+// does not result in missing elements in the markup-view after reload.
+// Non-regression test for bug 1506792.
+
+const TEST_URL = URL_ROOT + "doc_markup_pseudo.html";
+
+add_task(async function() {
+  const { inspector, testActor } = await openInspectorForURL(TEST_URL);
+
+  await selectNode("div", inspector);
+
+  info("Check that the markup-view shows the expected nodes before reload");
+  await checkMarkupView(inspector);
+
+  await reloadPage(inspector, testActor);
+
+  info("Check that the markup-view shows the expected nodes after reload");
+  await checkMarkupView(inspector);
+});
+
+async function checkMarkupView(inspector) {
+  const articleContainer = await getContainerForSelector("article", inspector);
+  ok(articleContainer, "The parent <article> element was found");
+
+  const childrenContainers = articleContainer.getChildContainers();
+  const beforeNode = childrenContainers[0].node;
+  const divNode = childrenContainers[1].node;
+  const afterNode = childrenContainers[2].node;
+
+  ok(beforeNode.isBeforePseudoElement, "The first child is the ::before pseudo element");
+  is(divNode.displayName, "div", "The second child is the <div> element");
+  ok(afterNode.isAfterPseudoElement, "The last child is the ::after pseudo element");
+}
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/markup/test/doc_markup_pseudo.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<style>
+article::before,
+article::after {
+  content: "";
+}
+</style>
+<article>
+  <div>test node</div>
+</article>
--- a/devtools/server/actors/inspector/css-logic.js
+++ b/devtools/server/actors/inspector/css-logic.js
@@ -672,16 +672,26 @@ CssLogic.getComputedStyle = function(nod
   if (!node ||
       Cu.isDeadWrapper(node) ||
       node.nodeType !== nodeConstants.ELEMENT_NODE ||
       !node.ownerGlobal) {
     return null;
   }
 
   const {bindingElement, pseudo} = CssLogic.getBindingElementAndPseudo(node);
+
+  // For reasons that still escape us, pseudo-elements can sometimes be "unattached" (i.e.
+  // not have a parentNode defined). This seems to happen when a page is reloaded while
+  // the inspector is open. Bailing out here ensures that the inspector does not fail at
+  // presenting DOM nodes and CSS styles when this happens. This is a temporary measure.
+  // See bug 1506792.
+  if (!bindingElement) {
+    return null;
+  }
+
   return node.ownerGlobal.getComputedStyle(bindingElement, pseudo);
 };
 
 /**
  * Get a source for a stylesheet, taking into account embedded stylesheets
  * for which we need to use document.defaultView.location.href rather than
  * sheet.href
  *