Bug 1277348 - Fix support of document.open against the inspector. r=pbro draft
authorAlexandre Poirot <poirot.alex@gmail.com>
Thu, 13 Oct 2016 07:50:18 -0700
changeset 427803 760859bd5c4944c40adffdafa236ba81770c3fc4
parent 427802 955d6d6a7013b430d7b200afea211e9350cd0f1b
child 427804 f140a6cf9c9199fe4c20d0c86a66a5b860f86e9a
push id33114
push userbmo:poirot.alex@gmail.com
push dateThu, 20 Oct 2016 21:37:37 +0000
reviewerspbro
bugs1277348
milestone52.0a1
Bug 1277348 - Fix support of document.open against the inspector. r=pbro MozReview-Commit-ID: FFUB3RaEtPd
devtools/client/inspector/test/browser.ini
devtools/client/inspector/test/browser_inspector_open_with_document_open.js
devtools/server/actors/inspector.js
devtools/shared/specs/inspector.js
--- a/devtools/client/inspector/test/browser.ini
+++ b/devtools/client/inspector/test/browser.ini
@@ -128,16 +128,17 @@ subsuite = clipboard
 [browser_inspector_menu-03-paste-items.js]
 subsuite = clipboard
 [browser_inspector_menu-03-paste-items-svg.js]
 subsuite = clipboard
 [browser_inspector_menu-04-use-in-console.js]
 [browser_inspector_menu-05-attribute-items.js]
 [browser_inspector_menu-06-other.js]
 [browser_inspector_navigation.js]
+[browser_inspector_open_with_document_open.js]
 [browser_inspector_pane-toggle-01.js]
 [browser_inspector_pane-toggle-02.js]
 [browser_inspector_pane-toggle-03.js]
 [browser_inspector_pane-toggle-05.js]
 skip-if = os == "mac" # Full keyboard navigation on OSX only works if Full Keyboard Access setting is set to All Control in System Keyboard
 [browser_inspector_picker-stop-on-destroy.js]
 [browser_inspector_picker-stop-on-tool-change.js]
 [browser_inspector_portrait_mode.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/test/browser_inspector_open_with_document_open.js
@@ -0,0 +1,34 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+"use strict";
+
+// Test that inspector loads correctly when you navigate to a document calling
+// document.open() without calling document.close() after load event.
+
+const TEST_URL_1 = "data:text/html,charset=urf-8;" +
+  "<script id=\"test-window-open\">new " + function () {
+    window.onload = function () {
+      setTimeout(function () {
+        document.open();
+      });
+    };
+  } + "</script>";
+
+const TEST_URL_2 = "data:text/html,charset=utf-8;" +
+  "<div id=\"test-window-open2\" />";
+
+add_task(function* () {
+  // Test opening the inspector on such document
+  yield addTab(TEST_URL_1);
+  let { inspector } = yield openInspector();
+  ok(true, "Inspector loaded on the already opened document");
+
+  // Verify that the markup-view has the right document nodes
+  yield selectNode("#test-window-open", inspector);
+
+  // Test navigating to a patch without document.open
+  yield navigateTo(inspector, TEST_URL_2);
+
+  yield selectNode("#test-window-open2", inspector);
+});
--- a/devtools/server/actors/inspector.js
+++ b/devtools/server/actors/inspector.js
@@ -1087,19 +1087,28 @@ var WalkerActor = protocol.ActorClassWit
   /**
    * Return the documentElement for the document containing the
    * given node.
    * @param NodeActor node
    *        The node whose documentElement is requested, or null
    *        to use the root document.
    */
   documentElement: function (node) {
-    let elt = isNodeDead(node)
-              ? this.rootDoc.documentElement
-              : nodeDocument(node.rawNode).documentElement;
+    let elt;
+    if (isNodeDead(node)) {
+      // In a loading document, the documentElement may not be available yet,
+      // or we may still refer to the previously loaded document, which is dead
+      if (!this.rootDoc || Cu.isDeadWrapper(this.rootDoc) ||
+          !this.rootDoc.documentElement) {
+        return null;
+      }
+      elt = this.rootDoc.documentElement;
+    } else {
+      elt = nodeDocument(node.rawNode).documentElement;
+    }
     return this._ref(elt);
   },
 
   /**
    * Return all parents of the given node, ordered from immediate parent
    * to root.
    * @param NodeActor node
    *    The node whose parents are requested.
@@ -2621,34 +2630,23 @@ exports.InspectorActor = protocol.ActorC
   getWalker: function (options = {}) {
     if (this._walkerPromise) {
       return this._walkerPromise;
     }
 
     let deferred = promise.defer();
     this._walkerPromise = deferred.promise;
 
-    let window = this.window;
-    let domReady = () => {
-      let tabActor = this.tabActor;
-      window.removeEventListener("DOMContentLoaded", domReady, true);
-      this.walker = WalkerActor(this.conn, tabActor, options);
-      this.manage(this.walker);
-      events.once(this.walker, "destroyed", () => {
-        this._walkerPromise = null;
-        this._pageStylePromise = null;
-      });
-      deferred.resolve(this.walker);
-    };
-
-    if (window.document.readyState === "loading") {
-      window.addEventListener("DOMContentLoaded", domReady, true);
-    } else {
-      domReady();
-    }
+    this.walker = WalkerActor(this.conn, this.tabActor, options);
+    this.manage(this.walker);
+    events.once(this.walker, "destroyed", () => {
+      this._walkerPromise = null;
+      this._pageStylePromise = null;
+    });
+    deferred.resolve(this.walker);
 
     return this._walkerPromise;
   },
 
   getPageStyle: function () {
     if (this._pageStylePromise) {
       return this._pageStylePromise;
     }
--- a/devtools/shared/specs/inspector.js
+++ b/devtools/shared/specs/inspector.js
@@ -148,17 +148,17 @@ const walkerSpec = generateActorSpec({
       request: {node: Arg(0, "nullable:domnode")}
     },
     document: {
       request: { node: Arg(0, "nullable:domnode") },
       response: { node: RetVal("domnode") },
     },
     documentElement: {
       request: { node: Arg(0, "nullable:domnode") },
-      response: { node: RetVal("domnode") },
+      response: { node: RetVal("nullable:domnode") },
     },
     parents: {
       request: {
         node: Arg(0, "domnode"),
         sameDocument: Option(1),
         sameTypeRootTreeItem: Option(1)
       },
       response: {