Bug 1358500 - fix find bar being disabled all the time, r?mikedeboer draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Mon, 29 May 2017 16:17:56 +0100
changeset 586416 d8ed5a8073ee345409c174cb89560d014580ec2c
parent 585650 35099b4caec14bf0e3c5e3fed7a17dd3faf51dbe
child 630969 2095d2dabe85cdb92540e3497904be556d5333d0
push id61384
push userbmo:gijskruitbosch+bugs@gmail.com
push dateTue, 30 May 2017 09:45:39 +0000
reviewersmikedeboer
bugs1358500
milestone55.0a1
Bug 1358500 - fix find bar being disabled all the time, r?mikedeboer MozReview-Commit-ID: BpnCdKgtYiG
browser/base/content/test/general/browser_bug537013.js
toolkit/content/tests/chrome/findbar_window.xul
toolkit/content/widgets/findbar.xml
--- a/browser/base/content/test/general/browser_bug537013.js
+++ b/browser/base/content/test/general/browser_bug537013.js
@@ -40,17 +40,17 @@ function test() {
   });
   texts.forEach(aText => addTabWithText(aText));
 
   // Set up the first tab
   gBrowser.selectedTab = tabs[0];
 
   setFindString(texts[0]);
   // Turn on highlight for testing bug 891638
-  gFindBar.toggleHighlight(true);
+  gFindBar.getElement("highlight").checked = true;
 
   // Make sure the second tab is correct, then set it up
   gBrowser.selectedTab = tabs[1];
   gBrowser.selectedTab.addEventListener("TabFindInitialized", continueTests1);
   // Initialize the findbar
   gFindBar;
 }
 function continueTests1() {
@@ -67,17 +67,17 @@ function continueTests1() {
   gBrowser.selectedTab = tabs[0];
   ok(!gFindBar.hidden, "First tab shows find bar!");
   // When the Find Clipboard is supported, this test not relevant.
   if (!HasFindClipboard)
     is(gFindBar._findField.value, texts[0], "First tab persists find value!");
   ok(gFindBar.getElement("highlight").checked,
      "Highlight button state persists!");
 
-  // While we're here, let's test the backout of bug 253793.
+  // While we're here, let's test bug 253793
   gBrowser.reload();
   gBrowser.addEventListener("DOMContentLoaded", continueTests2, true);
 }
 
 function continueTests2() {
   gBrowser.removeEventListener("DOMContentLoaded", continueTests2, true);
   ok(gFindBar.getElement("highlight").checked, "Highlight never reset!");
   continueTests3();
@@ -123,13 +123,13 @@ function continueTests3() {
 // Test that findbar gets restored when a tab is moved to a new window.
 function checkNewWindow() {
   ok(!newWindow.gFindBar.hidden, "New window shows find bar!");
   // Disabled the following assertion due to intermittent failure on OSX 10.6 Debug.
   if (!HasFindClipboard) {
     is(newWindow.gFindBar._findField.value, texts[1],
        "New window find bar has correct find value!");
     ok(!newWindow.gFindBar.getElement("find-next").disabled,
-       "New window findbar has disabled buttons!");
+       "New window findbar has enabled buttons!");
   }
   newWindow.close();
   finish();
 }
--- a/toolkit/content/tests/chrome/findbar_window.xul
+++ b/toolkit/content/tests/chrome/findbar_window.xul
@@ -149,38 +149,26 @@
       await testQuickFindClose();
       // TODO: This doesn't seem to work when the findbar is connected to a
       //       remote browser element.
       if (!gBrowser.hasAttribute("remote"))
         await testFindAgainNotFound();
       await testToggleEntireWord();
     }
 
-    function testFindButtonsState(enabled = null) {
-      // If `enabled` is not explicitly passed in, we set it to its most logical
-      // value.
-      if (enabled === null)
-        enabled = !!gFindBar._findField.value;
-      is(gFindBar.getElement("find-next").disabled, !enabled,
-        `Expected the 'next' button to be ${enabled ? 'enabled' : 'disabled'}`);
-      is(gFindBar.getElement("find-previous").disabled, !enabled,
-        `Expected the 'previous' button to be ${enabled ? 'enabled' : 'disabled'}`);
-    }
-
     async function testFindbarSelection() {
       function checkFindbarState(aTestName, aExpSelection) {
         ok(!gFindBar.hidden, "testFindbarSelection: failed to open findbar: " + aTestName);
         ok(document.commandDispatcher.focusedElement == gFindBar._findField.inputField,
            "testFindbarSelection: find field is not focused: " + aTestName);
         if (!gHasFindClipboard) {
           ok(gFindBar._findField.value == aExpSelection,
              "Incorrect selection in testFindbarSelection: "  + aTestName +
              ". Selection: " + gFindBar._findField.value);
         }
-        testFindButtonsState();
 
         // Clear the value, close the findbar.
         gFindBar._findField.value = "";
         gFindBar.close();
       }
 
       // Test normal selected text.
       await ContentTask.spawn(gBrowser, null, async function() {
@@ -328,32 +316,30 @@
       await enterStringIntoFindField(searchStr);
 
       let sel = await ContentTask.spawn(gBrowser, { searchStr }, async function(args) {
         let sel = content.getSelection().toString();
         Assert.equal(sel.toLowerCase(), args.searchStr,
           "testNormalFind: failed to find '" + args.searchStr + "'");
         return sel;
       });
-      testFindButtonsState();
       testClipboardSearchString(sel);
 
       if (!matchCaseCheckbox.hidden) {
         promise = promiseFindResult();
         matchCaseCheckbox.click();
         await promise;
         enterStringIntoFindField("t");
         await ContentTask.spawn(gBrowser, { searchStr }, async function(args) {
           Assert.notEqual(content.getSelection().toString(), args.searchStr,
             "testNormalFind: Case-sensitivy is broken '" + args.searchStr + "'");
         });
         promise = promiseFindResult();
         matchCaseCheckbox.click();
         await promise;
-        testFindButtonsState();
       }
     }
 
     function openFindbar() {
       document.getElementById("cmd_find").doCommand();
       return gFindBar._startFindDeferred && gFindBar._startFindDeferred.promise;
     }
 
@@ -383,28 +369,26 @@
           },
           "caret": { "start": searchStr.length, "length": 0 }
         });
 
       await ContentTask.spawn(gBrowser, { searchStr }, async function(args) {
         Assert.notEqual(content.getSelection().toString().toLowerCase(), args.searchStr,
           "testNormalFindWithComposition: text shouldn't be found during composition");
       });
