Bug 317847 Implement "Save this Search" (r=mano)

browser/components/places/content/places.js

@@ -192,10 +192,6 @@ var PlacesOrganizer = {
     // Items are only excluded on the left pane
     var options = node.queryOptions.clone();
     options.excludeItems = false;
-    // Unset excludeQueries so incremental update is enabled for the content
-    // pane.
-    // XXXmano: remove that once we unset excludeQueries for the left pane.
-    options.excludeQueries = false;
 
     this._content.load(queries, 
                        OptionsFilter.filter(queries, options, null));
@@ -343,6 +339,57 @@ var PlacesOrganizer = {
     }
     gEditItemOverlay.uninitPanel();
     deck.selectedIndex = 0;
+  },
+
+  /**
+   * Save the current search (or advanced query) to the bookmarks root.
+   */
+  saveSearch: function PP_saveSearch() {
+    // Get the place: uri for the query.
+    // If the advanced query builder is showing, use that.
+    var queries = [];
+    var options = this.getCurrentOptions();
+    options.excludeQueries = true;
+    var advancedSearch = document.getElementById("advancedSearch");
+    if (!advancedSearch.collapsed) {
+      queries = PlacesQueryBuilder.queries;
+    }
+    // If not, use the value of the search box.
+    else if (PlacesSearchBox.value && PlacesSearchBox.value.length > 0) {
+      var query = PlacesUtils.history.getNewQuery();
+      query.searchTerms = PlacesSearchBox.value;
+      queries.push(query);
+    }
+    // if there is no query, do nothing.
+    else {
+      // XXX should probably have a dialog here to explain that the user needs to search first.
+     return;
+    }
+    var placeSpec = PlacesUtils.history.queriesToQueryString(queries,
+                                                             queries.length,
+                                                             options);
+    var placeURI = IO.newURI(placeSpec);
+
+    // Prompt the user for a name for the query.
+    // XXX - using prompt service for now; will need to make
+    // a real dialog and localize when we're sure this is the UI we want.
+    var title = PlacesUtils.getString("saveSearch.title");
+    var inputLabel = PlacesUtils.getString("saveSearch.inputLabel");
+    var defaultText = PlacesUtils.getString("saveSearch.defaultText");
+
+    var prompts = Cc["@mozilla.org/embedcomp/prompt-service;1"].
+                    getService(Ci.nsIPromptService);
+    var check = {value: false};
+    var input = {value: defaultText};
+    var save = prompts.prompt(null, title, inputLabel, input, null, check);
+
+    // Don't add the query if the user cancels or clears the seach name.
+    if (!save || input.value == "")
+     return;
+
+    // Add the place: uri as a bookmark under the places root.
+    var txn = PlacesUtils.ptm.createItem(placeURI, PlacesUtils.bookmarks.bookmarksRoot, PlacesUtils.bookmarks.DEFAULT_INDEX, input.value);
+    PlacesUtils.ptm.commitTransaction(txn);
   }
 };
 
