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

--HG--
extra : rebase_source : 03dc19bc9ba83f7601d4d57d2bd9b78d312fda1b

toolkit/content/tests/chrome/bug263683_window.xul

@@ -23,10 +23,12 @@
     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];
     }
@@ -82,12 +84,16 @@
     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) {
@@ -96,13 +102,14 @@
                                  .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) {
@@ -116,6 +123,7 @@
         input.value = args.search;
       });
  
+      yield new Promise(resolve => setTimeout(resolve, kIteratorTimeout));
       yield toggleHighlightAndWait(true);
 
       yield ContentTask.spawn(gBrowser, { search }, function* (args) {
@@ -129,6 +137,7 @@
           args.search, "Added the correct match");
       });
 
+      yield new Promise(resolve => setTimeout(resolve, kIteratorTimeout));
       yield toggleHighlightAndWait(false);
 
       yield ContentTask.spawn(gBrowser, null, function* () {
@@ -139,7 +148,83 @@
         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>
 

toolkit/modules/Finder.jsm

@@ -57,8 +57,11 @@ Finder.prototype = {
     if (this._iterator)
       this._iterator.reset();
     if (this._highlighter) {
-      this._highlighter.clear();
+      // 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.listeners = [];
     this._docShell.QueryInterface(Ci.nsIInterfaceRequestor)
@@ -331,10 +334,10 @@ Finder.prototype = {
   },
 
   onHighlightAllChange(highlightAll) {
-    if (this._iterator)
-      this._iterator.reset();
     if (this._highlighter)
       this._highlighter.onHighlightAllChange(highlightAll);
+    if (this._iterator)
+      this._iterator.reset();
   },
 
   keyPress: function (aEvent) {

toolkit/modules/FinderHighlighter.jsm

@@ -291,9 +291,9 @@ FinderHighlighter.prototype = {
     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);
   },
 
   /**
@@ -316,6 +316,12 @@ FinderHighlighter.prototype = {
     } 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) {
@@ -360,11 +366,16 @@ FinderHighlighter.prototype = {
     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
@@ -477,6 +488,9 @@ FinderHighlighter.prototype = {
    */
   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;
@@ -537,7 +551,7 @@ FinderHighlighter.prototype = {
    */
   onHighlightAllChange(highlightAll) {
     this._highlightAll = highlightAll;
-    if (this._modal && !highlightAll) {
+    if (!highlightAll) {
       this.clear();
       this._scheduleRepaintOfMask(this.finder._getWindow());
     }
@@ -1006,6 +1020,9 @@ FinderHighlighter.prototype = {
    *                            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);

toolkit/modules/FinderIterator.jsm

@@ -536,7 +536,7 @@ this.FinderIterator = {
    */
   _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