-      testFindButtonsState();
 
       synthesizeComposition({ type: "compositioncommitasis" });
 
       let sel = await ContentTask.spawn(gBrowser, { searchStr }, async function(args) {
         let sel = content.getSelection().toString();
         Assert.equal(sel.toLowerCase(), args.searchStr,
           "testNormalFindWithComposition: text should be found after committing composition");
         return sel;
       });
       testClipboardSearchString(sel);
-      testFindButtonsState();
 
       if (clicked) {
         matchCaseCheckbox.click();
       }
     }
 
     async function testAutoCaseSensitivityUI() {
       var matchCaseCheckbox = gFindBar.getElement("find-case-sensitive");
@@ -458,17 +442,16 @@
          "testQuickFindLink: find field is not focused");
 
       var searchStr = "Link Test";
       await enterStringIntoFindField(searchStr);
       await ContentTask.spawn(gBrowser, { searchStr }, async function(args) {
         Assert.equal(content.getSelection().toString(), args.searchStr,
           "testQuickFindLink: failed to find sample link");
       });
-      testFindButtonsState();
       testClipboardSearchString(searchStr);
     }
 
     // See bug 963925 for more details on this test.
     async function testFindWithHighlight() {
       gFindBar._findField.value = "";
 
       // For this test, we want to closely control the selection. The easiest
@@ -577,17 +560,16 @@
       ok(document.commandDispatcher.focusedElement == gFindBar._findField.inputField,
         "testQuickFindText: find field is not focused");
 
       await enterStringIntoFindField(SEARCH_TEXT);
       await ContentTask.spawn(gBrowser, { SEARCH_TEXT }, async function(args) {
         Assert.equal(content.getSelection().toString(), args.SEARCH_TEXT,
           "testQuickFindText: failed to find '" + args.SEARCH_TEXT + "'");
       });
-      testFindButtonsState();
       testClipboardSearchString(SEARCH_TEXT);
     }
 
     async function testFindCountUI(linksOnly = false) {
       await clearFocus();
 
       if (linksOnly) {
         await ContentTask.spawn(gBrowser, null, async function() {
@@ -634,17 +616,16 @@
       }];
       let regex = /([\d]*)\sof\s([\d]*)/;
 
       function assertMatches(aTest, aMatches) {
         is(aMatches[1], String(aTest.current),
           `${linksOnly ? "[Links-only] " : ""}Currently highlighted match should be at ${aTest.current} for '${aTest.text}'`);
         is(aMatches[2], String(aTest.total),
           `${linksOnly ? "[Links-only] " : ""}Total amount of matches should be ${aTest.total} for '${aTest.text}'`);
-        testFindButtonsState(aMatches.total !== 0);
       }
 
       for (let test of tests) {
         gFindBar._findField.select();
         gFindBar._findField.focus();
 
         let timeout = ITERATOR_TIMEOUT;
         if (test.text.length == 1)
@@ -684,43 +665,39 @@
       gFindBar.clear();
 
       gPrefsvc.setIntPref("accessibility.typeaheadfind.casesensitive", 0);
 
       await enterStringIntoFindField("t");
       await ContentTask.spawn(gBrowser, null, async function() {
         Assert.equal(content.getSelection().toString(), "T", "First T should be selected.");
       });
-      testFindButtonsState();
 
       gPrefsvc.setIntPref("accessibility.typeaheadfind.casesensitive", 1);
       await ContentTask.spawn(gBrowser, null, async function() {
         Assert.equal(content.getSelection().toString(), "t", "First t should be selected.");
       });
-      testFindButtonsState();
     }
 
     // Make sure that _findFailedString is cleared:
     // 1. Do a search that fails with case sensitivity but matches with no case sensitivity.
     // 2. Uncheck case sensitivity button to match the string.
     async function testFailedStringReset() {
       gPrefsvc.setIntPref("accessibility.typeaheadfind.casesensitive", 1);
 
       await enterStringIntoFindField(SEARCH_TEXT.toUpperCase(), false);
       await ContentTask.spawn(gBrowser, null, async function() {
         Assert.equal(content.getSelection().toString(), "", "Not found.");
       });
-      testFindButtonsState(false);
 
       gPrefsvc.setIntPref("accessibility.typeaheadfind.casesensitive", 0);
       await ContentTask.spawn(gBrowser, { SEARCH_TEXT }, async function(args) {
         Assert.equal(content.getSelection().toString(), args.SEARCH_TEXT,
           "Search text should be selected.");
       });
-      testFindButtonsState();
     }
 
     function testClipboardSearchString(aExpected) {
       if (!gHasFindClipboard)
         return;
 
       if (!aExpected)
         aExpected = "";
@@ -729,50 +706,45 @@
         "testClipboardSearchString: search string not set to '" + aExpected +
         "', instead found '" + searchStr + "'");
     }
 
     // See bug 967982.
     async function testFindAgainNotFound() {
       await openFindbar();
       await enterStringIntoFindField(NOT_FOUND_TEXT, false);
-      testFindButtonsState(false);
       gFindBar.close();
       ok(gFindBar.hidden, "The findbar is closed.");
       let promise = promiseFindResult();
       gFindBar.onFindAgainCommand();
       await promise;
       ok(!gFindBar.hidden, "Unsuccessful Find Again opens the find bar.");
-      testFindButtonsState(false);
 
       await enterStringIntoFindField(SEARCH_TEXT);
-      testFindButtonsState();
       gFindBar.close();
       ok(gFindBar.hidden, "The findbar is closed.");
       promise = promiseFindResult();
       gFindBar.onFindAgainCommand();
       await promise;
       ok(gFindBar.hidden, "Successful Find Again leaves the find bar closed.");
     }
 
     async function testToggleEntireWord() {
       await openFindbar();
       let promise = promiseFindResult();
       await enterStringIntoFindField("Tex", false);
       let result = await promise;
       is(result.result, Ci.nsITypeAheadFind.FIND_FOUND, "Text should be found");
-      testFindButtonsState();
 
       await new Promise(resolve => setTimeout(resolve, ITERATOR_TIMEOUT + 20));
       promise = promiseFindResult();
       let check = gFindBar.getElement("find-entire-word");
       check.click();
       result = await promise;
       is(result.result, Ci.nsITypeAheadFind.FIND_NOTFOUND, "Text should NOT be found");
-      testFindButtonsState(false);
 
       check.click();
       gFindBar.close(true);
     }
   ]]></script>
 
   <commandset>
     <command id="cmd_find" oncommand="document.getElementById('FindToolbar').onFindCommand();"/>
