Bug 1309207 - the finder iterator doesn't find occurrences properly in links-only mode. r?jaws draft
authorMike de Boer <mdeboer@mozilla.com>
Wed, 02 Nov 2016 13:38:59 +0100
changeset 432600 93211c454a63fa9813a9778fa4f22973cb103496
parent 432597 5a6416971be3800be91952c455ce8f41fa103d8b
child 535710 09206d36542f4b8f7ec2a6cc92d20fb076d022a7
push id34376
push usermdeboer@mozilla.com
push dateWed, 02 Nov 2016 12:42:39 +0000
reviewersjaws
bugs1309207
milestone52.0a1
Bug 1309207 - the finder iterator doesn't find occurrences properly in links-only mode. r?jaws MozReview-Commit-ID: 4kdxyX9zrKl
toolkit/content/tests/chrome/findbar_window.xul
toolkit/modules/FinderIterator.jsm
--- a/toolkit/content/tests/chrome/findbar_window.xul
+++ b/toolkit/content/tests/chrome/findbar_window.xul
@@ -131,21 +131,24 @@
       ok(gFindBar.hidden, "Failed to close findbar after testFindbarSelection");
       // TODO: I don't know how to drop a content element on a chrome input.
       if (!gBrowser.hasAttribute("remote"))
         testDrop();
       yield testQuickFindLink();
       if (gHasFindClipboard) {
         yield testStatusText();
       }
-      if (!gBrowser.hasAttribute("remote")) {
-        yield testFindCountUI();
-        gFindBar.close();
-        ok(gFindBar.hidden, "Failed to close findbar after testFindCountUI");
-      }
+
+      yield testFindCountUI();
+      gFindBar.close();
+      ok(gFindBar.hidden, "Failed to close findbar after testFindCountUI");
+      yield testFindCountUI(true);
+      gFindBar.close();
+      ok(gFindBar.hidden, "Failed to close findbar after testFindCountUI - linksOnly");
+
       yield openFindbar();
       yield testFindAfterCaseChanged();
       gFindBar.close();
       yield openFindbar();
       yield testFailedStringReset();
       gFindBar.close();
       yield testQuickFindClose();
       // TODO: This doesn't seem to work when the findbar is connected to a
@@ -504,20 +507,30 @@
       yield enterStringIntoFindField(SEARCH_TEXT);
       yield ContentTask.spawn(gBrowser, { SEARCH_TEXT }, function* (args) {
         Assert.equal(content.getSelection().toString(), args.SEARCH_TEXT,
           "testQuickFindText: failed to find '" + args.SEARCH_TEXT + "'");
       });
       testClipboardSearchString(SEARCH_TEXT);
     }
 
-    function* testFindCountUI(callback) {
+    function* testFindCountUI(linksOnly = false) {
       yield clearFocus();
 
-      document.getElementById("cmd_find").doCommand();
+      if (linksOnly) {
+        yield ContentTask.spawn(gBrowser, null, function* () {
+          let document = content.document;
+          let event = document.createEvent("KeyboardEvent");
+          event.initKeyEvent("keypress", true, true, null, false, false,
+                             false, false, 0, "'".charCodeAt(0));
+          document.documentElement.dispatchEvent(event);
+        });
+      } else {
+        document.getElementById("cmd_find").doCommand();
+      }
 
       ok(!gFindBar.hidden, "testFindCountUI: failed to open findbar");
       ok(document.commandDispatcher.focusedElement == gFindBar._findField.inputField,
          "testFindCountUI: find field is not focused");
 
       let promise;
       let matchCase = gFindBar.getElement("find-case-sensitive");
       if (matchCase.checked) {
@@ -525,38 +538,38 @@
         matchCase.click();
         yield new Promise(resolve => setTimeout(resolve, ITERATOR_TIMEOUT + 20));
         yield promise;
       }
 
       let foundMatches = gFindBar._foundMatches;
       let tests = [{
         text: "t",
-        current: 5,
-        total: 10,
+        current: linksOnly ? 1 : 5,
+        total: linksOnly ? 2 : 10,
       }, {
         text: "te",
-        current: 3,
-        total: 5,
+        current: linksOnly ? 1 : 3,
+        total: linksOnly ? 1 : 5,
       }, {
         text: "tes",
         current: 1,
-        total: 2,
+        total: linksOnly ? 1 : 2,
       }, {
         text: "texxx",
         current: 0,
         total: 0
       }];
       let regex = /([\d]*)\sof\s([\d]*)/;
 
       function assertMatches(aTest, aMatches) {
         is(aMatches[1], String(aTest.current),
-          `Currently highlighted match should be at ${aTest.current} for '${aTest.text}'`);
+          `${linksOnly ? "[Links-only] " : ""}Currently highlighted match should be at ${aTest.current} for '${aTest.text}'`);
         is(aMatches[2], String(aTest.total),
-          `Total amount of matches should be ${aTest.total} for '${aTest.text}'`);
+          `${linksOnly ? "[Links-only] " : ""}Total amount of matches should be ${aTest.total} for '${aTest.text}'`);
       }
 
       for (let test of tests) {
         gFindBar._findField.select();
         gFindBar._findField.focus();
 
         let timeout = ITERATOR_TIMEOUT;
         if (test.text.length == 1)
--- a/toolkit/modules/FinderIterator.jsm
+++ b/toolkit/modules/FinderIterator.jsm
@@ -461,17 +461,17 @@ this.FinderIterator = {
     for (let frame of frames) {
       for (let range of this._iterateDocument(this._currentParams, frame)) {
         // Between iterations, for example after a sleep of one cycle, we could
         // have gotten the signal to stop iterating. Make sure we do here.
         if (!this.running || spawnId !== this._spawnId)
           return;
 
         // Deal with links-only mode here.
-        if (linksOnly && this._rangeStartsInLink(range))
+        if (linksOnly && !this._rangeStartsInLink(range))
           continue;
 
         this.ranges.push(range);
 
         // Call each listener with the range we just found.
         for (let [listener, { limit, onEnd }] of this._listeners) {
           if (this._catchingUp.has(listener))
             continue;