Bug 1327725 - ensure the pagehide event we received are for the window's highlighter; r=gl
Some tests were failing since they navigate in the history of frames; and that
was triggering the `onPageHide` for the highlighter.
MozReview-Commit-ID: 7lGRTg4CerU
--- a/devtools/server/actors/highlighters.js
+++ b/devtools/server/actors/highlighters.js
@@ -1,15 +1,15 @@
/* 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";
-const { Ci } = require("chrome");
+const { Ci, Cu } = require("chrome");
const { XPCOMUtils } = require("resource://gre/modules/XPCOMUtils.jsm");
const EventEmitter = require("devtools/shared/event-emitter");
const events = require("sdk/event/core");
const protocol = require("devtools/shared/protocol");
const Services = require("Services");
const { isWindowIncluded } = require("devtools/shared/layout/utils");
const { highlighterSpec, customHighlighterSpec } = require("devtools/shared/specs/highlighters");
@@ -394,16 +394,21 @@ exports.HighlighterActor = protocol.Acto
target.addEventListener("mouseup", this._preventContentEvent, true);
target.addEventListener("dblclick", this._preventContentEvent, true);
target.addEventListener("keydown", this._onKey, true);
target.addEventListener("keyup", this._preventContentEvent, true);
},
_stopPickerListeners: function () {
let target = this._highlighterEnv.pageListenerTarget;
+
+ if (!target) {
+ return;
+ }
+
target.removeEventListener("mousemove", this._onHovered, true);
target.removeEventListener("click", this._onPick, true);
target.removeEventListener("mousedown", this._preventContentEvent, true);
target.removeEventListener("mouseup", this._preventContentEvent, true);
target.removeEventListener("dblclick", this._preventContentEvent, true);
target.removeEventListener("keydown", this._onKey, true);
target.removeEventListener("keyup", this._preventContentEvent, true);
},
@@ -609,31 +614,35 @@ HighlighterEnvironment.prototype = {
return isXUL(this.window);
},
get window() {
if (!this.isInitialized) {
throw new Error("Initialize HighlighterEnvironment with a tabActor " +
"or window first");
}
- return this._tabActor ? this._tabActor.window : this._win;
+ let win = this._tabActor ? this._tabActor.window : this._win;
+
+ return Cu.isDeadWrapper(win) ? null : win;
},
get document() {
- return this.window.document;
+ return this.window && this.window.document;
},
get docShell() {
- return this.window.QueryInterface(Ci.nsIInterfaceRequestor)
+ return this.window &&
+ this.window.QueryInterface(Ci.nsIInterfaceRequestor)
.getInterface(Ci.nsIWebNavigation)
.QueryInterface(Ci.nsIDocShell);
},
get webProgress() {
- return this.docShell.QueryInterface(Ci.nsIInterfaceRequestor)
+ return this.docShell &&
+ this.docShell.QueryInterface(Ci.nsIInterfaceRequestor)
.getInterface(Ci.nsIWebProgress);
},
/**
* Get the right target for listening to events on the page.
* - If the environment was initialized from a TabActor *and* if we're in the
* Browser Toolbox (to inspect firefox desktop): the tabActor is the
* RootActor, in which case, the window property can be used to listen to
@@ -644,17 +653,17 @@ HighlighterEnvironment.prototype = {
* events, even from nested iframes.
* - If the environment was initialized from a window, we also use the
* chromeEventHandler.
*/
get pageListenerTarget() {
if (this._tabActor && this._tabActor.isRootActor) {
return this.window;
}
- return this.docShell.chromeEventHandler;
+ return this.docShell && this.docShell.chromeEventHandler;
},
relayTabActorWindowReady: function (data) {
this.emit("window-ready", data);
},
relayTabActorNavigate: function (data) {
this.emit("navigate", data);
--- a/devtools/server/actors/highlighters/box-model.js
+++ b/devtools/server/actors/highlighters/box-model.js
@@ -99,19 +99,23 @@ function BoxModelHighlighter(highlighter
this._buildMarkup.bind(this));
/**
* Optionally customize each region's fill color by adding an entry to the
* regionFill property: `highlighter.regionFill.margin = "red";
*/
this.regionFill = {};
+ this.onPageHide = this.onPageHide.bind(this);
this.onWillNavigate = this.onWillNavigate.bind(this);
this.highlighterEnv.on("will-navigate", this.onWillNavigate);
+
+ let { pageListenerTarget } = highlighterEnv;
+ pageListenerTarget.addEventListener("pagehide", this.onPageHide);
}
BoxModelHighlighter.prototype = extend(AutoRefreshHighlighter.prototype, {
typeName: "BoxModelHighlighter",
ID_CLASS_PREFIX: "box-model-",
_buildMarkup: function () {
@@ -255,16 +259,22 @@ BoxModelHighlighter.prototype = extend(A
return highlighterContainer;
},
/**
* Destroy the nodes. Remove listeners.
*/
destroy: function () {
this.highlighterEnv.off("will-navigate", this.onWillNavigate);
+
+ let { pageListenerTarget } = this.highlighterEnv;
+ if (pageListenerTarget) {
+ pageListenerTarget.removeEventListener("pagehide", this.onPageHide);
+ }
+
this.markup.destroy();
AutoRefreshHighlighter.prototype.destroy.call(this);
},
getElement: function (id) {
return this.markup.getElement(this.ID_CLASS_PREFIX + id);
},
@@ -705,15 +715,23 @@ BoxModelHighlighter.prototype = extend(A
*/
_moveInfobar: function () {
let bounds = this._getOuterBounds();
let container = this.getElement("infobar-container");
moveInfobar(container, bounds, this.win);
},
+ onPageHide: function ({ target }) {
+ // If a pagehide event is triggered for current window's highlighter, hide the
+ // highlighter.
+ if (target.defaultView === this.win) {
+ this.hide();
+ }
+ },
+
onWillNavigate: function ({ isTopLevel }) {
if (isTopLevel) {
this.hide();
}
}
});
exports.BoxModelHighlighter = BoxModelHighlighter;
--- a/devtools/server/actors/highlighters/css-grid.js
+++ b/devtools/server/actors/highlighters/css-grid.js
@@ -143,17 +143,17 @@ function CssGridHighlighter(highlighterE
width: 0,
height: 0
};
this.markup = new CanvasFrameAnonymousContentHelper(this.highlighterEnv,
this._buildMarkup.bind(this));
this.onNavigate = this.onNavigate.bind(this);
- this.onPageHide = this.hide.bind(this);
+ this.onPageHide = this.onPageHide.bind(this);
this.onWillNavigate = this.onWillNavigate.bind(this);
this.highlighterEnv.on("navigate", this.onNavigate);
this.highlighterEnv.on("will-navigate", this.onWillNavigate);
let { pageListenerTarget } = highlighterEnv;
pageListenerTarget.addEventListener("pagehide", this.onPageHide);
}
@@ -331,17 +331,19 @@ CssGridHighlighter.prototype = extend(Au
},
destroy() {
let { highlighterEnv } = this;
highlighterEnv.off("navigate", this.onNavigate);
highlighterEnv.off("will-navigate", this.onWillNavigate);
let { pageListenerTarget } = highlighterEnv;
- pageListenerTarget.removeEventListener("pagehide", this.onPageHide);
+ if (pageListenerTarget) {
+ pageListenerTarget.removeEventListener("pagehide", this.onPageHide);
+ }
this.markup.destroy();
// Clear the pattern cache to avoid dead object exceptions (Bug 1342051).
this._clearCache();
AutoRefreshHighlighter.prototype.destroy.call(this);
},
@@ -406,16 +408,24 @@ CssGridHighlighter.prototype = extend(Au
/**
* Called when the page navigates. Used to clear the cached gap patterns and avoid
* using DeadWrapper objects as gap patterns the next time.
*/
onNavigate() {
this._clearCache();
},
+ onPageHide: function ({ target }) {
+ // If a page hide event is triggered for current window's highlighter, hide the
+ // highlighter.
+ if (target.defaultView === this.win) {
+ this.hide();
+ }
+ },
+
onWillNavigate({ isTopLevel }) {
if (isTopLevel) {
this.hide();
}
},
_show() {
if (Services.prefs.getBoolPref(CSS_GRID_ENABLED_PREF) && !this.isGrid()) {
--- a/devtools/server/actors/highlighters/eye-dropper.js
+++ b/devtools/server/actors/highlighters/eye-dropper.js
@@ -173,21 +173,24 @@ EyeDropper.prototype = {
hide() {
if (this.highlighterEnv.isXUL) {
return;
}
this.pageImage = null;
let {pageListenerTarget} = this.highlighterEnv;
- pageListenerTarget.removeEventListener("mousemove", this);
- pageListenerTarget.removeEventListener("click", this, true);
- pageListenerTarget.removeEventListener("keydown", this);
- pageListenerTarget.removeEventListener("DOMMouseScroll", this);
- pageListenerTarget.removeEventListener("FullZoomChange", this);
+
+ if (pageListenerTarget) {
+ pageListenerTarget.removeEventListener("mousemove", this);
+ pageListenerTarget.removeEventListener("click", this, true);
+ pageListenerTarget.removeEventListener("keydown", this);
+ pageListenerTarget.removeEventListener("DOMMouseScroll", this);
+ pageListenerTarget.removeEventListener("FullZoomChange", this);
+ }
this.getElement("root").setAttribute("hidden", "true");
this.getElement("root").removeAttribute("drawn");
this.emit("hidden");
},
prepareImageCapture() {
--- a/devtools/server/actors/highlighters/geometry-editor.js
+++ b/devtools/server/actors/highlighters/geometry-editor.js
@@ -367,38 +367,45 @@ GeometryEditorHighlighter.prototype = ex
// Avoiding exceptions if `destroy` is called multiple times; and / or the
// highlighter environment was already destroyed.
if (!this.highlighterEnv) {
return;
}
let { pageListenerTarget } = this.highlighterEnv;
- DOM_EVENTS.forEach(type =>
- pageListenerTarget.removeEventListener(type, this));
+ if (pageListenerTarget) {
+ DOM_EVENTS.forEach(type =>
+ pageListenerTarget.removeEventListener(type, this));
+ }
AutoRefreshHighlighter.prototype.destroy.call(this);
this.markup.destroy();
this.definedProperties.clear();
this.definedProperties = null;
this.offsetParent = null;
},
handleEvent: function (event, id) {
// No event handling if the highlighter is hidden
if (this.getElement("root").hasAttribute("hidden")) {
return;
}
- const { type, pageX, pageY } = event;
+ const { target, type, pageX, pageY } = event;
switch (type) {
case "pagehide":
- this.destroy();
+ // If a page hide event is triggered for current window's highlighter, hide the
+ // highlighter.
+ if (target.defaultView === this.win) {
+ this.destroy();
+ }
+
break;
case "mousedown":
// The mousedown event is intended only for the handler
if (!id) {
return;
}
let handlerSide = this.markup.getElement(id).getAttribute("data-side");
--- a/devtools/server/actors/highlighters/measuring-tool.js
+++ b/devtools/server/actors/highlighters/measuring-tool.js
@@ -196,22 +196,24 @@ MeasuringToolHighlighter.prototype = {
destroy() {
this.hide();
this._cancelUpdate();
let { pageListenerTarget } = this.env;
- pageListenerTarget.removeEventListener("mousedown", this);
- pageListenerTarget.removeEventListener("mousemove", this);
- pageListenerTarget.removeEventListener("mouseup", this);
- pageListenerTarget.removeEventListener("scroll", this);
- pageListenerTarget.removeEventListener("pagehide", this);
- pageListenerTarget.removeEventListener("mouseleave", this);
+ if (pageListenerTarget) {
+ pageListenerTarget.removeEventListener("mousedown", this);
+ pageListenerTarget.removeEventListener("mousemove", this);
+ pageListenerTarget.removeEventListener("mouseup", this);
+ pageListenerTarget.removeEventListener("scroll", this);
+ pageListenerTarget.removeEventListener("pagehide", this);
+ pageListenerTarget.removeEventListener("mouseleave", this);
+ }
this.markup.destroy();
events.emit(this, "destroy");
},
show() {
setIgnoreLayoutChanges(true);
@@ -530,14 +532,18 @@ MeasuringToolHighlighter.prototype = {
if (!this._isDragging) {
this.hideLabel("position");
}
break;
case "scroll":
this.hideLabel("position");
break;
case "pagehide":
- this.destroy();
+ // If a page hide event is triggered for current window's highlighter, hide the
+ // highlighter.
+ if (event.target.defaultView === this.env.window) {
+ this.destroy();
+ }
break;
}
}
};
exports.MeasuringToolHighlighter = MeasuringToolHighlighter;
--- a/devtools/server/actors/highlighters/rulers.js
+++ b/devtools/server/actors/highlighters/rulers.js
@@ -196,17 +196,21 @@ RulersHighlighter.prototype = {
},
handleEvent: function (event) {
switch (event.type) {
case "scroll":
this._onScroll(event);
break;
case "pagehide":
- this.destroy();
+ // If a page hide event is triggered for current window's highlighter, hide the
+ // highlighter.
+ if (event.target.defaultView === this.env.window) {
+ this.destroy();
+ }
break;
}
},
_onScroll: function (event) {
let prefix = this.ID_CLASS_PREFIX;
let { scrollX, scrollY } = event.view;
@@ -260,18 +264,21 @@ RulersHighlighter.prototype = {
this.markup.getElement(this.ID_CLASS_PREFIX + "root").setAttribute("style",
`stroke-width:${strokeWidth};`);
},
destroy: function () {
this.hide();
let { pageListenerTarget } = this.env;
- pageListenerTarget.removeEventListener("scroll", this);
- pageListenerTarget.removeEventListener("pagehide", this);
+
+ if (pageListenerTarget) {
+ pageListenerTarget.removeEventListener("scroll", this);
+ pageListenerTarget.removeEventListener("pagehide", this);
+ }
this.markup.destroy();
events.emit(this, "destroy");
},
show: function () {
this.markup.removeAttributeForElement(this.ID_CLASS_PREFIX + "elements",
--- a/devtools/server/actors/highlighters/utils/markup.js
+++ b/devtools/server/actors/highlighters/utils/markup.js
@@ -461,17 +461,17 @@ CanvasFrameAnonymousContentHelper.protot
break;
}
}
node = node.parentNode;
}
},
_removeAllListeners: function () {
- if (this.highlighterEnv) {
+ if (this.highlighterEnv && this.highlighterEnv.pageListenerTarget) {
let target = this.highlighterEnv.pageListenerTarget;
for (let [type] of this.listeners) {
target.removeEventListener(type, this, true);
}
}
this.listeners.clear();
},