--- a/toolkit/content/widgets/findbar.xml
+++ b/toolkit/content/widgets/findbar.xml
@@ -361,16 +361,18 @@
 
       <constructor><![CDATA[
         // These elements are accessed frequently and are therefore cached
         this._findField = this.getElement("findbar-textbox");
         this._foundMatches = this.getElement("found-matches");
         this._findStatusIcon = this.getElement("find-status-icon");
         this._findStatusDesc = this.getElement("find-status");
 
+        this._foundURL = null;
+
         let prefsvc = this._prefsvc;
 
         this._quickFindTimeoutLength =
           prefsvc.getIntPref("accessibility.typeaheadfind.timeout");
         this._flashFindBar =
           prefsvc.getIntPref("accessibility.typeaheadfind.flashBar");
         this._useModalHighlight = prefsvc.getBoolPref("findbar.modalHighlight");
 
@@ -388,27 +390,27 @@
           prefsvc.getBoolPref("accessibility.typeaheadfind");
         this._typeAheadLinksOnly =
           prefsvc.getBoolPref("accessibility.typeaheadfind.linksonly");
         this._typeAheadCaseSensitive =
           prefsvc.getIntPref("accessibility.typeaheadfind.casesensitive");
         this._entireWord = prefsvc.getBoolPref("findbar.entireword");
         this._highlightAll = prefsvc.getBoolPref("findbar.highlightAll");
 
-        // Convenience.
+        // Convenience
         this.nsITypeAheadFind = Components.interfaces.nsITypeAheadFind;
+        this.nsISelectionController = Components.interfaces.nsISelectionController;
+        this._findSelection = this.nsISelectionController.SELECTION_FIND;
 
         this._findResetTimeout = -1;
 
         // Make sure the FAYT keypress listener is attached by initializing the
-        // browser property.
+        // browser property
         if (this.getAttribute("browserid"))
-          setTimeout(() => this.browser = this.browser, 0);
-
-        this._enableFindButtons(false);
+          setTimeout(function(aSelf) { aSelf.browser = aSelf.browser; }, 0, this);
       ]]></constructor>
 
       <destructor><![CDATA[
         this.destroy();
       ]]></destructor>
 
       <!-- This is necessary because the destructor isn't called when
            we are removed from a document that is not destroyed. This
