Bug 1174036 - Handle dynamically-removed textareas gracefully. r=mikedeboer r=ehsan
authorBlake Kaplan <mrbkap@gmail.com>
Tue, 22 Mar 2016 18:18:30 -0700
changeset 290750 6a5f05ddfa83b5be135e329d46ab0d0a40e2696f
parent 290749 b92ce093fe6582c6d5a4879941f46e9a529cc20e
child 290751 e7b4afa0a2caeff9a83c065e6e34a8768f074abb
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer, ehsan
bugs1174036
milestone48.0a1
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.
toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
toolkit/modules/Finder.jsm
toolkit/modules/RemoteFinder.jsm
toolkit/modules/tests/browser/browser.ini
toolkit/modules/tests/browser/browser_Finder_hidden_textarea.js
--- a/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
+++ b/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
@@ -310,16 +310,20 @@ nsTypeAheadFind::FindItNow(nsIPresShell 
 
   if (!presShell) {
     presShell = startingPresShell;  // this is the current document
 
     if (!presShell)
       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<nsPresContext> presContext = presShell->GetPresContext();
 
   if (!presContext)
     return NS_ERROR_FAILURE;
 
   nsCOMPtr<nsISelection> selection;
   nsCOMPtr<nsISelectionController> selectionController = 
     do_QueryReferent(mSelectionController);
--- a/toolkit/modules/Finder.jsm
+++ b/toolkit/modules/Finder.jsm
@@ -1,18 +1,16 @@
 // vim: set ts=2 sw=2 sts=2 tw=80:
 // 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/.
 
 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");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 
 XPCOMUtils.defineLazyServiceGetter(this, "TextToSubURIService",
                                          "@mozilla.org/intl/texttosuburi;1",
@@ -514,20 +512,25 @@ Finder.prototype = {
       return null;
 
     let selection = win.getSelection();
     if (!selection.rangeCount || selection.isCollapsed) {
       // The selection can be into an input or a textarea element.
       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.
           }
         }
       }
     }
 
     if (!selection.rangeCount || selection.isCollapsed) {
       return null;
     }
--- a/toolkit/modules/RemoteFinder.jsm
+++ b/toolkit/modules/RemoteFinder.jsm
@@ -1,25 +1,27 @@
 // -*- indent-tabs-mode: nil; js-indent-level: 2 -*-
 // vim: set ts=2 sw=2 sts=2 et tw=80: */
 // 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/.
 
 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();
   this._searchString = null;
 
   this.swapBrowser(browser);
 }
 
@@ -57,16 +59,20 @@ RemoteFinder.prototype = {
 
   receiveMessage: function (aMessage) {
     // Only Finder:Result messages have the searchString field.
     let callback;
     let params;
     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;
       case "Finder:MatchesResult":
         callback = "onMatchesCountResult";
         params = [ aMessage.data ];
         break;
       case "Finder:CurrentSelectionResult":
--- a/toolkit/modules/tests/browser/browser.ini
+++ b/toolkit/modules/tests/browser/browser.ini
@@ -20,16 +20,17 @@ support-files =
   file_script_xhr.js
   WebRequest_dynamic.sjs
   WebRequest_redirection.sjs
 
 [browser_AsyncPrefs.js]
 [browser_Battery.js]
 [browser_Deprecated.js]
 [browser_Finder.js]
+[browser_Finder_hidden_textarea.js]
 [browser_Geometry.js]
 [browser_InlineSpellChecker.js]
 [browser_WebNavigation.js]
 [browser_WebRequest.js]
 [browser_WebRequest_cookies.js]
 [browser_WebRequest_filtering.js]
 [browser_PageMetadata.js]
 [browser_PromiseMessage.js]
new file mode 100644
--- /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 =
+    "<body><textarea>e1</textarea><textarea>e2</textarea><textarea>e3</textarea></body>";
+  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);
+    });
+});