Bug 1358500 - fix find bar being disabled all the time, r=mikedeboer
☠☠ backed out by d594a107a7ad ☠ ☠
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Mon, 29 May 2017 16:17:56 +0100
changeset 409224 b89704b3bc199bc0f227d103532e187cf81a08e3
parent 409223 7b9687c90aea55f7893ecfb0ccd5f0c954e36eb0
child 409225 958633822c0969edd7796117e741984c13b29354
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer
bugs1358500
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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,29 +67,30 @@ 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();
+  waitForCondition(() => !gFindBar.getElement("highlight").checked,
+                   continueTests3,
+                   "Highlight never reset!");
 }
 
 function continueTests3() {
-  ok(gFindBar.getElement("highlight").checked, "Highlight button reset!");
+  ok(!gFindBar.getElement("highlight").checked, "Highlight button reset!");
   gFindBar.close();
   ok(gFindBar.hidden, "First tab doesn't show find bar!");
   gBrowser.selectedTab = tabs[1];
   ok(!gFindBar.hidden, "Second tab shows find bar!");
   // Test for bug 892384
   is(gFindBar._findField.getAttribute("focused"), "true",
      "Open findbar refocused on tab change!");
   gURLBar.focus();
@@ -123,13 +124,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>
 
       <!--