@@ -993,16 +995,17 @@
               this._entireWord) {
             // Getting here means the user commanded a find op. Make sure any
             // initial prefilling is ignored if it hasn't happened yet.
             if (this._startFindDeferred) {
               this._startFindDeferred.resolve();
               this._startFindDeferred = null;
             }
 
+            this._enableFindButtons(val);
             this._updateCaseSensitivity(val);
             this._setEntireWord();
 
             this.browser.finder.fastFind(val, this._findMode == this.FIND_LINKS,
                                          this._findMode != this.FIND_NORMAL);
           }
 
           if (this._findMode != this.FIND_NORMAL)
@@ -1100,16 +1103,17 @@
             entireWord: this._entireWord,
             highlightAll: this._highlightAll,
             findPrevious: aFindPrevious
           });
           return this.dispatchEvent(event);
         ]]></body>
       </method>
 
+
       <!--
         - Opens the findbar, focuses the findfield and selects its contents.
         - Also flashes the findbar the first time it's used.
         - @param aMode
         -        the find mode to be used, which is either FIND_NORMAL,
         -        FIND_TYPEAHEAD or FIND_LINKS. If not passed, the last
         -        find mode if any or FIND_NORMAL.
         -->
@@ -1264,17 +1268,17 @@
               this._findField.select();
               this._findField.focus();
             }
             this._findFailedString = aData.searchString;
           } else {
             this._findFailedString = null;
           }
 
-          this.updateControlState(aData.result, aData.findBackwards);
+          this._updateStatusUI(aData.result, aData.findBackwards);
           this._updateStatusUIBar(aData.linkURL);
 
           if (this._findMode != this.FIND_NORMAL)
             this._setFindCloseTimeout();
         ]]></body>
       </method>
 
       <!--