Bug 1341756 - Clear any highlighters on window-ready. r=zer0 draft
authorJ. Ryan Stinnett <jryans@gmail.com>
Wed, 08 Mar 2017 16:55:31 -0600
changeset 495537 5a4f04e4e19d251db2129dabc92eee060c30338d
parent 495323 800ba54a4bd52628833c4db005ddd182586666c4
child 548399 00683ad6fa0b299383c45988c0f8c963bec81b98
push id48356
push userbmo:jryans@gmail.com
push dateWed, 08 Mar 2017 22:57:52 +0000
reviewerszer0
bugs1341756
milestone55.0a1
Bug 1341756 - Clear any highlighters on window-ready. r=zer0 By always clearing any possible highlighters first, it helps handle the case of toggling RDM, which does trigger `window-ready` even though the old highlighter content is still live on the page. MozReview-Commit-ID: FCRHW1AyiyB
devtools/server/actors/highlighters/utils/markup.js
devtools/server/actors/tab.js
--- a/devtools/server/actors/highlighters/utils/markup.js
+++ b/devtools/server/actors/highlighters/utils/markup.js
@@ -248,23 +248,17 @@ function CanvasFrameAnonymousContentHelp
   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._remove();
     this.highlighterEnv.off("window-ready", this._onWindowReady);
     this.highlighterEnv = this.nodeBuilder = this._content = null;
     this.anonymousContentDocument = null;
     this.anonymousContentGlobal = null;
 
     this._removeAllListeners();
   },
 
@@ -301,18 +295,36 @@ 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);
   },
 
+  _remove() {
+    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.
+    }
+  },
+
+  /**
+   * The "window-ready" event can be triggered when:
+   *   - a new window is created
+   *   - a window is unfrozen from bfcache
+   *   - when first attaching to a page
+   *   - when swapping frame loaders (moving tabs, toggling RDM)
+   */
   _onWindowReady: function (e, {isTopLevel}) {
     if (isTopLevel) {
+      this._remove();
       this._removeAllListeners();
       this._insert();
       this.anonymousContentDocument = this.highlighterEnv.document;
     }
   },
 
   getTextContentForElement: function (id) {
     if (!this.content) {
--- a/devtools/server/actors/tab.js
+++ b/devtools/server/actors/tab.js
@@ -147,25 +147,27 @@ function getInnerId(window) {
  * iframes).
  *  - will-navigate
  *    This event fires once navigation starts.
  *    All pending user prompts are dealt with,
  *    but it is fired before the first request starts.
  *  - navigate
  *    This event is fired once the document's readyState is "complete".
  *  - window-ready
- *    This event is fired on three distinct scenarios:
+ *    This event is fired in various distinct scenarios:
  *     * When a new Window object is crafted, equivalent of `DOMWindowCreated`.
  *       It is dispatched before any page script is executed.
  *     * We will have already received a window-ready event for this window
  *       when it was created, but we received a window-destroyed event when
  *       it was frozen into the bfcache, and now the user navigated back to
  *       this page, so it's now live again and we should resume handling it.
  *     * For each existing document, when an `attach` request is received.
  *       At this point scripts in the page will be already loaded.
+ *     * When `swapFrameLoaders` is used, such as with moving tabs between
+ *       windows or toggling Responsive Design Mode.
  *  - window-destroyed
  *    This event is fired in two cases:
  *     * When the window object is destroyed, i.e. when the related document
  *       is garbage collected. This can happen when the tab is closed or the
  *       iframe is removed from the DOM.
  *       It is equivalent of `inner-window-destroyed` event.
  *     * When the page goes into the bfcache and gets frozen.
  *       The equivalent of `pagehide`.