Bug 881219 - When filtering sources, hiding items in the sources list is redundant, r=rcampbell
authorVictor Porof <vporof@mozilla.com>
Fri, 13 Sep 2013 16:23:14 +0300
changeset 147069 ead76fb1a864c7b411fc4fb7e162c28352079a6b
parent 147068 c5b199404e8a8757396baedc511496c806d1796c
child 147070 f45580c36618439fbbb32e796879f7dff1708964
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersrcampbell
bugs881219
milestone26.0a1
Bug 881219 - When filtering sources, hiding items in the sources list is redundant, r=rcampbell
browser/devtools/debugger/debugger-panes.js
browser/devtools/debugger/debugger-toolbar.js
browser/devtools/debugger/debugger-view.js
browser/devtools/shared/widgets/ViewHelpers.jsm
browser/locales/en-US/chrome/browser/devtools/debugger.properties
--- a/browser/devtools/debugger/debugger-panes.js
+++ b/browser/devtools/debugger/debugger-panes.js
@@ -37,17 +37,16 @@ SourcesView.prototype = Heritage.extend(
     dumpn("Initializing the SourcesView");
 
     this.widget = new SideMenuWidget(document.getElementById("sources"), {
       showCheckboxes: true,
       showArrows: true
     });
 
     this.emptyText = L10N.getStr("noSourcesText");
-    this.unavailableText = L10N.getStr("noMatchingSourcesText");
     this._blackBoxCheckboxTooltip = L10N.getStr("blackBoxCheckboxTooltip");
 
     this._commandset = document.getElementById("debuggerCommands");
     this._popupset = document.getElementById("debuggerPopupset");
     this._cmPopup = document.getElementById("sourceEditorContextMenu");
     this._cbPanel = document.getElementById("conditional-breakpoint-panel");
     this._cbTextbox = document.getElementById("conditional-breakpoint-panel-textbox");
     this._editorDeck = document.getElementById("editor-deck");
--- a/browser/devtools/debugger/debugger-toolbar.js
+++ b/browser/devtools/debugger/debugger-toolbar.js
@@ -338,17 +338,16 @@ ChromeGlobalsView.prototype = Heritage.e
   /**
    * Initialization function, called when the debugger is started.
    */
   initialize: function() {
     dumpn("Initializing the ChromeGlobalsView");
 
     this.widget = document.getElementById("chrome-globals");
     this.emptyText = L10N.getStr("noGlobalsText");
-    this.unavailableText = L10N.getStr("noMatchingGlobalsText");
 
     this.widget.addEventListener("select", this._onSelect, false);
     this.widget.addEventListener("click", this._onClick, false);
 
     // Show an empty label by default.
     this.empty();
   },
 
@@ -916,78 +915,17 @@ FilterView.prototype = {
    * Clears the text from the searchbox and resets any changed view.
    */
   clearSearch: function() {
     this._searchbox.value = "";
     this._searchboxHelpPanel.hidePopup();
   },
 
   /**
-   * Performs a file search if necessary.
-   *
-   * @param string aFile
-   *        The source location to search for.
    */
-  _performFileSearch: function(aFile) {
-    // Don't search for files if the input hasn't changed.
-    if (this._prevSearchedFile == aFile) {
-      return;
-    }
-
-    // This is the target container to be currently filtered. Clicking on a
-    // container generally means it should become a target.
-    let view = this._target;
-
-    // If we're not searching for a file anymore, unhide all the items.
-    if (!aFile) {
-      for (let item in view) {
-        item.target.hidden = false;
-      }
-      view.refresh();
-    }
-    // If the searched file string is valid, hide non-matched items.
-    else {
-      let found = false;
-      let lowerCaseFile = aFile.toLowerCase();
-
-      for (let item in view) {
-        let element = item.target;
-        let lowerCaseLabel = item.label.toLowerCase();
-
-        // Search is not case sensitive, and is tied to the label not the value.
-        if (lowerCaseLabel.match(lowerCaseFile)) {
-          element.hidden = false;
-
-          // Automatically select the first match.
-          if (!found) {
-            found = true;
-            view.selectedItem = item;
-          }
-        }
-        // Item not matched, hide the corresponding node.
-        else {
-          element.hidden = true;
-        }
-      }
-      // If no matches were found, display the appropriate info.
-      if (!found) {
-        view.setUnavailable();
-      }
-    }
-    // Synchronize with the view's filtered sources container.
-    DebuggerView.FilteredSources.syncFileSearch();
-
-    // Hide all the groups with no visible children.
-    view.widget.hideEmptyGroups();
-
-    // Ensure the currently selected item is visible.
-    view.widget.ensureSelectionIsVisible({ withGroup: true });
-
-    // Remember the previously searched file to avoid redundant filtering.
-    this._prevSearchedFile = aFile;
   },
 
   /**
    * Performs a line search if necessary.
    * (Jump to lines in the currently visible source).
    *
    * @param number aLine
    *        The source line number to jump to.
@@ -1021,19 +959,16 @@ FilterView.prototype = {
       if (offset > -1) {
         editor.setSelection(offset, offset + aToken.length)
       }
     }
     // Can't search for tokens and lines at the same time.
     if (this._prevSearchedLine && !aToken) {
       this._target.refresh();
     }
-
-    // Remember the previously searched token to avoid redundant filtering.
-    this._prevSearchedToken = aToken;
   },
 
   /**
    * The click listener for the search container.
    */
   _onClick: function() {
     this._searchboxHelpPanel.openPopup(this._searchbox);
   },
@@ -1339,51 +1274,96 @@ FilteredSourcesView.prototype = Heritage
     dumpn("Destroying the FilteredSourcesView");
 
     this.widget.removeEventListener("select", this._onSelect, false);
     this.widget.removeEventListener("click", this._onClick, false);
     this.anchor = null;
   },
 
   /**
-   * Updates the list of sources displayed in this container.
+   * Schedules searching for a source.
+   *
+   * @param string aToken
+   *        The function to search for.
+   * @param number aWait
+   *        The amount of milliseconds to wait until draining.
    */
-  syncFileSearch: function() {
-    this.empty();
+  scheduleSearch: function(aToken, aWait) {
+    // The amount of time to wait for the requests to settle.
+    let maxDelay = FILE_SEARCH_ACTION_MAX_DELAY;
+    let delay = aWait === undefined ? maxDelay / aToken.length : aWait;
 
-    // If there's no currently searched file, or there are no matches found,
-    // hide the popup and avoid creating the view again.
-    if (!DebuggerView.Filtering.searchedFile ||
-        !DebuggerView.Sources.visibleItems.length) {
-      this.hidden = true;
+    // Allow requests to settle down first.
+    setNamedTimeout("sources-search", delay, () => this._doSearch(aToken));
+  },
+
+  /**
+   * Finds file matches in all the displayed sources.
+   *
+   * @param string aToken
+   *        The string to search for.
+   */
+  _doSearch: function(aToken, aStore = []) {
+    // Don't continue filtering if the searched token is an empty string.
+    // In contrast with function searching, in this case we don't want to
+    // show a list of all the files when no search token was supplied.
+    if (!aToken) {
       return;
     }
 
-    // Get the currently visible items in the sources container.
-    let visibleItems = DebuggerView.Sources.visibleItems;
-    let displayedItems = visibleItems.slice(0, RESULTS_PANEL_MAX_RESULTS);
+    for (let item of DebuggerView.Sources.items) {
+      let lowerCaseLabel = item.label.toLowerCase();
+      let lowerCaseToken = aToken.toLowerCase();
+      if (lowerCaseLabel.match(lowerCaseToken)) {
+        aStore.push(item);
+      }
+
+      // Once the maximum allowed number of results is reached, proceed
+      // with building the UI immediately.
+      if (aStore.length >= RESULTS_PANEL_MAX_RESULTS) {
+        this._syncView(aStore);
+        return;
+      }
+    }
 
-    // Append a location item item to this container.
-    for (let item of displayedItems) {
+    // Couldn't reach the maximum allowed number of results, but that's ok,
+    // continue building the UI.
+    this._syncView(aStore);
+  },
+
+  /**
+   * Updates the list of sources displayed in this container.
+   *
+   * @param array aSearchResults
+   *        The results array, containing search details for each source.
+   */
+  _syncView: function(aSearchResults) {
+    // If there are no matches found, keep the popup hidden and avoid
+    // creating the view.
+    if (!aSearchResults.length) {
+      return;
+    }
+
+    for (let item of aSearchResults) {
+      // Append a location item to this container for each match.
       let trimmedLabel = SourceUtils.trimUrlLength(item.label);
       let trimmedValue = SourceUtils.trimUrlLength(item.value, 0, "start");
-      let locationItem = this.push([trimmedLabel, trimmedValue], {
+
+      this.push([trimmedLabel, trimmedValue], {
+        index: -1, /* specifies on which position should the item be appended */
         relaxed: true, /* this container should allow dupes & degenerates */
         attachment: {
-          fullLabel: item.label,
-          fullValue: item.value
+          url: item.value
         }
       });
     }
 
     // Select the first entry in this container.
     this.selectedIndex = 0;
-
-    // Only display the results panel if there's at least one entry available.
-    this.hidden = this.itemCount == 0;
+    this.hidden = false;
   },
 
   /**
    * The click listener for this container.
    */
   _onClick: function(e) {
     let locationItem = this.getItemForElement(e.target);
     if (locationItem) {
@@ -1395,17 +1375,26 @@ FilteredSourcesView.prototype = Heritage
   /**
    * The select listener for this container.
    *
    * @param object aItem
    *        The item associated with the element to select.
    */
   _onSelect: function({ detail: locationItem }) {
     if (locationItem) {
-      DebuggerView.setEditorLocation(locationItem.attachment.fullValue, 0);
+      let targetUrl = locationItem.attachment.url;
+      let currentLine = DebuggerView.editor.getCaretPosition().line + 1;
+
+      // If the same file is selected, don't change the source editor's
+      // carent or debug locations.
+      if (DebuggerView.Sources.selectedValue == targetUrl) {
+        DebuggerView.setEditorLocation(targetUrl, currentLine, { noDebug: true });
+      } else {
+        DebuggerView.setEditorLocation(targetUrl);
+      }
     }
   }
 });
 
 /**
  * Functions handling the function search UI.
  */
 function FilteredFunctionsView() {
@@ -1442,16 +1431,17 @@ FilteredFunctionsView.prototype = Herita
    * Schedules searching for a function in all of the sources.
    *
    * @param string aToken
    *        The function to search for.
    * @param number aWait
    *        The amount of milliseconds to wait until draining.
    */
   scheduleSearch: function(aToken, aWait) {
+    // The amount of time to wait for the requests to settle.
     let maxDelay = FUNCTION_SEARCH_ACTION_MAX_DELAY;
     let delay = aWait === undefined ? maxDelay / aToken.length : aWait;
 
     // Allow requests to settle down first.
     setNamedTimeout("function-search", delay, () => {
       // Start fetching as many sources as possible, then perform the search.
       let urls = DebuggerView.Sources.values;
       let sourcesFetched = DebuggerController.SourceScripts.getTextForSources(urls);
@@ -1463,80 +1453,74 @@ FilteredFunctionsView.prototype = Herita
    * Finds function matches in all the sources stored in the cache, and groups
    * them by location and line number.
    *
    * @param string aToken
    *        The string to search for.
    * @param array aSources
    *        An array of [url, text] tuples for each source.
    */
-  _doSearch: function(aToken, aSources) {
-    // Get the currently searched token from the filtering input.
+  _doSearch: function(aToken, aSources, aStore = []) {
     // Continue parsing even if the searched token is an empty string, to
     // cache the syntax tree nodes generated by the reflection API.
 
     // Make sure the currently displayed source is parsed first. Once the
     // maximum allowed number of resutls are found, parsing will be halted.
     let currentUrl = DebuggerView.Sources.selectedValue;
     let currentSource = aSources.filter(([sourceUrl]) => sourceUrl == currentUrl)[0];
     aSources.splice(aSources.indexOf(currentSource), 1);
     aSources.unshift(currentSource);
 
     // If not searching for a specific function, only parse the displayed source,
     // which is now the first item in the sources array.
     if (!aToken) {
       aSources.splice(1);
     }
 
-    // Prepare the results array, containing search details for each source.
-    let searchResults = [];
-
     for (let [location, contents] of aSources) {
       let parserMethods = DebuggerController.Parser.get(location, contents);
       let sourceResults = parserMethods.getNamedFunctionDefinitions(aToken);
 
       for (let scriptResult of sourceResults) {
         for (let parseResult of scriptResult.parseResults) {
-          searchResults.push({
+          aStore.push({
             sourceUrl: scriptResult.sourceUrl,
             scriptOffset: scriptResult.scriptOffset,
             functionName: parseResult.functionName,
             functionLocation: parseResult.functionLocation,
             inferredName: parseResult.inferredName,
             inferredChain: parseResult.inferredChain,
             inferredLocation: parseResult.inferredLocation
           });
 
           // Once the maximum allowed number of results is reached, proceed
           // with building the UI immediately.
-          if (searchResults.length >= RESULTS_PANEL_MAX_RESULTS) {
-            this._syncFunctionSearch(searchResults);
+          if (aStore.length >= RESULTS_PANEL_MAX_RESULTS) {
+            this._syncView(aStore);
             return;
           }
         }
       }
     }
+
     // Couldn't reach the maximum allowed number of results, but that's ok,
     // continue building the UI.
-    this._syncFunctionSearch(searchResults);
+    this._syncView(aStore);
   },
 
   /**
    * Updates the list of functions displayed in this container.
    *
    * @param array aSearchResults
    *        The results array, containing search details for each source.
    */
-  _syncFunctionSearch: function(aSearchResults) {
-    this.empty();
-
-    // Show the popup even if the search token is an empty string. If there are
-    // no matches found, hide the popup and avoid creating the view again.
+  _syncView: function(aSearchResults) {
+    // If there are no matches found, keep the popup hidden and avoid
+    // creating the view.
     if (!aSearchResults.length) {
-      this.hidden = true;
       return;
     }
 
     for (let item of aSearchResults) {
       // Some function expressions don't necessarily have a name, but the
       // parser provides us with an inferred name from an enclosing
       // VariableDeclarator, AssignmentExpression, ObjectExpression node.
       if (item.functionName && item.inferredName &&
@@ -1556,33 +1540,31 @@ FilteredFunctionsView.prototype = Herita
       // Some function expressions have unexpected bounds, since they may not
       // necessarily have an associated name defining them.
       if (item.inferredLocation) {
         item.actualLocation = item.inferredLocation;
       } else {
         item.actualLocation = item.functionLocation;
       }
 
-      // Append a function item to this container.
+      // Append a function item to this container for each match.
       let trimmedLabel = SourceUtils.trimUrlLength(item.displayedName + "()");
       let trimmedValue = SourceUtils.trimUrlLength(item.sourceUrl, 0, "start");
       let description = (item.inferredChain || []).join(".");
 
       this.push([trimmedLabel, trimmedValue, description], {
         index: -1, /* specifies on which position should the item be appended */
         relaxed: true, /* this container should allow dupes & degenerates */
         attachment: item
       });
     }
 
     // Select the first entry in this container.
     this.selectedIndex = 0;
-
-    // Only display the results panel if there's at least one entry available.
-    this.hidden = this.itemCount == 0;
+    this.hidden = false;
   },
 
   /**
    * The click listener for this container.
    */
   _onClick: function(e) {
     let functionItem = this.getItemForElement(e.target);
     if (functionItem) {
--- a/browser/devtools/debugger/debugger-view.js
+++ b/browser/devtools/debugger/debugger-view.js
@@ -13,16 +13,17 @@ const STACK_FRAMES_POPUP_SOURCE_URL_MAX_
 const STACK_FRAMES_POPUP_SOURCE_URL_TRIM_SECTION = "center";
 const STACK_FRAMES_SCROLL_DELAY = 100; // ms
 const BREAKPOINT_LINE_TOOLTIP_MAX_LENGTH = 1000; // chars
 const BREAKPOINT_CONDITIONAL_POPUP_POSITION = "before_start";
 const BREAKPOINT_CONDITIONAL_POPUP_OFFSET_X = 7; // px
 const BREAKPOINT_CONDITIONAL_POPUP_OFFSET_Y = -3; // px
 const RESULTS_PANEL_POPUP_POSITION = "before_end";
 const RESULTS_PANEL_MAX_RESULTS = 10;
+const FILE_SEARCH_ACTION_MAX_DELAY = 300; // ms
 const GLOBAL_SEARCH_EXPAND_MAX_RESULTS = 50;
 const GLOBAL_SEARCH_LINE_MAX_LENGTH = 300; // chars
 const GLOBAL_SEARCH_ACTION_MAX_DELAY = 1500; // ms
 const FUNCTION_SEARCH_ACTION_MAX_DELAY = 400; // ms
 const SEARCH_GLOBAL_FLAG = "!";
 const SEARCH_FUNCTION_FLAG = "@";
 const SEARCH_TOKEN_FLAG = "#";
 const SEARCH_LINE_FLAG = ":";
--- a/browser/devtools/shared/widgets/ViewHelpers.jsm
+++ b/browser/devtools/shared/widgets/ViewHelpers.jsm
@@ -762,37 +762,22 @@ this.WidgetMethods = {
 
     this._itemsByLabel.clear();
     this._itemsByValue.clear();
     this._itemsByElement.clear();
     this._stagedItems.length = 0;
   },
 
   /**
-   * Does not remove any item in this container. Instead, it overrides the
-   * current label to signal that it is unavailable and removes the tooltip.
-   */
-  setUnavailable: function() {
-    this._widget.setAttribute("notice", this.unavailableText);
-    this._widget.setAttribute("label", this.unavailableText);
-    this._widget.removeAttribute("tooltiptext");
-  },
-
-  /**
    * The label string automatically added to this container when there are
    * no child nodes present.
    */
   emptyText: "",
 
   /**
-   * The label string added to this container when it is marked as unavailable.
-   */
-  unavailableText: "",
-
-  /**
    * Toggles all the items in this container hidden or visible.
    *
    * This does not change the default filtering predicate, so newly inserted
    * items will always be visible. Use WidgetMethods.filterContents if you care.
    *
    * @param boolean aVisibleFlag
    *        Specifies the intended visibility.
    */
--- a/browser/locales/en-US/chrome/browser/devtools/debugger.properties
+++ b/browser/locales/en-US/chrome/browser/devtools/debugger.properties
@@ -52,33 +52,25 @@ stepInTooltip=Step In (%S)
 # LOCALIZATION NOTE (stepOutTooltip): The label that is displayed on the
 # button that steps out of a function call.
 stepOutTooltip=Step Out (%S)
 
 # LOCALIZATION NOTE (emptyGlobalsText): The text to display in the menulist
 # when there are no chrome globals available.
 noGlobalsText=No globals
 
-# LOCALIZATION NOTE (noMatchingGlobalsText): The text to display in the
-# menulist when there are no matching chrome globals after filtering.
-noMatchingGlobalsText=No matching globals
-
 # LOCALIZATION NOTE (noSourcesText): The text to display in the sources menu
 # when there are no scripts.
 noSourcesText=This page has no sources.
 
 # LOCALIZATION NOTE (blackBoxCheckboxTooltip) = The tooltip text to display when
 # the user hovers over the checkbox used to toggle black boxing its associated
 # source.
 blackBoxCheckboxTooltip=Toggle black boxing
 
-# LOCALIZATION NOTE (noMatchingSourcesText): The text to display in the
-# sources menu when there are no matching scripts after filtering.
-noMatchingSourcesText=No matching sources.
-
 # LOCALIZATION NOTE (noMatchingStringsText): The text to display in the
 # global search results when there are no matching strings after filtering.
 noMatchingStringsText=No matches found
 
 # LOCALIZATION NOTE (emptySearchText): This is the text that appears in the
 # filter text box when it is empty and the scripts container is selected.
 emptySearchText=Search scripts (%S)