Bug 1295759 - make sure selected ranges in iframes are cleared when the findbar is hidden as well. Adds a test to guard against regressions. r?jaws
MozReview-Commit-ID: 5rudNSNK8GK
--- a/toolkit/content/tests/chrome/bug263683_window.xul
+++ b/toolkit/content/tests/chrome/bug263683_window.xul
@@ -18,20 +18,22 @@
<script type="application/javascript"><![CDATA[
const {interfaces: Ci, classes: Cc, results: Cr, utils: Cu} = Components;
Cu.import("resource://gre/modules/AppConstants.jsm");
Cu.import("resource://gre/modules/Task.jsm");
Cu.import("resource://testing-common/ContentTask.jsm");
ContentTask.setTestScope(window.opener.wrappedJSObject);
+ var gPrefsvc = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
+ const kIteratorTimeout = gPrefsvc.getIntPref("findbar.iteratorTimeout") + 20;
var gFindBar = null;
var gBrowser;
- var imports = ["SimpleTest", "ok", "info"];
+ var imports = ["SimpleTest", "ok", "info", "is"];
for (var name of imports) {
window[name] = window.opener.wrappedJSObject[name];
}
function startTest() {
Task.spawn(function* () {
gFindBar = document.getElementById("FindToolbar");
for (let browserId of ["content", "content-remote"]) {
@@ -77,73 +79,156 @@
gFindBar.browser.finder.addResultListener(listener);
gFindBar.toggleHighlight(highlight);
});
}
function* onDocumentLoaded() {
gFindBar.open();
var search = "mozilla";
+ gFindBar._findField.focus();
gFindBar._findField.value = search;
var matchCase = gFindBar.getElement("find-case-sensitive");
- if (matchCase.checked)
+ if (matchCase.checked) {
matchCase.doCommand();
+ yield new Promise(resolve => setTimeout(resolve, kIteratorTimeout));
+ }
gFindBar._find();
+ yield new Promise(resolve => setTimeout(resolve, kIteratorTimeout));
yield toggleHighlightAndWait(true);
yield ContentTask.spawn(gBrowser, { search }, function* (args) {
let controller = docShell.QueryInterface(Ci.nsIInterfaceRequestor)
.getInterface(Ci.nsISelectionDisplay)
.QueryInterface(Ci.nsISelectionController);
Assert.ok("SELECTION_FIND" in controller, "Correctly detects new selection type");
let selection = controller.getSelection(controller.SELECTION_FIND);
-
+
Assert.equal(selection.rangeCount, 1,
"Correctly added a match to the selection type");
Assert.equal(selection.getRangeAt(0).toString().toLowerCase(),
args.search, "Added the correct match");
});
+ yield new Promise(resolve => setTimeout(resolve, kIteratorTimeout));
yield toggleHighlightAndWait(false);
yield ContentTask.spawn(gBrowser, { search }, function* (args) {
let controller = docShell.QueryInterface(Ci.nsIInterfaceRequestor)
.getInterface(Ci.nsISelectionDisplay)
.QueryInterface(Ci.nsISelectionController);
let selection = controller.getSelection(controller.SELECTION_FIND);
Assert.equal(selection.rangeCount, 0, "Correctly removed the range");
let input = content.document.getElementById("inp");
input.value = args.search;
});
+ yield new Promise(resolve => setTimeout(resolve, kIteratorTimeout));
yield toggleHighlightAndWait(true);
yield ContentTask.spawn(gBrowser, { search }, function* (args) {
let input = content.document.getElementById("inp");
let inputController = input.editor.selectionController;
let inputSelection = inputController.getSelection(inputController.SELECTION_FIND);
Assert.equal(inputSelection.rangeCount, 1,
"Correctly added a match from input to the selection type");
Assert.equal(inputSelection.getRangeAt(0).toString().toLowerCase(),
args.search, "Added the correct match");
});
+ yield new Promise(resolve => setTimeout(resolve, kIteratorTimeout));
yield toggleHighlightAndWait(false);
yield ContentTask.spawn(gBrowser, null, function* () {
let input = content.document.getElementById("inp");
let inputController = input.editor.selectionController;
let inputSelection = inputController.getSelection(inputController.SELECTION_FIND);
Assert.equal(inputSelection.rangeCount, 0, "Correctly removed the range");
});
+ yield new Promise(resolve => setTimeout(resolve, kIteratorTimeout));
+
+ // For posterity, test iframes too.
+
+ let promise = ContentTask.spawn(gBrowser, null, function* () {
+ return new Promise(resolve => {
+ addEventListener("DOMContentLoaded", function listener() {
+ removeEventListener("DOMContentLoaded", listener);
+ resolve();
+ });
+ });
+ });
+ gBrowser.loadURI('data:text/html,<h2>Text mozilla</h2><iframe id="leframe" ' +
+ 'src="data:text/html,Text mozilla"></iframe>');
+ yield promise;
+
+ yield new Promise(resolve => setTimeout(resolve, kIteratorTimeout));
+ yield toggleHighlightAndWait(true);
+
+ yield ContentTask.spawn(gBrowser, { search }, function* (args) {
+ function getSelection(docShell) {
+ let controller = docShell.QueryInterface(Ci.nsIInterfaceRequestor)
+ .getInterface(Ci.nsISelectionDisplay)
+ .QueryInterface(Ci.nsISelectionController);
+ return controller.getSelection(controller.SELECTION_FIND);
+ }
+
+ let selection = getSelection(docShell);
+ Assert.equal(selection.rangeCount, 1,
+ "Correctly added a match to the selection type");
+ Assert.equal(selection.getRangeAt(0).toString().toLowerCase(),
+ args.search, "Added the correct match");
+
+ // Check the iframe too:
+ let frame = content.document.getElementById("leframe");
+ // Hoops! Get the docShell first, then the selection.
+ selection = getSelection(frame.contentWindow
+ .QueryInterface(Ci.nsIInterfaceRequestor)
+ .getInterface(Ci.nsIWebNavigation)
+ .QueryInterface(Ci.nsIDocShell));
+ Assert.equal(selection.rangeCount, 1,
+ "Correctly added a match to the selection type");
+ Assert.equal(selection.getRangeAt(0).toString().toLowerCase(),
+ args.search, "Added the correct match");
+ });
+
+ yield new Promise(resolve => setTimeout(resolve, kIteratorTimeout));
+ yield toggleHighlightAndWait(false);
+
+ let matches = gFindBar._foundMatches.value.match(/([\d]*)\sof\s([\d]*)/);
+ is(matches[1], "2", "Found correct amount of matches")
+
+ yield ContentTask.spawn(gBrowser, null, function* (args) {
+ function getSelection(docShell) {
+ let controller = docShell.QueryInterface(Ci.nsIInterfaceRequestor)
+ .getInterface(Ci.nsISelectionDisplay)
+ .QueryInterface(Ci.nsISelectionController);
+ return controller.getSelection(controller.SELECTION_FIND);
+ }
+
+ let selection = getSelection(docShell);
+ Assert.equal(selection.rangeCount, 0, "Correctly removed the range");
+
+ // Check the iframe too:
+ let frame = content.document.getElementById("leframe");
+ // Hoops! Get the docShell first, then the selection.
+ selection = getSelection(frame.contentWindow
+ .QueryInterface(Ci.nsIInterfaceRequestor)
+ .getInterface(Ci.nsIWebNavigation)
+ .QueryInterface(Ci.nsIDocShell));
+ Assert.equal(selection.rangeCount, 0, "Correctly removed the range");
+
+ content.document.documentElement.focus();
+ });
+
gFindBar.close();
+ yield new Promise(resolve => setTimeout(resolve, kIteratorTimeout));
}
]]></script>
<browser type="content-primary" flex="1" id="content" src="about:blank"/>
<browser type="content-primary" flex="1" id="content-remote" remote="true" src="about:blank"/>
<findbar id="FindToolbar" browserid="content"/>
</window>
--- a/toolkit/modules/Finder.jsm
+++ b/toolkit/modules/Finder.jsm
@@ -52,18 +52,21 @@ Finder.prototype = {
this._iterator = Cu.import("resource://gre/modules/FinderIterator.jsm", null).FinderIterator;
return this._iterator;
},
destroy: function() {
if (this._iterator)
this._iterator.reset();
if (this._highlighter) {
+ // if we clear all the references before we hide the highlights (in both
+ // highlighting modes), we simply can't use them to find the ranges we
+ // need to clear from the selection.
+ this._highlighter.hide();
this._highlighter.clear();
- this._highlighter.hide();
}
this.listeners = [];
this._docShell.QueryInterface(Ci.nsIInterfaceRequestor)
.getInterface(Ci.nsIWebProgress)
.removeProgressListener(this, Ci.nsIWebProgress.NOTIFY_LOCATION);
this._listeners = [];
this._currentFoundRange = this._fastFind = this._docShell = this._previousLink =
this._highlighter = null;
@@ -326,20 +329,20 @@ Finder.prototype = {
},
onModalHighlightChange(useModalHighlight) {
if (this._highlighter)
this._highlighter.onModalHighlightChange(useModalHighlight);
},
onHighlightAllChange(highlightAll) {
+ if (this._highlighter)
+ this._highlighter.onHighlightAllChange(highlightAll);
if (this._iterator)
this._iterator.reset();
- if (this._highlighter)
- this._highlighter.onHighlightAllChange(highlightAll);
},
keyPress: function (aEvent) {
let controller = this._getSelectionController(this._getWindow());
switch (aEvent.keyCode) {
case Ci.nsIDOMKeyEvent.DOM_VK_RETURN:
if (this._fastFind.foundLink) {
--- a/toolkit/modules/FinderHighlighter.jsm
+++ b/toolkit/modules/FinderHighlighter.jsm
@@ -286,19 +286,19 @@ FinderHighlighter.prototype = {
onIteratorRestart() {},
onIteratorStart(params) {
let window = this.finder._getWindow();
let dict = this.getForWindow(window);
// Save a clean params set for use later in the `update()` method.
dict.lastIteratorParams = params;
- this.clear(window);
if (!this._modal)
this.hide(window, this.finder._fastFind.getFoundRange());
+ this.clear(window);
},
/**
* Add a range to the find selection, i.e. highlight it, and if it's inside an
* editable node, track it.
*
* @param {nsIDOMRange} range Range object to be highlighted
*/
@@ -311,16 +311,22 @@ FinderHighlighter.prototype = {
controller = editableNode.editor.selectionController;
}
if (this._modal) {
this._modalHighlight(range, controller, window);
} else {
let findSelection = controller.getSelection(Ci.nsISelectionController.SELECTION_FIND);
findSelection.addRange(range);
+ // Check if the range is inside an iframe.
+ if (window != window.top) {
+ let dict = this.getForWindow(window.top);
+ if (!dict.frames.has(window))
+ dict.frames.set(window, null);
+ }
}
if (editableNode) {
// Highlighting added, so cache this editor, and hook up listeners
// to ensure we deal properly with edits within the highlighting
this._addEditorListeners(editableNode.editor);
}
},
@@ -355,21 +361,26 @@ FinderHighlighter.prototype = {
* @param {nsIDOMEvent} event When called from an event handler, this will
* be the triggering event.
*/
hide(window = null, skipRange = null, event = null) {
// Do not hide on anything but a left-click.
if (event && event.type == "click" && event.button !== 0)
return;
- window = (window || this.finder._getWindow()).top;
+ try {
+ window = (window || this.finder._getWindow()).top;
+ } catch (ex) {
+ Cu.reportError(ex);
+ return;
+ }
let dict = this.getForWindow(window);
this._clearSelection(this.finder._getSelectionController(window), skipRange);
- for (let frame of dict.frames)
+ for (let frame of dict.frames.keys())
this._clearSelection(this.finder._getSelectionController(frame), skipRange);
// Next, check our editor cache, for editors belonging to this
// document
if (this._editors) {
let doc = window.document;
for (let x = this._editors.length - 1; x >= 0; --x) {
if (this._editors[x].document == doc) {
@@ -472,16 +483,19 @@ FinderHighlighter.prototype = {
},
/**
* Invalidates the list by clearing the map of highglighted ranges that we
* keep to build the mask for.
*/
clear(window = null) {
if (!window) {
+ // Since we're clearing _all the things_, make sure we hide 'em all as well.
+ for (let win of gWindows.keys())
+ this.hide(win);
// Reset the Map, because no range references a node anymore.
gWindows.clear();
return;
}
let dict = this.getForWindow(window.top);
dict.currentFoundRange = null;
dict.dynamicRangesSet.clear();
@@ -532,17 +546,17 @@ FinderHighlighter.prototype = {
/**
* When 'Highlight All' is toggled during a session, this callback is invoked
* and when it's turned off, the found occurrences will be removed from the mask.
*
* @param {Boolean} highlightAll
*/
onHighlightAllChange(highlightAll) {
this._highlightAll = highlightAll;
- if (this._modal && !highlightAll) {
+ if (!highlightAll) {
this.clear();
this._scheduleRepaintOfMask(this.finder._getWindow());
}
},
/**
* Utility; removes all ranges from the find selection that belongs to a
* controller. Optionally skips a specific range.
@@ -1001,16 +1015,19 @@ FinderHighlighter.prototype = {
* {Boolean} contentChanged Whether the documents' content changed in the
* meantime. This happens when the DOM is updated
* whilst the page is loaded.
* {Boolean} scrollOnly TRUE when the page has scrolled in the meantime,
* which means that the fixed positioned elements
* need to be repainted.
*/
_scheduleRepaintOfMask(window, { contentChanged, scrollOnly } = { contentChanged: false, scrollOnly: false }) {
+ if (!this._modal)
+ return;
+
window = window.top;
let dict = this.getForWindow(window);
let repaintFixedNodes = (scrollOnly && !!dict.dynamicRangesSet.size);
// When we request to repaint unconditionally, we mean to call
// `_repaintHighlightAllMask()` right after the timeout.
if (!dict.unconditionalRepaintRequested)
dict.unconditionalRepaintRequested = !contentChanged || repaintFixedNodes;
--- a/toolkit/modules/FinderIterator.jsm
+++ b/toolkit/modules/FinderIterator.jsm
@@ -531,17 +531,17 @@ this.FinderIterator = {
* visible (i)frames inside a window.
*
* @param {nsIDOMWindow} window The window to extract the (i)frames from
* @param {Finder} finder The Finder instance
* @return {Array} Stack of frames to iterate over
*/
_collectFrames(window, finder) {
let frames = [];
- if (!window.frames || !window.frames.length)
+ if (!("frames" in window) || !window.frames.length)
return frames;
// Casting `window.frames` to an Iterator doesn't work, so we're stuck with
// a plain, old for-loop.
for (let i = 0, l = window.frames.length; i < l; ++i) {
let frame = window.frames[i];
// Don't count matches in hidden frames.
let frameEl = frame && frame.frameElement;