Bug 1358500 - Don't disable the findbar buttons when the search string is not found. r?Gijs draft
authorMike de Boer <mdeboer@mozilla.com>
Mon, 15 May 2017 14:23:37 +0200
changeset 577813 ccde36d65d1905c78431b6ec53cef78b687b4d22
parent 577812 e50a0a8274b37c7e5d7c1ef722a465645159841e
child 628596 b575b25a2c1d400a47f58fc5714f7a52e56ffd4c
push id58794
push usermdeboer@mozilla.com
push dateMon, 15 May 2017 12:27:10 +0000
reviewersGijs
bugs1358500, 935521
milestone55.0a1
Bug 1358500 - Don't disable the findbar buttons when the search string is not found. r?Gijs Since bug 935521 introduced a disabled state for the findbar buttons, we also overhauled and fine-tuned their behavior. The case we missed there was that when the findbar is open on a page, but the with its state NOT_FOUND thus the buttons disabled, and the user navigates to another page, the buttons would not re-enable. Therefore I remove this behavior, since navigation is more important and there are too many cases where it's not valid. MozReview-Commit-ID: 9VnEChFo7Iu
toolkit/content/tests/browser/browser_findbar.js
toolkit/content/widgets/findbar.xml
--- a/toolkit/content/tests/browser/browser_findbar.js
+++ b/toolkit/content/tests/browser/browser_findbar.js
@@ -157,17 +157,17 @@ add_task(async function test_reinitializ
 
   await BrowserTestUtils.removeTab(tab);
 });
 
 /**
  * Ensure that the initial typed characters aren't lost immediately after
  * opening the find bar.
  */
-add_task(async function() {
+add_task(async function test_no_lost_characters() {
   // This test only makes sence in e10s evironment.
   if (!gMultiProcessBrowser) {
     info("Skipping this test because of non-e10s environment.");
     return;
   }
 
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_PAGE_URI);
   let browser = tab.linkedBrowser;
@@ -192,16 +192,48 @@ add_task(async function() {
   await Promise.all(promises);
   is(document.activeElement, findBar._findField.inputField,
     "findbar is now focused");
   is(findBar._findField.value, "abc", "abc fully entered as find query");
 
   await BrowserTestUtils.removeTab(tab);
 });
 
+add_task(async function test_find_buttons_state() {
+  await BrowserTestUtils.withNewTab({
+    gBrowser,
+    url: `data:text/html;charset=utf-8,
+      <div>
+        <a href="data:text/html,%3Cdiv%3Esomething%3C%2Fdiv%3E">nothing</a>
+      </div>`
+  }, async function(browser) {
+    await promiseFindFinished("something", false);
+
+    let findbar = gBrowser.getFindBar();
+    ok(!findbar.getElement("find-next").disabled, "Next button should be enabled");
+    ok(!findbar.getElement("find-previous").disabled, "Previous button should be enabled");
+
+    let promise = new Promise(resolve => {
+      let listener = () => {
+        browser.messageManager.removeMessageListener("Content:LocationChange", listener);
+        resolve();
+      };
+      browser.messageManager.addMessageListener("Content:LocationChange", listener);
+    });
+    await ContentTask.spawn(browser, null, async function() {
+      const {document} = content;
+      document.getElementsByTagName("a")[0].click();
+    });
+    await promise;
+
+    ok(!findbar.getElement("find-next").disabled, "Next button should be enabled");
+    ok(!findbar.getElement("find-previous").disabled, "Previous button should be enabled");
+  });
+});
+
 function promiseFindFinished(searchText, highlightOn) {
   return new Promise(resolve => {
 
     let findbar = gBrowser.getFindBar();
     findbar.startFind(findbar.FIND_NORMAL);
     let highlightElement = findbar.getElement("highlight");
     if (highlightElement.checked != highlightOn)
       highlightElement.click();
--- a/toolkit/content/widgets/findbar.xml
+++ b/toolkit/content/widgets/findbar.xml
@@ -1079,18 +1079,17 @@
         ]]></body>
       </method>
 
       <method name="updateControlState">
         <parameter name="aResult"/>
         <parameter name="aFindPrevious"/>
         <body><![CDATA[
           this._updateStatusUI(aResult, aFindPrevious);
-          this._enableFindButtons(aResult !== this.nsITypeAheadFind.FIND_NOTFOUND &&
-            !!this._findField.value);
+          this._enableFindButtons(!!this._findField.value);
         ]]></body>
       </method>
 
       <method name="_dispatchFindEvent">
         <parameter name="aType"/>
         <parameter name="aFindPrevious"/>
         <body><![CDATA[
           let event = document.createEvent("CustomEvent");