Bug 1379412 - Don't fall back to the history visit title when fetching folder children. r=mak draft
authorKit Cambridge <kit@yakshaving.ninja>
Mon, 10 Jul 2017 11:27:39 -0700
changeset 607666 d06d92a7c780ef561d2e5add47746773ea591577
parent 606124 91c943f7373722ad4e122d98a2ddd6c79708b732
child 637115 e33fd2acf454355ca37061a6a36beadb773042cc
push id68076
push userbmo:kit@mozilla.com
push dateWed, 12 Jul 2017 17:41:02 +0000
reviewersmak
bugs1379412
milestone56.0a1
Bug 1379412 - Don't fall back to the history visit title when fetching folder children. r=mak MozReview-Commit-ID: 3ynWSLAPDZ3
toolkit/components/places/nsNavBookmarks.cpp
toolkit/components/places/tests/bookmarks/test_untitled.js
toolkit/components/places/tests/bookmarks/xpcshell.ini
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -1096,17 +1096,17 @@ nsNavBookmarks::GetDescendantChildren(in
   {
     // Collect children informations.
     // Select all children of a given folder, sorted by position.
     // This is a LEFT JOIN because not all bookmarks types have a place.
     // We construct a result where the first columns exactly match
     // kGetInfoIndex_* order, and additionally contains columns for position,
     // item_child, and folder_child from moz_bookmarks.
     nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
-      "SELECT h.id, h.url, IFNULL(b.title, h.title), h.rev_host, h.visit_count, "
+      "SELECT h.id, h.url, b.title, h.rev_host, h.visit_count, "
              "h.last_visit_date, null, b.id, b.dateAdded, b.lastModified, "
              "b.parent, null, h.frecency, h.hidden, h.guid, null, null, null, "
              "b.guid, b.position, b.type, b.fk, b.syncStatus "
       "FROM moz_bookmarks b "
       "LEFT JOIN moz_places h ON b.fk = h.id "
       "WHERE b.parent = :parent "
       "ORDER BY b.position ASC"
     );
@@ -2130,17 +2130,17 @@ nsNavBookmarks::QueryFolderChildren(
   NS_ENSURE_ARG_POINTER(aChildren);
 
   // Select all children of a given folder, sorted by position.
   // This is a LEFT JOIN because not all bookmarks types have a place.
   // We construct a result where the first columns exactly match those returned
   // by mDBGetURLPageInfo, and additionally contains columns for position,
   // item_child, and folder_child from moz_bookmarks.
   nsCOMPtr<mozIStorageStatement> stmt = mDB->GetStatement(
-    "SELECT h.id, h.url, IFNULL(b.title, h.title), h.rev_host, h.visit_count, "
+    "SELECT h.id, h.url, b.title, h.rev_host, h.visit_count, "
            "h.last_visit_date, null, b.id, b.dateAdded, b.lastModified, "
            "b.parent, null, h.frecency, h.hidden, h.guid, null, null, null, "
            "b.guid, b.position, b.type, b.fk "
     "FROM moz_bookmarks b "
     "LEFT JOIN moz_places h ON b.fk = h.id "
     "WHERE b.parent = :parent "
     "ORDER BY b.position ASC"
   );
@@ -2209,18 +2209,23 @@ nsNavBookmarks::ProcessFolderNodeRow(
     // ExcludeReadOnlyFolders currently means "ExcludeLivemarks" (to be fixed in
     // bug 1072833)
     if (aOptions->ExcludeReadOnlyFolders()) {
       if (IsLivemark(id))
         return NS_OK;
     }
 
     nsAutoCString title;
-    rv = aRow->GetUTF8String(nsNavHistory::kGetInfoIndex_Title, title);
+    bool isNull;
+    rv = aRow->GetIsNull(nsNavHistory::kGetInfoIndex_Title, &isNull);
     NS_ENSURE_SUCCESS(rv, rv);
+    if (!isNull) {
+      rv = aRow->GetUTF8String(nsNavHistory::kGetInfoIndex_Title, title);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
 
     node = new nsNavHistoryFolderResultNode(title, aOptions, id);
 
     rv = aRow->GetUTF8String(kGetChildrenIndex_Guid, node->mBookmarkGuid);
     NS_ENSURE_SUCCESS(rv, rv);
     node->GetAsFolder()->mTargetFolderGuid = node->mBookmarkGuid;
 
     rv = aRow->GetInt64(nsNavHistory::kGetInfoIndex_ItemDateAdded,
@@ -2267,17 +2272,17 @@ nsNavBookmarks::QueryFolderChildrenAsync
   NS_ENSURE_ARG_POINTER(_pendingStmt);
 
   // Select all children of a given folder, sorted by position.
   // This is a LEFT JOIN because not all bookmarks types have a place.
   // We construct a result where the first columns exactly match those returned
   // by mDBGetURLPageInfo, and additionally contains columns for position,
   // item_child, and folder_child from moz_bookmarks.
   nsCOMPtr<mozIStorageAsyncStatement> stmt = mDB->GetAsyncStatement(
-    "SELECT h.id, h.url, IFNULL(b.title, h.title), h.rev_host, h.visit_count, "
+    "SELECT h.id, h.url, b.title, h.rev_host, h.visit_count, "
            "h.last_visit_date, null, b.id, b.dateAdded, b.lastModified, "
            "b.parent, null, h.frecency, h.hidden, h.guid, null, null, null, "
            "b.guid, b.position, b.type, b.fk "
     "FROM moz_bookmarks b "
     "LEFT JOIN moz_places h ON b.fk = h.id "
     "WHERE b.parent = :parent "
     "ORDER BY b.position ASC"
   );
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/bookmarks/test_untitled.js
@@ -0,0 +1,88 @@
+add_task(async function test_untitled_visited_bookmark() {
+  let fxURI = uri("http://getfirefox.com");
+
+  await PlacesUtils.history.insert({
+    url: fxURI,
+    title: "Get Firefox!",
+    visits: [{
+      date: new Date(),
+      transition: PlacesUtils.history.TRANSITIONS.TYPED,
+    }],
+  });
+
+  let fxBmk = await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+    url: fxURI,
+  });
+  strictEqual(fxBmk.title, "", "Visited bookmark should not have title");
+
+  await PlacesTestUtils.promiseAsyncUpdates();
+
+  let fxBmkId = await PlacesUtils.promiseItemId(fxBmk.guid);
+  strictEqual(PlacesUtils.bookmarks.getItemTitle(fxBmkId), "",
+    "Should return empty string for untitled visited bookmark");
+
+  // add a visit for the bookmark first
+  // make sure title still falls back to URL, not visit title
+  let {root: node} = PlacesUtils.getFolderContents(PlacesUtils.toolbarFolderId);
+  try {
+    let fxBmkNode = node.getChild(0);
+    equal(fxBmkNode.itemId, fxBmkId, "Visited bookmark ID should match");
+    strictEqual(fxBmkNode.title, "",
+      "Visited bookmark node should not have title");
+  } finally {
+    node.containerOpen = false;
+  }
+
+  await PlacesUtils.bookmarks.eraseEverything();
+});
+
+add_task(async function test_untitled_unvisited_bookmark() {
+  let tbURI = uri("http://getthunderbird.com");
+
+  let tbBmk = await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+    url: tbURI,
+  });
+  strictEqual(tbBmk.title, "", "Unvisited bookmark should not have title");
+
+  await PlacesTestUtils.promiseAsyncUpdates();
+
+  let tbBmkId = await PlacesUtils.promiseItemId(tbBmk.guid);
+  strictEqual(PlacesUtils.bookmarks.getItemTitle(tbBmkId), "",
+    "Should return empty string for untitled unvisited bookmark");
+
+  let {root: node} = PlacesUtils.getFolderContents(PlacesUtils.toolbarFolderId);
+  try {
+    let tbBmkNode = node.getChild(0);
+    equal(tbBmkNode.itemId, tbBmkId, "Unvisited bookmark ID should match");
+    strictEqual(tbBmkNode.title, "",
+      "Unvisited bookmark node should not have title");
+  } finally {
+    node.containerOpen = false;
+  }
+
+  await PlacesUtils.bookmarks.eraseEverything();
+});
+
+add_task(async function test_untitled_folder() {
+  let folder = await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+    type: PlacesUtils.bookmarks.TYPE_FOLDER,
+  });
+
+  await PlacesTestUtils.promiseAsyncUpdates();
+
+  let folderId = await PlacesUtils.promiseItemId(folder.guid);
+  strictEqual(PlacesUtils.bookmarks.getItemTitle(folderId), "",
+    "Should return empty string for untitled folder");
+
+  let {root: node} = PlacesUtils.getFolderContents(PlacesUtils.toolbarFolderId);
+  try {
+    let folderNode = node.getChild(0);
+    equal(folderNode.itemId, folderId, "Folder ID should match");
+    strictEqual(folderNode.title, "", "Folder node should not have title");
+  } finally {
+    node.containerOpen = false;
+  }
+});
--- a/toolkit/components/places/tests/bookmarks/xpcshell.ini
+++ b/toolkit/components/places/tests/bookmarks/xpcshell.ini
@@ -41,8 +41,9 @@ skip-if = toolkit == 'android'
 [test_getBookmarkedURIFor.js]
 [test_keywords.js]
 [test_nsINavBookmarkObserver.js]
 [test_protectRoots.js]
 [test_removeFolderTransaction_reinsert.js]
 [test_removeItem.js]
 [test_savedsearches.js]
 [test_sync_fields.js]
+[test_untitled.js]