From a0e2a6b3a1b7f61b99308ab44f5c94d5dae480d0 Mon Sep 17 00:00:00 2001
From: Marco Bonardo <mbonardo@mozilla.com>
Date: Fri, 19 Aug 2011 15:01:21 +0200
Subject: [PATCH] Bug 669905 - Searching in Downloads folder should limit the
 search to downloads. r=dietrich

---
 browser/components/places/content/places.js   |  74 ++++++--
 browser/components/places/content/places.xul  |  12 +-
 .../tests/browser/browser_library_search.js   | 169 ++++++++++--------
 .../chrome/browser/places/places.properties   |   1 +
 4 files changed, 163 insertions(+), 93 deletions(-)

diff --git a/browser/components/places/content/places.js b/browser/components/places/content/places.js
index 51e3f5d666b..d47ea60d175 100644
--- a/browser/components/places/content/places.js
+++ b/browser/components/places/content/places.js
@@ -277,17 +277,30 @@ var PlacesOrganizer = {
    *          the node to set up scope from
    */
   _setSearchScopeForNode: function PO__setScopeForNode(aNode) {
-    var itemId = aNode.itemId;
+    let itemId = aNode.itemId;
+
+    // Set default buttons status.
+    let bookmarksButton = document.getElementById("scopeBarAll");
+    bookmarksButton.hidden = false;
+    let downloadsButton = document.getElementById("scopeBarDownloads");
+    downloadsButton.hidden = true;
+
     if (PlacesUtils.nodeIsHistoryContainer(aNode) ||
         itemId == PlacesUIUtils.leftPaneQueries["History"]) {
       PlacesQueryBuilder.setScope("history");
     }
-    // Default to All Bookmarks for all other nodes, per bug 469437.
-    else
+    else if (itemId == PlacesUIUtils.leftPaneQueries["Downloads"]) {
+      downloadsButton.hidden = false;
+      bookmarksButton.hidden = true;
+      PlacesQueryBuilder.setScope("downloads");
+    }
+    else {
+      // Default to All Bookmarks for all other nodes, per bug 469437.
       PlacesQueryBuilder.setScope("bookmarks");
+    }
 
     // Enable or disable the folder scope button.
-    var folderButton = document.getElementById("scopeBarFolder");
+    let folderButton = document.getElementById("scopeBarFolder");
     folderButton.hidden = !PlacesUtils.nodeIsFolder(aNode) ||
                           itemId == PlacesUIUtils.allBookmarksFolderId;
   },
@@ -901,9 +914,21 @@ var PlacesSearchBox = {
         options.queryType = Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY;
         content.load([query], options);
       }
-      else
+      else {
         content.applyFilter(filterString);
+      }
       break;
+    case "downloads": {
+        let query = PlacesUtils.history.getNewQuery();
+        query.searchTerms = filterString;
+        query.setTransitions([Ci.nsINavHistoryService.TRANSITION_DOWNLOAD], 1);
+        let options = currentOptions.clone();
+        // Make sure we're getting uri results.
+        options.resultType = currentOptions.RESULT_TYPE_URI;
+        options.queryType = Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY;
+        content.load([query], options);
+      break;
+    }
     default:
       throw "Invalid filterCollection on search";
       break;
@@ -933,17 +958,28 @@ var PlacesSearchBox = {
 
   /**
    * Updates the display with the title of the current collection.
-   * @param   title
+   * @param   aTitle
    *          The title of the current collection.
    */
-  updateCollectionTitle: function PSB_updateCollectionTitle(title) {
-    if (title)
-      this.searchFilter.placeholder =
-        PlacesUIUtils.getFormattedString("searchCurrentDefault", [title]);
-    else
-      this.searchFilter.placeholder = this.filterCollection == "history" ?
-                                      PlacesUIUtils.getString("searchHistory") :
-                                      PlacesUIUtils.getString("searchBookmarks");
+  updateCollectionTitle: function PSB_updateCollectionTitle(aTitle) {
+    let title = "";
+    if (aTitle) {
+      title = PlacesUIUtils.getFormattedString("searchCurrentDefault",
+                                               [aTitle]);
+    }
+    else {
+      switch(this.filterCollection) {
+        case "history":
+          title = PlacesUIUtils.getString("searchHistory");
+          break;
+        case "downloads":
+          title = PlacesUIUtils.getString("searchDownloads");
+          break;
+        default:
+          title = PlacesUIUtils.getString("searchBookmarks");                                    
+      }
+    }
+    this.searchFilter.placeholder = title;
   },
 
   /**
@@ -1025,6 +1061,9 @@ var PlacesQueryBuilder = {
     case "scopeBarFolder":
       this.setScope("collection");
       break;
+    case "scopeBarDownloads":
+      this.setScope("downloads");
+      break;
     case "scopeBarAll":
       this.setScope("bookmarks");
       break;
@@ -1040,7 +1079,8 @@ var PlacesQueryBuilder = {
    * PSB_search()).  If there is an active search, it's performed again to
    * update the content tree.
    * @param   aScope
-   *          the search scope, "bookmarks", "collection", or "history"
+   *          The search scope: "bookmarks", "collection", "downloads" or
+   *          "history".
    */
   setScope: function PQB_setScope(aScope) {
     // Determine filterCollection, folders, and scopeButtonId based on aScope.
@@ -1072,6 +1112,10 @@ var PlacesQueryBuilder = {
                    PlacesUtils.toolbarFolderId,
                    PlacesUtils.unfiledBookmarksFolderId);
       break;
+    case "downloads":
+      filterCollection = "downloads";
+      scopeButtonId = "scopeBarDownloads";
+      break;
     default:
       throw "Invalid search scope";
       break;
diff --git a/browser/components/places/content/places.xul b/browser/components/places/content/places.xul
index 63c4517213a..a355037fb79 100644
--- a/browser/components/places/content/places.xul
+++ b/browser/components/places/content/places.xul
@@ -412,18 +412,16 @@
                          oncommand="PlacesQueryBuilder.onScopeSelected(this);"
                          label="&search.scopeBookmarks.label;"
                          accesskey="&search.scopeBookmarks.accesskey;"/>
-          <!--
-          <toolbarbutton id="scopeBarDownloads" class="small-margin"
-                         type="radio" group="scopeBar"
-                         oncommand="PlacesQueryBuilder.onScopeSelected(this);"
-                         label="&search.scopeDownloads.label;"
-                         accesskey="&search.scopeDownloads.accesskey;"/>
-          -->
           <toolbarbutton id="scopeBarHistory" class="small-margin"
                          type="radio" group="scopeBar"
                          oncommand="PlacesQueryBuilder.onScopeSelected(this);"
                          label="&search.scopeHistory.label;"
                          accesskey="&search.scopeHistory.accesskey;"/>
+          <toolbarbutton id="scopeBarDownloads" class="small-margin"
+                         type="radio" group="scopeBar"
+                         oncommand="PlacesQueryBuilder.onScopeSelected(this);"
+                         label="&search.scopeDownloads.label;"
+                         accesskey="&search.scopeDownloads.accesskey;"/>
           <toolbarbutton id="scopeBarFolder" class="small-margin"
                          type="radio" group="scopeBar"
                          oncommand="PlacesQueryBuilder.onScopeSelected(this);"
diff --git a/browser/components/places/tests/browser/browser_library_search.js b/browser/components/places/tests/browser/browser_library_search.js
index 5cc2bfb34c5..c40b4d1b221 100644
--- a/browser/components/places/tests/browser/browser_library_search.js
+++ b/browser/components/places/tests/browser/browser_library_search.js
@@ -61,67 +61,74 @@
  */
 
 const TEST_URL = "http://dummy.mozilla.org/";
+const TEST_DOWNLOAD_URL = "http://dummy.mozilla.org/dummy.pdf";
 
-// Add your tests here.  Each is a function that's called by testHelper().
-var testCases = [
+let gLibrary;
 
-  // All Bookmarks
-  function () {
-    var defScope = getDefaultScope(PlacesUIUtils.allBookmarksFolderId);
+let testCases = [
+  function allBookmarksScope() {
+    let defScope = getDefaultScope(PlacesUIUtils.allBookmarksFolderId);
     search(PlacesUIUtils.allBookmarksFolderId, "dummy", defScope);
-    is(selectScope("scopeBarFolder"), false,
+    ok(!selectScope("scopeBarFolder"),
        "Folder scope should be disabled for All Bookmarks");
-    resetSearch(defScope);
+    ok(selectScope("scopeBarAll"),
+       "Bookmarks scope should be enabled for All Bookmarks");
+    resetSearch("scopeBarAll");
   },
 
-  // History
-  function () {
-    var defScope = getDefaultScope(PlacesUIUtils.leftPaneQueries["History"]);
+  function historyScope() {
+    let defScope = getDefaultScope(PlacesUIUtils.leftPaneQueries["History"]);
     search(PlacesUIUtils.leftPaneQueries["History"], "dummy", defScope);
-    is(selectScope("scopeBarFolder"), false,
+    ok(!selectScope("scopeBarFolder"),
        "Folder scope should be disabled for History");
+    ok(selectScope("scopeBarAll"),
+       "Bookmarks scope should be enabled for History");
+    resetSearch("scopeBarAll");
+  },
+
+  function downloadsScope() {
+    let defScope = getDefaultScope(PlacesUIUtils.leftPaneQueries["Downloads"]);
+    search(PlacesUIUtils.leftPaneQueries["Downloads"], "dummy", defScope);
+    ok(!selectScope("scopeBarFolder"),
+       "Folder scope should be disabled for Downloads");
+    ok(!selectScope("scopeBarAll"),
+       "Bookmarks scope should be disabled for Downloads");
     resetSearch(defScope);
   },
 
-  // Toolbar folder
-  function () {
-    var defScope = getDefaultScope(bmsvc.toolbarFolder);
-    search(bmsvc.toolbarFolder, "dummy", defScope);
-    is(selectScope("scopeBarFolder"), true,
+  function toolbarFolderScope() {
+    let defScope = getDefaultScope(PlacesUtils.toolbarFolderId);
+    search(PlacesUtils.toolbarFolderId, "dummy", defScope);
+    ok(selectScope("scopeBarAll"),
+       "Bookmarks scope should be enabled for toolbar folder");
+    ok(selectScope("scopeBarFolder"),
        "Folder scope should be enabled for toolbar folder");
     // Ensure that folder scope is still selected after resetting and searching
     // again.
     resetSearch("scopeBarFolder");
-    search(bmsvc.toolbarFolder, "dummy", "scopeBarFolder");
+    search(PlacesUtils.toolbarFolderId, "dummy", "scopeBarFolder");
   },
 
-  // A regular non-root subfolder
-  function () {
-    var folderId = bmsvc.createFolder(bmsvc.toolbarFolder,
-                                      "dummy folder",
-                                      bmsvc.DEFAULT_INDEX);
-    var defScope = getDefaultScope(folderId);
+  function subFolderScope() {
+    let folderId = PlacesUtils.bookmarks.createFolder(PlacesUtils.toolbarFolderId,
+                                                      "dummy folder",
+                                                      PlacesUtils.bookmarks.DEFAULT_INDEX);
+    let defScope = getDefaultScope(folderId);
     search(folderId, "dummy", defScope);
-    is(selectScope("scopeBarFolder"), true,
+    ok(selectScope("scopeBarAll"),
+       "Bookmarks scope should be enabled for regularfolder");
+    ok(selectScope("scopeBarFolder"),
        "Folder scope should be enabled for regular subfolder");
     // Ensure that folder scope is still selected after resetting and searching
     // again.
     resetSearch("scopeBarFolder");
     search(folderId, "dummy", "scopeBarFolder");
-    bmsvc.removeItem(folderId);
+    PlacesUtils.bookmarks.removeItem(folderId);
   },
 ];
 
 ///////////////////////////////////////////////////////////////////////////////
 
-var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
-              getService(Ci.nsINavBookmarksService);
-var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].
-                getService(Ci.nsINavHistoryService);
-var libraryWin;
-
-///////////////////////////////////////////////////////////////////////////////
-
 /**
  * Returns the default search scope for a given folder.
  *
@@ -130,9 +137,14 @@ var libraryWin;
  * @return the default scope when the folder is newly selected
  */
 function getDefaultScope(aFolderId) {
-  return aFolderId === PlacesUIUtils.leftPaneQueries["History"] ?
-         "scopeBarHistory" :
-         "scopeBarAll";
+  switch (aFolderId) {
+    case PlacesUIUtils.leftPaneQueries["History"]:
+      return "scopeBarHistory"
+    case PlacesUIUtils.leftPaneQueries["Downloads"]:
+      return "scopeBarDownloads";
+    default:
+      return "scopeBarAll";
+  }
 }
 
 /**
@@ -141,8 +153,8 @@ function getDefaultScope(aFolderId) {
  * @return the ID of the selected scope folder button
  */
 function getSelectedScopeButtonId() {
-  var doc = libraryWin.document;
-  var scopeButtons = doc.getElementById("organizerScopeBar").childNodes;
+  let doc = gLibrary.document;
+  let scopeButtons = doc.getElementById("organizerScopeBar").childNodes;
   for (let i = 0; i < scopeButtons.length; i++) {
     if (scopeButtons[i].checked)
       return scopeButtons[i].id;
@@ -158,8 +170,8 @@ function getSelectedScopeButtonId() {
  * @return an nsINavHistoryQuery object
  */
 function queryStringToQuery(aPlaceURI) {
-  var queries = {};
-  histsvc.queryStringToQueries(aPlaceURI, queries, {}, {});
+  let queries = {};
+  PlacesUtils.history.queryStringToQueries(aPlaceURI, queries, {}, {});
   return queries.value[0];
 }
 
@@ -188,9 +200,9 @@ function resetSearch(aExpectedScopeButtonId) {
  *         after searching the selected scope button should be this
  */
 function search(aFolderId, aSearchStr, aExpectedScopeButtonId) {
-  var doc = libraryWin.document;
-  var folderTree = doc.getElementById("placesList");
-  var contentTree = doc.getElementById("placeContent");
+  let doc = gLibrary.document;
+  let folderTree = doc.getElementById("placesList");
+  let contentTree = doc.getElementById("placeContent");
 
   // First, ensure that selecting the folder in the left pane updates the
   // content tree properly.
@@ -201,10 +213,11 @@ function search(aFolderId, aSearchStr, aExpectedScopeButtonId) {
 
     // getFolders() on a History query returns an empty array, so no use
     // comparing against aFolderId in that case.
-    if (aFolderId !== PlacesUIUtils.leftPaneQueries["History"]) {
+    if (aFolderId !== PlacesUIUtils.leftPaneQueries["History"] &&
+        aFolderId !== PlacesUIUtils.leftPaneQueries["Downloads"]) {
       // contentTree.place should be equal to contentTree.result.root.uri,
       // but it's not until bug 476952 is fixed.
-      var query = queryStringToQuery(contentTree.result.root.uri);
+      let query = queryStringToQuery(contentTree.result.root.uri);
       is(query.getFolders()[0], aFolderId,
          "Content tree's folder should be what was selected in the left pane");
     }
@@ -212,27 +225,41 @@ function search(aFolderId, aSearchStr, aExpectedScopeButtonId) {
 
   // Second, ensure that searching updates the content tree and search UI
   // properly.
-  var searchBox = doc.getElementById("searchFilter");
+  let searchBox = doc.getElementById("searchFilter");
   searchBox.value = aSearchStr;
-  libraryWin.PlacesSearchBox.search(searchBox.value);
-  query = queryStringToQuery(contentTree.result.root.uri);
+  gLibrary.PlacesSearchBox.search(searchBox.value);
+  let query = queryStringToQuery(contentTree.result.root.uri);
   if (aSearchStr) {
     is(query.searchTerms, aSearchStr,
        "Content tree's searchTerms should be text in search box");
     is(doc.getElementById("searchModifiers").hidden, false,
        "Scope bar should not be hidden after searching");
-    if (getSelectedScopeButtonId() == "scopeBarHistory" ||
-        getSelectedScopeButtonId() == "scopeBarAll" ||
-        aFolderId == PlacesUtils.bookmarks.unfiledBookmarksFolder) {
+
+    let scopeButtonId = getSelectedScopeButtonId();
+    if (scopeButtonId == "scopeBarDownloads" ||
+        scopeButtonId == "scopeBarHistory" ||
+        scopeButtonId == "scopeBarAll" ||
+        aFolderId == PlacesUtils.unfiledBookmarksFolderId) {
       // Check that the target node exists in the tree's search results.
-      var node = null;
-      for (var i = 0; i < contentTree.view.rowCount; i++) {
+      let url, count;
+      if (scopeButtonId == "scopeBarDownloads") {
+        url = TEST_DOWNLOAD_URL;
+        count = 1;
+      }
+      else {
+        url = TEST_URL;
+        count = scopeButtonId == "scopeBarHistory" ? 2 : 1;
+      }
+      is(contentTree.view.rowCount, count, "Found correct number of results");
+
+      let node = null;
+      for (let i = 0; i < contentTree.view.rowCount; i++) {
         node = contentTree.view.nodeForTreeIndex(i);
-        if (node.uri === TEST_URL)
+        if (node.uri === url)
           break;
       }
       isnot(node, null, "At least the target node should be in the tree");
-      is(node.uri, TEST_URL, "URI of node should match target URL");
+      is(node.uri, url, "URI of node should match target URL");
     }
   }
   else {
@@ -253,10 +280,10 @@ function search(aFolderId, aSearchStr, aExpectedScopeButtonId) {
  * @return true if the button is enabled, false otherwise
  */
 function selectScope(aScopeButtonId) {
-  var doc = libraryWin.document;
-  var button = doc.getElementById(aScopeButtonId);
+  let doc = gLibrary.document;
+  let button = doc.getElementById(aScopeButtonId);
   isnot(button, null,
-     "Sanity check: scope button with ID " + aScopeButtonId + "should exist");
+     "Sanity check: scope button with ID " + aScopeButtonId + " should exist");
   // Bug 469436 may hide an inappropriate scope button instead of disabling it.
   if (button.disabled || button.hidden)
     return false;
@@ -267,21 +294,17 @@ function selectScope(aScopeButtonId) {
 /**
  * test() contains window-launching boilerplate that calls this to really kick
  * things off.  Add functions to the testCases array, and this will call them.
- *
- * @param  aLibraryWin
- *         the Places Library window
  */
-function testHelper(aLibraryWin) {
-  libraryWin = aLibraryWin;
+function onLibraryAvailable() {
   testCases.forEach(function (aTest) aTest());
-  aLibraryWin.close();
+
+  gLibrary.close();
+  gLibrary = null;
 
   // Cleanup.
   PlacesUtils.tagging.untagURI(PlacesUtils._uri(TEST_URL), ["dummyTag"]);
-  PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.bookmarks.unfiledBookmarksFolder);
-  PlacesUtils.history.QueryInterface(Ci.nsIBrowserHistory).removeAllPages();
-
-  finish();
+  PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.unfiledBookmarksFolderId);
+  waitForClearHistory(finish);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -291,15 +314,19 @@ function test() {
 
   // Sanity:
   ok(PlacesUtils, "PlacesUtils in context");
-  // Add a visit, a bookmark and a tag.
+
+  // Add visits, a bookmark and a tag.
   PlacesUtils.history.addVisit(PlacesUtils._uri(TEST_URL),
                                Date.now() * 1000, null,
                                PlacesUtils.history.TRANSITION_TYPED, false, 0);
-  PlacesUtils.bookmarks.insertBookmark(PlacesUtils.bookmarks.unfiledBookmarksFolder,
+  PlacesUtils.history.addVisit(PlacesUtils._uri(TEST_DOWNLOAD_URL),
+                               Date.now() * 1000, null,
+                               PlacesUtils.history.TRANSITION_DOWNLOAD, false, 0);
+  PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
                                        PlacesUtils._uri(TEST_URL),
                                        PlacesUtils.bookmarks.DEFAULT_INDEX,
                                        "dummy");
   PlacesUtils.tagging.tagURI(PlacesUtils._uri(TEST_URL), ["dummyTag"]);
 
-  openLibrary(testHelper);
+  gLibrary = openLibrary(onLibraryAvailable);
 }
diff --git a/browser/locales/en-US/chrome/browser/places/places.properties b/browser/locales/en-US/chrome/browser/places/places.properties
index cafcd4e5ad3..3bdcb6005e3 100644
--- a/browser/locales/en-US/chrome/browser/places/places.properties
+++ b/browser/locales/en-US/chrome/browser/places/places.properties
@@ -42,6 +42,7 @@ view.sortBy.tags.accesskey=T
 
 searchBookmarks=Search Bookmarks
 searchHistory=Search History
+searchDownloads=Search Downloads
 searchCurrentDefault=Search in '%S'
 findInPrefix=Find in '%S'…