Issue 4510 - Filter preferences: replace the built-in findbar widget by our own look-alike

chrome/content/ui/filters-search.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 58 matching lines @@
   /**
    * Closes the find bar.
    */
   close: function()
   {
     E("findbar").hidden = true;
   },
 
   /**
    * Performs a filter search.
-   * @param {Integer} [direction]  search direction: -1 (backwards), 0 (forwards
-   *                  starting with current), 1 (forwards starting with next)
+   * @param {Integer} [direction]
+   *   See @link{FilterSearch#search}
+   * @return {String}
+   *   result status, one of "" (success), "notFound", "wrappedEnd",
+   *   "wrappedStart"
    */
-  search: function(direction)
+  _search: function(direction)
   {
     let text = E("findbar-textbox").value.trim();
+    if (!text)
+      return "";
+
     let caseSensitive = E("findbar-case-sensitive").checked;
 
     if (typeof direction == "undefined")
       direction = (text == this.lastSearchString ? 1 : 0);
     this.lastSearchString = text;
 
     function normalizeString(string)
     {
       return caseSensitive ? string : string.toLowerCase();
     }
 
     function findText(startIndex)
     {
       let list = E("filtersTree");
       let col = list.columns.getNamedColumn("col-filter");
       let count = list.view.rowCount;
       for (let i = startIndex + direction; i >= 0 && i < count; i += (direction || 1))
       {
         let filter = normalizeString(list.view.getCellText(i, col));
         if (filter.indexOf(text) >= 0)
         {
           FilterView.selectRow(i);
           return true;
         }
       }
       return false;
     }
 
-    function setStatus(currentStatus)
-    {
-      for (let status of ["wrappedStart", "wrappedEnd", "notFound"])
-        E("findbar-status-" + status).hidden = status != currentStatus;
-      E("findbar-textbox").setAttribute("status", currentStatus);

[Thomas Greiner, 2016/10/12 14:03:13]

Usually, what we do is set a "data-*" attribute on a parent element and
implement the logic in CSS. Thereby you'd avoid having to modify each of the
elements each time and you could even reuse it for the
`#findbar-textbox[status="notFound"]` rule.

[Wladimir Palant, 2016/10/12 15:22:00]

Done.

-    }
-
-    if (!text)
-      return setStatus("");

[Thomas Greiner, 2016/10/12 14:03:13]

Detail: `setStatus()` doesn't have a return value so I assume the reason you did
this is because the previous code actually returned a value.

Since this is no longer the case, however, I'd rather avoid this unnecessary
confusion and write it as two separate lines.

[Wladimir Palant, 2016/10/12 15:22:00]

Actually, I wanted to make sure that each return sets a status. Well, I can do
this differently...

[Thomas Greiner, 2016/10/12 17:18:10]

Thanks!

-
     text = normalizeString(text);
 
     // First try to find the entry in the current list
     if (findText(E("filtersTree").currentIndex))
-      return setStatus("");
+      return "";
 
     // Now go through the other subscriptions
     let result = "";
     let subscriptions = FilterStorage.subscriptions.slice();
     subscriptions.sort((s1, s2) => (s1 instanceof SpecialSubscription) - (s2 instanceof SpecialSubscription));
     let current = subscriptions.indexOf(FilterView.subscription);
     direction = direction || 1;
     for (let i = current + direction; ; i+= direction)
     {
       if (i < 0)
@@ skipping 25 matching lines @@
           E("tabs").selectedIndex = (subscription instanceof SpecialSubscription ? 1 : 0);
           list.ensureElementIsVisible(node);
           list.selectItem(node);
           if (oldFocus)
           {
             oldFocus.focus();
             Utils.runAsync(() => oldFocus.focus());
           }
 
           Utils.runAsync(() => findText(direction == 1 ? -1 :  subscription.filters.length));
-          return setStatus(result);
+          return result;
         }
       }
     }
 
-    return setStatus("notFound");
+    return "notFound";
+  },
+
+  /**
+   * Performs a filter search and displays the resulting search status.
+   * @param {Integer} [direction]
+   *   search direction: -1 (backwards), 0 (forwards starting with current),
+   *   1 (forwards starting with next)
+   */
+  search: function(direction)
+  {
+    E("findbar").setAttribute("data-status", this._search(direction));
   }
 };
 
 window.addEventListener("load", event =>
 {
   E("findbar").setAttribute("data-os", Services.appinfo.OS.toLowerCase());
 });