From 99310215c044d38c2e5e23a954e31bf3e560339e Mon Sep 17 00:00:00 2001 From: Blake Kaplan Date: Tue, 22 Mar 2016 18:18:30 -0700 Subject: [PATCH] Bug 1174036 - Handle dynamically-removed textareas gracefully. r=mikedeboer r=ehsan Also, flush layout when starting a find in order to avoid racing with textarea-hiding notifications and maintain JS type correctness when objects are passed over IPC. --- .../typeaheadfind/nsTypeAheadFind.cpp | 4 ++ toolkit/modules/Finder.jsm | 17 +++--- toolkit/modules/RemoteFinder.jsm | 12 +++-- toolkit/modules/tests/browser/browser.ini | 1 + .../browser/browser_Finder_hidden_textarea.js | 52 +++++++++++++++++++ 5 files changed, 76 insertions(+), 10 deletions(-) create mode 100644 toolkit/modules/tests/browser/browser_Finder_hidden_textarea.js diff --git a/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp b/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp index 1e4b90a0bac1..08c35c04d532 100644 --- a/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp +++ b/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp @@ -315,6 +315,10 @@ nsTypeAheadFind::FindItNow(nsIPresShell *aPresShell, bool aIsLinksOnly, return NS_ERROR_FAILURE; } + // There could be unflushed notifications which hide textareas or other + // elements that we don't want to find text in. + presShell->FlushPendingNotifications(Flush_Layout); + RefPtr presContext = presShell->GetPresContext(); if (!presContext) diff --git a/toolkit/modules/Finder.jsm b/toolkit/modules/Finder.jsm index 84a4701eba7a..664560d4d0a1 100644 --- a/toolkit/modules/Finder.jsm +++ b/toolkit/modules/Finder.jsm @@ -5,9 +5,7 @@ this.EXPORTED_SYMBOLS = ["Finder","GetClipboardSearchString"]; -const Ci = Components.interfaces; -const Cc = Components.classes; -const Cu = Components.utils; +const { interfaces: Ci, classes: Cc, utils: Cu } = Components; Cu.import("resource://gre/modules/XPCOMUtils.jsm"); Cu.import("resource://gre/modules/Geometry.jsm"); @@ -519,10 +517,15 @@ Finder.prototype = { let nodes = win.document.querySelectorAll("input, textarea"); for (let node of nodes) { if (node instanceof Ci.nsIDOMNSEditableElement && node.editor) { - let sc = node.editor.selectionController; - selection = sc.getSelection(Ci.nsISelectionController.SELECTION_NORMAL); - if (selection.rangeCount && !selection.isCollapsed) { - break; + try { + let sc = node.editor.selectionController; + selection = sc.getSelection(Ci.nsISelectionController.SELECTION_NORMAL); + if (selection.rangeCount && !selection.isCollapsed) { + break; + } + } catch (e) { + // If this textarea is hidden, then its selection controller might + // not be intialized. Ignore the failure. } } } diff --git a/toolkit/modules/RemoteFinder.jsm b/toolkit/modules/RemoteFinder.jsm index 9b3e3ce606fe..932e9b433b52 100644 --- a/toolkit/modules/RemoteFinder.jsm +++ b/toolkit/modules/RemoteFinder.jsm @@ -6,15 +6,17 @@ this.EXPORTED_SYMBOLS = ["RemoteFinder", "RemoteFinderListener"]; -const Ci = Components.interfaces; -const Cc = Components.classes; -const Cu = Components.utils; +const { interfaces: Ci, classes: Cc, utils: Cu } = Components; Cu.import("resource://gre/modules/XPCOMUtils.jsm"); +Cu.import("resource://gre/modules/Geometry.jsm"); XPCOMUtils.defineLazyGetter(this, "GetClipboardSearchString", () => Cu.import("resource://gre/modules/Finder.jsm", {}).GetClipboardSearchString ); +XPCOMUtils.defineLazyGetter(this, "Rect", + () => Cu.import("resource://gre/modules/Geometry.jsm", {}).Rect +); function RemoteFinder(browser) { this._listeners = new Set(); @@ -62,6 +64,10 @@ RemoteFinder.prototype = { switch (aMessage.name) { case "Finder:Result": this._searchString = aMessage.data.searchString; + // The rect stops being a Geometry.jsm:Rect over IPC. + if (aMessage.data.rect) { + aMessage.data.rect = Rect.fromRect(aMessage.data.rect); + } callback = "onFindResult"; params = [ aMessage.data ]; break; diff --git a/toolkit/modules/tests/browser/browser.ini b/toolkit/modules/tests/browser/browser.ini index ac85d6f2c855..04f40c4ddec7 100644 --- a/toolkit/modules/tests/browser/browser.ini +++ b/toolkit/modules/tests/browser/browser.ini @@ -25,6 +25,7 @@ support-files = [browser_Battery.js] [browser_Deprecated.js] [browser_Finder.js] +[browser_Finder_hidden_textarea.js] [browser_Geometry.js] [browser_InlineSpellChecker.js] [browser_WebNavigation.js] diff --git a/toolkit/modules/tests/browser/browser_Finder_hidden_textarea.js b/toolkit/modules/tests/browser/browser_Finder_hidden_textarea.js new file mode 100644 index 000000000000..99d838adaf71 --- /dev/null +++ b/toolkit/modules/tests/browser/browser_Finder_hidden_textarea.js @@ -0,0 +1,52 @@ +/* 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/. */ +add_task(function* test_bug1174036() { + const URI = + ""; + yield BrowserTestUtils.withNewTab({ gBrowser, url: "data:text/html;charset=utf-8," + encodeURIComponent(URI) }, + function* (browser) { + // Hide the first textarea. + yield ContentTask.spawn(browser, null, function() { + content.document.getElementsByTagName("textarea")[0].style.display = "none"; + }); + + let finder = browser.finder; + let listener = { + onFindResult: function () { + ok(false, "callback wasn't replaced"); + } + }; + finder.addResultListener(listener); + + function waitForFind() { + return new Promise(resolve => { + listener.onFindResult = resolve; + }) + } + + // Find the first 'e' (which should be in the second textarea). + let promiseFind = waitForFind(); + finder.fastFind("e", false, false); + let findResult = yield promiseFind; + is(findResult.result, Ci.nsITypeAheadFind.FIND_FOUND, "find first string"); + + let firstRect = findResult.rect; + + // Find the second 'e' (in the third textarea). + promiseFind = waitForFind(); + finder.findAgain(false, false, false); + findResult = yield promiseFind; + is(findResult.result, Ci.nsITypeAheadFind.FIND_FOUND, "find second string"); + ok(!findResult.rect.equals(firstRect), "found new string"); + + // Ensure that we properly wrap to the second textarea. + promiseFind = waitForFind(); + finder.findAgain(false, false, false); + findResult = yield promiseFind; + is(findResult.result, Ci.nsITypeAheadFind.FIND_WRAPPED, "wrapped to first string"); + ok(findResult.rect.equals(firstRect), "wrapped to original string"); + + finder.removeResultListener(listener); + }); +});