Bug 1151909 - Make the highlighter work on DOMContentLoaded instead of load. r=pbro draft
authorAlexandre Poirot <poirot.alex@gmail.com>
Tue, 22 Nov 2016 07:21:24 -0800
changeset 444600 5947fd8a6f08e5c90213b8f44a3661962ed1e926
parent 444599 0d1d2b60f9cd347b8ac04cd3e8a0104be6ced00c
child 538329 66320113677a5d1235f9b1aa6b5957c828adf7f9
push id37302
push userbmo:poirot.alex@gmail.com
push dateMon, 28 Nov 2016 09:49:58 +0000
reviewerspbro
bugs1151909
milestone53.0a1
Bug 1151909 - Make the highlighter work on DOMContentLoaded instead of load. r=pbro MozReview-Commit-ID: FyXEvKAgDJq
devtools/server/actors/highlighters.js
devtools/server/actors/highlighters/utils/markup.js
--- a/devtools/server/actors/highlighters.js
+++ b/devtools/server/actors/highlighters.js
@@ -521,38 +521,40 @@ var CustomHighlighterActor = exports.Cus
       this._highlighterEnv = null;
     }
   }
 });
 
 /**
  * The HighlighterEnvironment is an object that holds all the required data for
  * highlighters to work: the window, docShell, event listener target, ...
- * It also emits "will-navigate" and "navigate" events, similarly to the
- * TabActor.
+ * It also emits "will-navigate", "navigate" and "window-ready" events,
+ * similarly to the TabActor.
  *
  * It can be initialized either from a TabActor (which is the most frequent way
  * of using it, since highlighters are usually initialized by the
  * HighlighterActor or CustomHighlighterActor, which have a tabActor reference).
  * It can also be initialized just with a window object (which is useful for
  * when a highlighter is used outside of the debugger server context, for
  * instance from a gcli command).
  */
 function HighlighterEnvironment() {
+  this.relayTabActorWindowReady = this.relayTabActorWindowReady.bind(this);
   this.relayTabActorNavigate = this.relayTabActorNavigate.bind(this);
   this.relayTabActorWillNavigate = this.relayTabActorWillNavigate.bind(this);
 
   EventEmitter.decorate(this);
 }
 
 exports.HighlighterEnvironment = HighlighterEnvironment;
 
 HighlighterEnvironment.prototype = {
   initFromTabActor: function (tabActor) {
     this._tabActor = tabActor;
+    events.on(this._tabActor, "window-ready", this.relayTabActorWindowReady);
     events.on(this._tabActor, "navigate", this.relayTabActorNavigate);
     events.on(this._tabActor, "will-navigate", this.relayTabActorWillNavigate);
   },
 
   initFromWindow: function (win) {
     this._win = win;
 
     // We need a progress listener to know when the window will navigate/has
@@ -644,26 +646,31 @@ HighlighterEnvironment.prototype = {
    */
   get pageListenerTarget() {
     if (this._tabActor && this._tabActor.isRootActor) {
       return this.window;
     }
     return this.docShell.chromeEventHandler;
   },
 
+  relayTabActorWindowReady: function (data) {
+    this.emit("window-ready", data);
+  },
+
   relayTabActorNavigate: function (data) {
     this.emit("navigate", data);
   },
 
   relayTabActorWillNavigate: function (data) {
     this.emit("will-navigate", data);
   },
 
   destroy: function () {
     if (this._tabActor) {
+      events.off(this._tabActor, "window-ready", this.relayTabActorWindowReady);
       events.off(this._tabActor, "navigate", this.relayTabActorNavigate);
       events.off(this._tabActor, "will-navigate", this.relayTabActorWillNavigate);
     }
 
     // In case the environment was initialized from a window, we need to remove
     // the progress listener.
     if (this._win) {
       try {
--- a/devtools/server/actors/highlighters/utils/markup.js
+++ b/devtools/server/actors/highlighters/utils/markup.js
@@ -238,54 +238,56 @@ function CanvasFrameAnonymousContentHelp
   this.highlighterEnv = highlighterEnv;
   this.nodeBuilder = nodeBuilder;
   this.anonymousContentDocument = this.highlighterEnv.document;
   // XXX the next line is a wallpaper for bug 1123362.
   this.anonymousContentGlobal = Cu.getGlobalForObject(
                                 this.anonymousContentDocument);
 
   // Only try to create the highlighter when the document is loaded,
-  // otherwise, wait for the navigate event to fire.
+  // otherwise, wait for the window-ready event to fire.
   let doc = this.highlighterEnv.document;
   if (doc.documentElement && doc.readyState != "uninitialized") {
     this._insert();
   }
 
-  this._onNavigate = this._onNavigate.bind(this);
-  this.highlighterEnv.on("navigate", this._onNavigate);
+  this._onWindowReady= this._onWindowReady.bind(this);
+  this.highlighterEnv.on("window-ready", this._onWindowReady);
 
   this.listeners = new Map();
 }
 
 CanvasFrameAnonymousContentHelper.prototype = {
   destroy: function () {
     try {
       let doc = this.anonymousContentDocument;
       doc.removeAnonymousContent(this._content);
     } catch (e) {
       // If the current window isn't the one the content was inserted into, this
       // will fail, but that's fine.
     }
-    this.highlighterEnv.off("navigate", this._onNavigate);
+    this.highlighterEnv.off("window-ready", this._onWindowReady);
     this.highlighterEnv = this.nodeBuilder = this._content = null;
     this.anonymousContentDocument = null;
     this.anonymousContentGlobal = null;
 
     this._removeAllListeners();
   },
 
   _insert: function () {
     let doc = this.highlighterEnv.document;
-    // Insert the content node only if the document:
-    // * is loaded (navigate event will fire once it is),
-    // * still exists,
-    // * isn't in XUL.
-    if (doc.readyState == "uninitialized" ||
-        !doc.documentElement ||
-        isXUL(this.highlighterEnv.window)) {
+    // Wait for DOMContentLoaded before injecting the anonymous content.
+    if (doc.readyState != "interactive" && doc.readyState != "complete") {
+      doc.addEventListener("DOMContentLoaded", this._insert.bind(this),
+                           { once: true });
+      return;
+    }
+    // Reject XUL documents. Check that after DOMContentLoaded as we query
+    // documentElement which is only available after this event.
+    if (isXUL(this.highlighterEnv.window)) {
       return;
     }
 
     // For now highlighters.css is injected in content as a ua sheet because
     // <style scoped> doesn't work inside anonymous content (see bug 1086532).
     // If it did, highlighters.css would be injected as an anonymous content
     // node using CanvasFrameAnonymousContentHelper instead.
     installHelperSheet(this.highlighterEnv.window,
@@ -295,17 +297,17 @@ CanvasFrameAnonymousContentHelper.protot
     // It was stated that hidden documents don't accept
     // `insertAnonymousContent` calls yet. That doesn't seems the case anymore,
     // at least on desktop. Therefore, removing the code that was dealing with
     // that scenario, fixes when we're adding anonymous content in a tab that
     // is not the active one (see bug 1260043 and bug 1260044)
     this._content = doc.insertAnonymousContent(node);
   },
 
-  _onNavigate: function (e, {isTopLevel}) {
+  _onWindowReady: function (e, {isTopLevel}) {
     if (isTopLevel) {
       this._removeAllListeners();
       this._insert();
       this.anonymousContentDocument = this.highlighterEnv.document;
     }
   },
 
   getTextContentForElement: function (id) {