Bug 1432436 - Remove getItemType. r=standard8 draft
authorMarco Bonardo <mbonardo@mozilla.com>
Tue, 23 Jan 2018 16:02:03 +0100
changeset 724060 9f976dcd5a5bdd92aa96c10f86b5c4ec3cdc8701
parent 723950 b0baaec09caf3e1b30ec6b238f5c46ef9b3188be
child 724092 2d453281eb9c77c3f23f15a5a390a17e7b880c34
push id96624
push usermak77@bonardo.net
push dateWed, 24 Jan 2018 10:30:32 +0000
reviewersstandard8
bugs1432436
milestone60.0a1
Bug 1432436 - Remove getItemType. r=standard8 MozReview-Commit-ID: 1gnQzJkYZgH
toolkit/components/places/nsINavBookmarksService.idl
toolkit/components/places/nsNavBookmarks.cpp
toolkit/components/places/nsNavHistory.cpp
toolkit/components/places/tests/bookmarks/test_bookmarks.js
toolkit/components/places/tests/bookmarks/xpcshell.ini
toolkit/components/places/tests/legacy/test_bookmarks.js
toolkit/components/places/tests/legacy/xpcshell.ini
toolkit/components/places/tests/queries/test_bookmarks.js
toolkit/components/places/tests/queries/xpcshell.ini
--- a/toolkit/components/places/nsINavBookmarksService.idl
+++ b/toolkit/components/places/nsINavBookmarksService.idl
@@ -601,22 +601,16 @@ interface nsINavBookmarksService : nsISu
    *
    *  @throws If aNewIndex is out of bounds.
    */
   void setItemIndex(in long long aItemId,
                     in long aNewIndex,
                     [optional] in unsigned short aSource);
 
   /**
-   * Get an item's type (bookmark, separator, folder).
-   * The type is one of the TYPE_* constants defined above.
-   */
-  unsigned short getItemType(in long long aItemId);
-
-  /**
    * Change the bookmarked URI for a bookmark.
    * This changes which "place" the bookmark points at,
    * which means all annotations, etc are carried along.
    */
   void changeBookmarkURI(in long long aItemId,
                          in nsIURI aNewURI,
                          [optional] in unsigned short aSource);
 
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -2078,31 +2078,16 @@ nsNavBookmarks::GetBookmarkURI(int64_t a
 
   rv = NS_NewURI(_URI, bookmark.url);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
 
-NS_IMETHODIMP
-nsNavBookmarks::GetItemType(int64_t aItemId, uint16_t* _type)
-{
-  NS_ENSURE_ARG_MIN(aItemId, 1);
-  NS_ENSURE_ARG_POINTER(_type);
-
-  BookmarkData bookmark;
-  nsresult rv = FetchItemInfo(aItemId, bookmark);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  *_type = static_cast<uint16_t>(bookmark.type);
-  return NS_OK;
-}
-
-
 nsresult
 nsNavBookmarks::ResultNodeForContainer(int64_t aItemId,
                                        nsNavHistoryQueryOptions* aOptions,
                                        nsNavHistoryResultNode** aNode)
 {
   BookmarkData bookmark;
   nsresult rv = FetchItemInfo(aItemId, bookmark);
   NS_ENSURE_SUCCESS(rv, rv);
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -806,30 +806,33 @@ nsNavHistory::GetUpdateRequirements(cons
   }
 
   bool nonTimeBasedItems = false;
   bool domainBasedItems = false;
 
   for (i = 0; i < aQueries.Count(); i ++) {
     nsNavHistoryQuery* query = aQueries[i];
 
+    bool hasSearchTerms = !query->SearchTerms().IsEmpty();
     if (query->Folders().Length() > 0 ||
         query->OnlyBookmarked() ||
-        query->Tags().Length() > 0) {
+        query->Tags().Length() > 0 ||
+        (aOptions->QueryType() == nsINavHistoryQueryOptions::QUERY_TYPE_BOOKMARKS &&
+         hasSearchTerms)) {
       return QUERYUPDATE_COMPLEX_WITH_BOOKMARKS;
     }
 
     // Note: we don't currently have any complex non-bookmarked items, but these
     // are expected to be added. Put detection of these items here.
-    if (!query->SearchTerms().IsEmpty() ||
+    if (hasSearchTerms ||
         !query->Domain().IsVoid() ||
         query->Uri() != nullptr)
       nonTimeBasedItems = true;
 
-    if (! query->Domain().IsVoid())
+    if (!query->Domain().IsVoid())
       domainBasedItems = true;
   }
 
   if (aOptions->ResultType() ==
       nsINavHistoryQueryOptions::RESULTS_AS_TAG_QUERY)
     return QUERYUPDATE_COMPLEX_WITH_BOOKMARKS;
 
   // Whenever there is a maximum number of results,
--- a/toolkit/components/places/tests/bookmarks/xpcshell.ini
+++ b/toolkit/components/places/tests/bookmarks/xpcshell.ini
@@ -21,17 +21,16 @@ skip-if = toolkit == 'android'
 [test_818587_compress-bookmarks-backups.js]
 [test_818593-store-backup-metadata.js]
 [test_992901-backup-unsorted-hierarchy.js]
 [test_997030-bookmarks-html-encode.js]
 [test_1129529.js]
 [test_async_observers.js]
 [test_bmindex.js]
 [test_bookmarkstree_cache.js]
-[test_bookmarks.js]
 [test_bookmarks_eraseEverything.js]
 [test_bookmarks_fetch.js]
 [test_bookmarks_getRecent.js]
 [test_bookmarks_insert.js]
 [test_bookmarks_insertTree.js]
 [test_bookmarks_notifications.js]
 [test_bookmarks_remove.js]
 [test_bookmarks_reorder.js]
rename from toolkit/components/places/tests/bookmarks/test_bookmarks.js
rename to toolkit/components/places/tests/legacy/test_bookmarks.js
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks.js
+++ b/toolkit/components/places/tests/legacy/test_bookmarks.js
@@ -111,19 +111,16 @@ add_task(async function test_bookmarks()
   Assert.equal(bookmarksObserver._itemAddedParent, root);
   Assert.equal(bookmarksObserver._itemAddedIndex, bmStartIndex);
   Assert.equal(bookmarksObserver._itemAddedURI, null);
   let testStartIndex = 0;
 
   // test getItemIndex for folders
   Assert.equal(bs.getItemIndex(testRoot), bmStartIndex);
 
-  // test getItemType for folders
-  Assert.equal(bs.getItemType(testRoot), bs.TYPE_FOLDER);
-
   // insert a bookmark.
   // the time before we insert, in microseconds
   let beforeInsert = Date.now() * 1000;
   Assert.ok(beforeInsert > 0);
 
   let newId = bs.insertBookmark(testRoot, uri("http://google.com/"),
                                 bs.DEFAULT_INDEX, "");
   Assert.equal(bookmarksObserver._itemAddedId, newId);
@@ -170,19 +167,16 @@ add_task(async function test_bookmarks()
   info("lastModified2 = " + lastModified2);
   Assert.ok(is_time_ordered(lastModified, lastModified2));
   Assert.ok(is_time_ordered(dateAdded, lastModified2));
 
   // get item title
   let title = bs.getItemTitle(newId);
   Assert.equal(title, "Google");
 
-  // test getItemType for bookmarks
-  Assert.equal(bs.getItemType(newId), bs.TYPE_BOOKMARK);
-
   // get item title bad input
   try {
     bs.getItemTitle(-3);
     do_throw("getItemTitle accepted bad input");
   } catch (ex) {}
 
   // get the folder that the bookmark is in
   let folderId = bs.getFolderIdForItem(newId);
--- a/toolkit/components/places/tests/legacy/xpcshell.ini
+++ b/toolkit/components/places/tests/legacy/xpcshell.ini
@@ -1,12 +1,13 @@
 # This directory is for tests for the legacy, sync APIs as somewhere to put them
 # until we remove the APIs themselves.
 
 [DEFAULT]
 head = head_legacy.js
 firefox-appdir = browser
 
 [test_418643_removeFolderChildren.js]
+[test_bookmarks.js]
 [test_bookmarks_setNullTitle.js]
 [test_changeBookmarkURI.js]
 [test_protectRoots.js]
 [test_removeItem.js]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/queries/test_bookmarks.js
@@ -0,0 +1,95 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+add_task(async function test_eraseEverything() {
+  info("Test folder with eraseEverything");
+  await PlacesUtils.bookmarks.insertTree({
+    guid: PlacesUtils.bookmarks.unfiledGuid,
+    children: [
+      { title: "remove-folder",
+        type: PlacesUtils.bookmarks.TYPE_FOLDER,
+        children: [
+          { url: "http://mozilla.org/", title: "title 1" },
+          { url: "http://mozilla.org/", title: "title 2" },
+          { title: "sub-folder",
+            type: PlacesUtils.bookmarks.TYPE_FOLDER },
+          { type: PlacesUtils.bookmarks.TYPE_SEPARATOR },
+        ]
+      }
+    ]
+  });
+
+  let unfiled = PlacesUtils.getFolderContents(PlacesUtils.unfiledBookmarksFolderId).root;
+  Assert.equal(unfiled.childCount, 1, "There should be 1 folder");
+  let folder = unfiled.getChild(0);
+  // Test dateAdded and lastModified properties.
+  Assert.equal(typeof folder.dateAdded, "number");
+  Assert.ok(folder.dateAdded > 0);
+  Assert.equal(typeof folder.lastModified, "number");
+  Assert.ok(folder.lastModified > 0);
+
+  let root = PlacesUtils.getFolderContents(folder.itemId).root;
+  Assert.equal(root.childCount, 4, "The folder should have 4 children");
+  for (let i = 0; i < root.childCount; ++i) {
+    let node = root.getChild(i);
+    Assert.ok(node.itemId > 0, "The node should have an itemId");
+  }
+  Assert.equal(root.getChild(0).title, "title 1");
+  Assert.equal(root.getChild(1).title, "title 2");
+  await PlacesUtils.bookmarks.eraseEverything();
+  Assert.equal(unfiled.childCount, 0);
+  unfiled.containerOpen = false;
+});
+
+add_task(async function test_search_title() {
+  let title = "ZZZXXXYYY";
+  let bm = await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    url: "http://mozilla.org/",
+    title,
+  });
+  let query = PlacesUtils.history.getNewQuery();
+  query.searchTerms = "ZZZXXXYYY";
+  let options = PlacesUtils.history.getNewQueryOptions();
+  options.queryType = Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS;
+  let root = PlacesUtils.history.executeQuery(query, options).root;
+  root.containerOpen = true;
+  Assert.equal(root.childCount, 1);
+  let node = root.getChild(0);
+  Assert.equal(node.title, title);
+
+  // Test dateAdded and lastModified properties.
+  Assert.equal(typeof node.dateAdded, "number");
+  Assert.ok(node.dateAdded > 0);
+  Assert.equal(typeof node.lastModified, "number");
+  Assert.ok(node.lastModified > 0);
+  Assert.equal(node.bookmarkGuid, bm.guid);
+
+  await PlacesUtils.bookmarks.remove(bm);
+  Assert.equal(root.childCount, 0);
+  root.containerOpen = false;
+});
+
+add_task(async function test_long_title() {
+  let title = Array(TITLE_LENGTH_MAX + 5).join("A");
+  let bm = await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    url: "http://mozilla.org/",
+    title,
+  });
+  let root = PlacesUtils.getFolderContents(PlacesUtils.unfiledBookmarksFolderId).root;
+  root.containerOpen = true;
+  Assert.equal(root.childCount, 1);
+  let node = root.getChild(0);
+  Assert.equal(node.title, title.substr(0, TITLE_LENGTH_MAX));
+
+  // Update with another long title.
+  let newTitle = Array(TITLE_LENGTH_MAX + 5).join("B");
+  bm.title = newTitle;
+  await PlacesUtils.bookmarks.update(bm);
+  Assert.equal(node.title, newTitle.substr(0, TITLE_LENGTH_MAX));
+
+  await PlacesUtils.bookmarks.remove(bm);
+  Assert.equal(root.childCount, 0);
+  root.containerOpen = false;
+});
--- a/toolkit/components/places/tests/queries/xpcshell.ini
+++ b/toolkit/components/places/tests/queries/xpcshell.ini
@@ -2,16 +2,17 @@
 head = head_queries.js
 skip-if = toolkit == 'android'
 
 [test_415716.js]
 [test_abstime-annotation-domain.js]
 [test_abstime-annotation-uri.js]
 [test_async.js]
 skip-if = (os == 'win' && ccov) # Bug 1423667
+[test_bookmarks.js]
 [test_containersQueries_sorting.js]
 [test_history_queries_tags_liveUpdate.js]
 [test_history_queries_titles_liveUpdate.js]
 [test_onlyBookmarked.js]
 [test_options_inherit.js]
 [test_queryMultipleFolder.js]
 [test_querySerialization.js]
 [test_redirects.js]