@@ -384,8 +431,10 @@ var PlacesSearchBox = {
       PO.setHeaderText(PO.HEADER_TYPE_SEARCH, filterString);
       break;
     case "bookmarks":
-      if (filterString != "")
+      if (filterString) {
         content.applyFilter(filterString, true);
+        PO.setHeaderText(PO.HEADER_TYPE_SEARCH, filterString);
+      }
       else
         PlacesOrganizer.onPlaceSelected();
       break;
@@ -445,6 +494,8 @@ var PlacesSearchBox = {
   },
   set filterCollection(collectionName) {
     this.searchFilter.setAttribute("collection", collectionName);
+    if (this.searchFilter.value)
+      return; // don't overwrite pre-existing search terms
     var newGrayText = null;
     if (collectionName == "collection")
       newGrayText = PlacesOrganizer._places.selectedNode.title;
@@ -484,6 +535,9 @@ var PlacesSearchBox = {
  */
 var PlacesQueryBuilder = {
 
+  queries: [],
+  queryOptions: null,
+
   _numRows: 0,
 
   /**
@@ -906,7 +960,7 @@ var PlacesQueryBuilder = {
   doSearch: function PQB_doSearch() {
     // Create the individual queries.
     var queryType = document.getElementById("advancedSearchType").selectedItem.value;
-    var queries = [];
+    this.queries = [];
     if (queryType == "and")
       queries.push(PlacesUtils.history.getNewQuery());
     var updated = 0;
@@ -922,7 +976,7 @@ var PlacesQueryBuilder = {
         // If they're being OR-ed, add a separate query for each row.
         var query;
         if (queryType == "and")
-          query = queries[0];
+          query = this.queries[0];
         else
           query = PlacesUtils.history.getNewQuery();
         
@@ -930,15 +984,15 @@ var PlacesQueryBuilder = {
         this._queryBuilders[querySubject](query, prefix);
         
         if (queryType == "or")
-          queries.push(query);
+          this.queries.push(query);
           
         ++updated;
       }
     }
     
     // Make sure we're getting uri results, not visits
-    var options = PlacesOrganizer.getCurrentOptions();
+    this.options = PlacesOrganizer.getCurrentOptions();
-    options.resultType = options.RESULT_TYPE_URI;
+    this.options.resultType = options.RESULT_TYPE_URI;
 
     // XXXben - find some public way of doing this!
     PlacesOrganizer._content.load(queries, 

browser/components/places/content/places.xul

@@ -86,7 +86,8 @@
              oncommand="PlacesOrganizer.exportBookmarks();"/>
     <command id="OrganizerCommand_import"
              oncommand="PlacesOrganizer.importBookmarks();"/>
-    <command id="OrganizerCommand_search:save"/>
+    <command id="OrganizerCommand_search:save"
+             oncommand="PlacesOrganizer.saveSearch();"/>
     <command id="OrganizerCommand_search:moreCriteria"
              oncommand="PlacesQueryBuilder.addRow();"/>
   </commandset>

browser/components/places/content/placesOverlay.xul

@@ -155,25 +155,25 @@
               command="cmd_cut"
               label="&cutCmd.label;"
               accesskey="&cutCmd.accesskey;" 
-              selection="separator|link|folder|mixed"
+              selection="separator|link|folder|mixed|query"
               forcehideselection="livemarkChild"/>
     <menuitem id="placesContext_copy"
               command="cmd_copy"
               label="&copyCmd.label;"
               accesskey="&copyCmd.accesskey;" 
-              selection="separator|link|folder"/>
+              selection="separator|link|folder|query"/>
     <menuitem id="placesContext_paste"
               command="cmd_paste"
               label="&pasteCmd.label;"
               accesskey="&pasteCmd.accesskey;"
-              selection="mutable"/>
+              selection="mutable|query"/>
     <menuseparator id="placesContext_editSeparator"/>
     <menuitem id="placesContext_delete"
               command="cmd_delete"
               label="&deleteCmd.label;"
               accesskey="&deleteCmd.accesskey;"
               closemenu="single"
-              selection="host|separator|link|folder|day"
+              selection="host|separator|link|folder|day|query"
               forcehideselection="livemarkChild"/>
     <menuseparator id="placesContext_deleteSeparator"/>
     <menuitem id="placesContext_reload"

browser/locales/en-US/chrome/browser/places/places.properties

@@ -77,3 +77,7 @@ status_foldercount = %S object(s)
 
 SelectImport=Import Bookmarks File
 EnterExport=Export Bookmarks File
+
+saveSearch.title=Save Search
+saveSearch.inputLabel=Name:
+saveSearch.defaultText=New Query

toolkit/components/places/src/nsNavHistory.cpp

@@ -1550,7 +1550,7 @@ nsNavHistory::EvaluateQueryForNode(const nsCOMArray<nsNavHistoryQuery>& aQueries
       nsCOMArray<nsNavHistoryResultNode> inputSet;
       inputSet.AppendObject(aNode);
       nsCOMArray<nsNavHistoryResultNode> filteredSet;
-      nsresult rv = FilterResultSet(inputSet, &filteredSet, query->SearchTerms());
+      nsresult rv = FilterResultSet(nsnull, inputSet, &filteredSet, query->SearchTerms());
       if (NS_FAILED(rv))
         continue;
       if (! filteredSet.Count())
@@ -2412,11 +2412,11 @@ nsNavHistory::GetQueryResults(nsNavHistoryQueryResultNode *aResultNode,
       // keyword search
       if (groupCount == 0) {
         // keyword search with no grouping: can filter directly into the result
-        FilterResultSet(toplevel, aResults, aQueries[0]->SearchTerms());
+        FilterResultSet(aResultNode, toplevel, aResults, aQueries[0]->SearchTerms());
       } else {
         // keyword searching with grouping: need intermediate filtered results
         nsCOMArray<nsNavHistoryResultNode> filteredResults;
-        FilterResultSet(toplevel, &filteredResults, aQueries[0]->SearchTerms());
+        FilterResultSet(aResultNode, toplevel, &filteredResults, aQueries[0]->SearchTerms());
         rv = RecursiveGroup(aResultNode, filteredResults, groupings, groupCount,
                             aResults);
         NS_ENSURE_SUCCESS(rv, rv);
@@ -4096,21 +4096,26 @@ nsNavHistory::GroupByHost(nsNavHistoryQueryResultNode *aResultNode,
 
 // nsNavHistory::FilterResultSet
 //
-//    Currently, this just does title/url filtering. This should be expanded in
+//    This does some post-query-execution filtering:
-//    the future.
+//      - searching on title & url
+//      - excludeQueries
 
 nsresult
-nsNavHistory::FilterResultSet(const nsCOMArray<nsNavHistoryResultNode>& aSet,
+nsNavHistory::FilterResultSet(nsNavHistoryQueryResultNode* aParentNode,
+                              const nsCOMArray<nsNavHistoryResultNode>& aSet,
                               nsCOMArray<nsNavHistoryResultNode>* aFiltered,
                               const nsString& aSearch)
 {
+  nsresult rv;
   nsStringArray terms;
   ParseSearchQuery(aSearch, &terms);
 
-  // if there are no search terms, just return everything (i.e. do nothing)
+  // filter against query options
-  if (terms.Count() == 0) {
+  // XXX only excludeQueries is supported at the moment
-    aFiltered->AppendObjects(aSet);
+  PRBool excludeQueries = PR_FALSE;
-    return NS_OK;
+  if (aParentNode) {
+    rv = aParentNode->mOptions->GetExcludeQueries(&excludeQueries);
+    NS_ENSURE_SUCCESS(rv, rv);
   }
 
   nsCStringArray searchAnnotations;
@@ -4124,6 +4129,14 @@ nsNavHistory::FilterResultSet(const nsCOMArray<nsNavHistoryResultNode>& aSet,
   */
 
   for (PRInt32 nodeIndex = 0; nodeIndex < aSet.Count(); nodeIndex ++) {
+    if (aParentNode) {
+      if (aParentNode->mItemId == aSet[nodeIndex]->mItemId)
+        continue; // filter out nodes that are the same as the parent
+    }
+
+    if (excludeQueries && IsQueryURI(aSet[nodeIndex]->mURI))
+      continue;
+
     PRBool allTermsFound = PR_TRUE;
 
     nsStringArray curAnnotations;
@@ -4141,6 +4154,9 @@ nsNavHistory::FilterResultSet(const nsCOMArray<nsNavHistoryResultNode>& aSet,
     }
     */
 
+    if (terms.Count() == 0) {
+        allTermsFound = PR_TRUE;
+    } else {
       for (PRInt32 termIndex = 0; termIndex < terms.Count(); termIndex ++) {
         PRBool termFound = PR_FALSE;
         // title and URL
@@ -4163,6 +4179,8 @@ nsNavHistory::FilterResultSet(const nsCOMArray<nsNavHistoryResultNode>& aSet,
           break;
         }
       }
+    }
+
     if (allTermsFound)
       aFiltered->AppendObject(aSet[nodeIndex]);
   }
@@ -4318,9 +4336,8 @@ nsNavHistory::RowToResult(mozIStorageValueArray* aRow,
 
   if (IsQueryURI(url)) {
     // special case "place:" URIs: turn them into containers
-    // XXX: should we set the bookmark identifier for this sort of nodes? It
+    PRInt64 itemId = aRow->AsInt64(kGetInfoIndex_ItemId);
-    // would sure break few assumption on the frontend side
+    return QueryRowToResult(itemId, url, title, accessCount, time, favicon, aResult);
-    return QueryRowToResult(url, title, accessCount, time, favicon, aResult);
   } else if (aOptions->ResultType() == nsNavHistoryQueryOptions::RESULTS_AS_URI) {
     *aResult = new nsNavHistoryResultNode(url, title, accessCount, time,
                                           favicon);
@@ -4387,7 +4404,8 @@ nsNavHistory::RowToResult(mozIStorageValueArray* aRow,
 //    folder or query node.
 
 nsresult
-nsNavHistory::QueryRowToResult(const nsACString& aURI, const nsACString& aTitle,
+nsNavHistory::QueryRowToResult(PRInt64 itemId, const nsACString& aURI,
+                               const nsACString& aTitle,
                                PRUint32 aAccessCount, PRTime aTime,
                                const nsACString& aFavicon,
                                nsNavHistoryResultNode** aNode)
@@ -4422,6 +4440,7 @@ nsNavHistory::QueryRowToResult(const nsACString& aURI, const nsACString& aTitle,
                                                queries, options);
       if (! *aNode)
         return NS_ERROR_OUT_OF_MEMORY;
+      (*aNode)->mItemId = itemId;
       NS_ADDREF(*aNode);
     }
   }

toolkit/components/places/src/nsNavHistory.h

@@ -249,7 +249,8 @@ public:
   nsresult RowToResult(mozIStorageValueArray* aRow,
                        nsNavHistoryQueryOptions* aOptions,
                        nsNavHistoryResultNode** aResult);
-  nsresult QueryRowToResult(const nsACString& aURI, const nsACString& aTitle,
+  nsresult QueryRowToResult(PRInt64 aItemId, const nsACString& aURI,
+                            const nsACString& aTitle,
                             PRUint32 aAccessCount, PRTime aTime,
                             const nsACString& aFavicon,
                             nsNavHistoryResultNode** aNode);
@@ -510,7 +511,8 @@ protected:
                        nsCOMArray<nsNavHistoryResultNode>* aDest,
                        PRBool aIsDomain);
 
-  nsresult FilterResultSet(const nsCOMArray<nsNavHistoryResultNode>& aSet,
+  nsresult FilterResultSet(nsNavHistoryQueryResultNode *aParentNode,
+                           const nsCOMArray<nsNavHistoryResultNode>& aSet,
                            nsCOMArray<nsNavHistoryResultNode>* aFiltered,
                            const nsString& aSearch);
 

toolkit/components/places/src/nsNavHistoryQuery.cpp

@@ -470,9 +470,9 @@ nsNavHistory::QueriesToQueryString(nsINavHistoryQuery **aQueries,
   }
 
   // expand queries
-  if (options->ExpandQueries()) {
+  if (!options->ExpandQueries()) {
     AppendAmpersandIfNonempty(queryString);
-    queryString += NS_LITERAL_CSTRING(QUERYKEY_EXPAND_QUERIES "=1");
+    queryString += NS_LITERAL_CSTRING(QUERYKEY_EXPAND_QUERIES "=0");
   }
 
   // include hidden

toolkit/components/places/src/nsNavHistoryQuery.h

@@ -122,7 +122,7 @@ public:
                                mExcludeItems(PR_FALSE),
                                mExcludeQueries(PR_FALSE),
                                mExcludeReadOnlyFolders(PR_FALSE),
-                               mExpandQueries(PR_FALSE),
+                               mExpandQueries(PR_TRUE),
                                mIncludeHidden(PR_FALSE),
                                mShowSessions(PR_FALSE),
                                mMaxResults(0),

toolkit/components/places/src/nsNavHistoryResult.cpp

@@ -1894,14 +1894,19 @@ nsNavHistoryQueryResultNode::nsNavHistoryQueryResultNode(
 //
 //    Whoever made us may want non-expanding queries. However, we always
 //    expand when we are the root node, or else asking for non-expanding
-//    queries would be useless.
+//    queries would be useless. A query node is not expandable if excludeItems=1
+//    or expandQueries=0.
 
 PRBool
 nsNavHistoryQueryResultNode::CanExpand()
 {
   nsNavHistoryQueryOptions* options = GetGeneratingOptions();
-  if (options && options->ExpandQueries())
+  if (options) {
+    if (options->ExcludeItems())
+      return PR_FALSE;
+    if (options->ExpandQueries())
       return PR_TRUE;
+  }
   if (mResult && mResult->mRootNode == this)
     return PR_TRUE;
   return PR_FALSE;
@@ -1939,11 +1944,11 @@ nsNavHistoryQueryResultNode::OnRemoving()
 nsresult
 nsNavHistoryQueryResultNode::OpenContainer()
 {
-  NS_ASSERTION(! mExpanded, "Container must be expanded to close it");
+  NS_ASSERTION(!mExpanded, "Container must be closed to open it");
   mExpanded = PR_TRUE;
-  if (! CanExpand())
+  if (!CanExpand())
     return NS_OK;
-  if (! mContentsValid) {
+  if (!mContentsValid) {
     nsresult rv = FillChildren();
     NS_ENSURE_SUCCESS(rv, rv);
   }
@@ -2568,7 +2573,7 @@ nsNavHistoryQueryResultNode::OnItemChanged(PRInt64 aItemId,
   if (mLiveUpdate == QUERYUPDATE_COMPLEX_WITH_BOOKMARKS)
     return Refresh();
   else
-    NS_NOTREACHED("history observers should not get OnItemChanged, but should get the corresponding history notifications instead");
+    NS_WARNING("history observers should not get OnItemChanged, but should get the corresponding history notifications instead");
   return NS_OK;
 }
 
@@ -3050,8 +3055,21 @@ nsNavHistoryFolderResultNode::OnItemAdded(PRInt64 aItemId,
   nsresult rv = bookmarks->GetItemType(aItemId, &itemType);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  // check for query URIs, which are bookmarks, but treated as containers
+  // in results and views.
+  PRBool isQuery = PR_FALSE;
+  if (itemType == nsINavBookmarksService::TYPE_BOOKMARK) {
+    nsCOMPtr<nsIURI> itemURI;
+    rv = bookmarks->GetBookmarkURI(aItemId, getter_AddRefs(itemURI));
+    NS_ENSURE_SUCCESS(rv, rv);
+    nsCAutoString itemURISpec;
+    rv = itemURI->GetSpec(itemURISpec);
+    NS_ENSURE_SUCCESS(rv, rv);
+    isQuery = IsQueryURI(itemURISpec);
+  }
+
   if (itemType != nsINavBookmarksService::TYPE_FOLDER &&
-      mOptions->ExcludeItems()) {
+      !isQuery && mOptions->ExcludeItems()) {
     // don't update items when we aren't displaying them, but we still need
     // to adjust bookmark indices to account for the insertion
     ReindexRange(aIndex, PR_INT32_MAX, 1);
@@ -3517,7 +3535,7 @@ nsNavHistoryResult::AddAllBookmarksObserver(nsNavHistoryQueryResultNode* aNode)
     mIsAllBookmarksObserver = PR_TRUE;
   }
   if (mAllBookmarksObservers.IndexOf(aNode) != mAllBookmarksObservers.NoIndex) {
-    NS_NOTREACHED("Attempting to register an observer twice!");
+    NS_WARNING("Attempting to register an observer twice!");
     return;
   }
   mAllBookmarksObservers.AppendElement(aNode);

toolkit/components/places/tests/bookmarks/test_bookmarks.js

@@ -443,7 +443,8 @@ function run_test() {
       var node = rootNode.getChild(i);
       if (node.type == node.RESULT_TYPE_FOLDER ||
           node.type == node.RESULT_TYPE_URI ||
-          node.type == node.RESULT_TYPE_SEPARATOR) {
+          node.type == node.RESULT_TYPE_SEPARATOR ||
+          node.type == node.RESULT_TYPE_QUERY) {
         do_check_true(node.itemId > 0);
       }
       else {

toolkit/components/places/tests/bookmarks/test_savedsearches.js

@@ -0,0 +1,250 @@
+/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim:set ts=2 sw=2 sts=2 et: */
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is mozilla.org code.
+ *
+ * The Initial Developer of the Original Code is Google Inc.
+ * Portions created by the Initial Developer are Copyright (C) 2005
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *  Darin Fisher <darin@meer.net>
+ *  Dietrich Ayala <dietrich@mozilla.com>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+// Get bookmark service
+try {
+  var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].getService(Ci.nsINavBookmarksService);
+} catch(ex) {
+  do_throw("Could not get nav-bookmarks-service\n");
+}
+
+// Get history service
+try {
+  var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].getService(Ci.nsINavHistoryService);
+} catch(ex) {
+  do_throw("Could not get history service\n");
+} 
+
+// Get annotation service
+try {
+  var annosvc= Cc["@mozilla.org/browser/annotation-service;1"].getService(Ci.nsIAnnotationService);
+} catch(ex) {
+  do_throw("Could not get annotation service\n");
+} 
+
+// Get global history service
+try {
+  var bhist = Cc["@mozilla.org/browser/global-history;2"].getService(Ci.nsIBrowserHistory);
+} catch(ex) {
+  do_throw("Could not get history service\n");
+} 
+
+// get bookmarks root id
+var root = bmsvc.bookmarksRoot;
+
+// main
+function run_test() {
+  // a search term that matches a default bookmark
+  var searchTerm = "about";
+
+  // create a folder to hold all the tests
+  // this makes the tests more tolerant of changes to the default bookmarks set
+  // also, name it using the search term, for testing that containers that match don't show up in query results
+  var testRoot = bmsvc.createFolder(root, searchTerm, bmsvc.DEFAULT_INDEX);
+
+  /******************************************
+  * saved searches - bookmarks 
+  ******************************************/
+
+  // add a bookmark that matches the search term
+  var bookmarkId = bmsvc.insertBookmark(root, uri("http://foo.com"), bmsvc.DEFAULT_INDEX, searchTerm);
+
+  // create a saved-search that matches a default bookmark
+  var searchId = bmsvc.insertBookmark(testRoot,
+                                      uri("place:terms=" + searchTerm + "&excludeQueries=1&expandQueries=1&queryType=1"),
+                                      bmsvc.DEFAULT_INDEX, searchTerm);
+
+  // query for the test root, expandQueries=0
+  // the query should show up as a regular bookmark
+  try {
+    var options = histsvc.getNewQueryOptions();
+    options.expandQueries = 0;
+    var query = histsvc.getNewQuery();
+    query.setFolders([testRoot], 1);
+    var result = histsvc.executeQuery(query, options);
+    var rootNode = result.root;
+    rootNode.containerOpen = true;
+    var cc = rootNode.childCount;
+    do_check_eq(cc, 1);
+    for (var i = 0; i < cc; i++) {
+      var node = rootNode.getChild(i);
+      // test that queries have valid itemId
+      do_check_true(node.itemId > 0);
+      // test that the container is closed
+      node.QueryInterface(Ci.nsINavHistoryContainerResultNode);
+      do_check_eq(node.containerOpen, false);
+    }
+  }
+  catch(ex) {
+    do_throw("expandQueries=0 query error: " + ex);
+  }
+
+  // bookmark saved search
+  // query for the test root, expandQueries=1
+  // the query should show up as a query container, with 1 child
+  try {
+    var options = histsvc.getNewQueryOptions();
+    options.expandQueries = 1;
+    var query = histsvc.getNewQuery();
+    query.setFolders([testRoot], 1);
+    var result = histsvc.executeQuery(query, options);
+    var rootNode = result.root;
+    rootNode.containerOpen = true;
+    var cc = rootNode.childCount;
+    do_check_eq(cc, 1);
+    for (var i = 0; i < cc; i++) {
+      var node = rootNode.getChild(i);
+      // test that query node type is container when expandQueries=1
+      do_check_eq(node.type, node.RESULT_TYPE_QUERY);
+      // test that queries (as containers) have valid itemId
+      do_check_true(node.itemId > 0);
+      node.QueryInterface(Ci.nsINavHistoryContainerResultNode);
+      node.containerOpen = true;
+
+      // test that queries have children when excludeItems=1
+      // test that query nodes don't show containers (shouldn't have our folder that matches)
+      // test that queries don't show themselves in query results (shouldn't have our saved search)
+      do_check_eq(node.childCount, 1);
+
+      // test that bookmark shows in query results
+      var item = node.getChild(0);
+      do_check_eq(item.itemId, bookmarkId);
+
+      // XXX - FAILING - test live-update of query results - add a bookmark that matches the query
+      //var tmpBmId = bmsvc.insertBookmark(root, uri("http://" + searchTerm + ".com"), bmsvc.DEFAULT_INDEX, searchTerm + "blah");
+      //do_check_eq(query.childCount, 2);
+
+      // XXX - test live-update of query results - delete a bookmark that matches the query
+      //bmsvc.removeItem(tmpBMId);
+      //do_check_eq(query.childCount, 1);
+
+      // test live-update of query results - add a folder that matches the query
+      bmsvc.createFolder(root, searchTerm + "zaa", bmsvc.DEFAULT_INDEX);
+      do_check_eq(node.childCount, 1);
+      // test live-update of query results - add a query that matches the query
+      bmsvc.insertBookmark(root, uri("place:terms=foo&excludeQueries=1&expandQueries=1&queryType=1"),
+                           bmsvc.DEFAULT_INDEX, searchTerm + "blah");
+      do_check_eq(node.childCount, 1);
+    }
+  }
+  catch(ex) {
+    do_throw("expandQueries=1 bookmarks query: " + ex);
+  }
+
+  // delete the bookmark search
+  bmsvc.removeItem(searchId);
+
+  /******************************************
+  * saved searches - history 
+  ******************************************/
+
+  // add a visit that matches the search term
+  var testURI = uri("http://" + searchTerm + ".com");
+  bhist.addPageWithDetails(testURI, searchTerm, Date.now());
+
+  // create a saved-search that matches the visit we added
+  var searchId = bmsvc.insertBookmark(testRoot,
+                                      uri("place:terms=" + searchTerm + "&excludeQueries=1&expandQueries=1&queryType=0"),
+                                      bmsvc.DEFAULT_INDEX, searchTerm);
+
+  // query for the test root, expandQueries=1
+  // the query should show up as a query container, with 1 child
+  try {
+    var options = histsvc.getNewQueryOptions();
+    options.expandQueries = 1;
+    var query = histsvc.getNewQuery();
+    query.setFolders([testRoot], 1);
+    var result = histsvc.executeQuery(query, options);
+    var rootNode = result.root;
+    rootNode.containerOpen = true;
+    var cc = rootNode.childCount;
+    do_check_eq(cc, 1);
+    for (var i = 0; i < cc; i++) {
+      var node = rootNode.getChild(i);
+      // test that query node type is container when expandQueries=1
+      do_check_eq(node.type, node.RESULT_TYPE_QUERY);
+      // test that queries (as containers) have valid itemId
+      do_check_eq(node.itemId, searchId);
+      node.QueryInterface(Ci.nsINavHistoryContainerResultNode);
+      node.containerOpen = true;
+
+      // test that queries have children when excludeItems=1
+      // test that query nodes don't show containers (shouldn't have our folder that matches)
+      // test that queries don't show themselves in query results (shouldn't have our saved search)
+      do_check_eq(node.childCount, 1);
+
+      // test that history visit shows in query results
+      var item = node.getChild(0);
+      do_check_eq(item.type, item.RESULT_TYPE_URI);
+      do_check_eq(item.itemId, -1); // history visit
+      do_check_eq(item.uri, testURI.spec); // history visit
+
+      // test live-update of query results - add a history visit that matches the query
+      bhist.addPageWithDetails(uri("http://foo.com"), searchTerm + "blah", Date.now());
+      do_check_eq(node.childCount, 2);
+
+      // test live-update of query results - delete a history visit that matches the query
+      bhist.removePage(uri("http://foo.com"));
+      do_check_eq(node.childCount, 1);
+    }
+
+    // test live-update of moved queries
+    var tmpFolderId = bmsvc.createFolder(testRoot, "foo", bmsvc.DEFAULT_INDEX); 
+    bmsvc.moveItem(searchId, tmpFolderId, bmsvc.DEFAULT_INDEX);
+    var tmpFolderNode = rootNode.getChild(0);
+    do_check_eq(tmpFolderNode.itemId, tmpFolderId);
+    tmpFolderNode.QueryInterface(Ci.nsINavHistoryContainerResultNode);
+    tmpFolderNode.containerOpen = true;
+    do_check_eq(tmpFolderNode.childCount, 1);
+
+    // test live-update of renamed queries
+    bmsvc.setItemTitle(searchId, "foo");
+    do_check_eq(tmpFolderNode.title, "foo");
+
+    // test live-update of deleted queries
+    bmsvc.removeItem(searchId);
+    try {
+      var tmpFolderNode = root.getChild(1);
+      do_throw("query was not removed");
+    } catch(ex) {}
+  }
+  catch(ex) {
+    do_throw("expandQueries=1 bookmarks query: " + ex);
+  }
+}