Bug 1085291 - A bookmark node that is inserted by live-update code is missing bookmarkGuid value. r=mak.

toolkit/components/places/nsNavBookmarks.h

@@ -210,6 +210,11 @@ public:
   nsresult GetDescendantFolders(int64_t aFolderId,
                                 nsTArray<int64_t>& aDescendantFoldersArray);
 
+  static const int32_t kGetChildrenIndex_Guid;
+  static const int32_t kGetChildrenIndex_Position;
+  static const int32_t kGetChildrenIndex_Type;
+  static const int32_t kGetChildrenIndex_PlaceID;
+
 private:
   static nsNavBookmarks* gBookmarksService;
 
@@ -356,11 +361,6 @@ private:
 
   int64_t RecursiveFindRedirectedBookmark(int64_t aPlaceId);
 
-  static const int32_t kGetChildrenIndex_Position;
-  static const int32_t kGetChildrenIndex_Type;
-  static const int32_t kGetChildrenIndex_PlaceID;
-  static const int32_t kGetChildrenIndex_Guid;
-
   class RemoveFolderTransaction MOZ_FINAL : public nsITransaction {
   public:
     explicit RemoveFolderTransaction(int64_t aID) : mID(aID) {}

toolkit/components/places/nsNavHistory.cpp

@@ -1512,7 +1512,8 @@ PlacesSQLQueryBuilder::SelectAsURI()
           "SELECT b2.fk, h.url, COALESCE(b2.title, h.title) AS page_title, "
             "h.rev_host, h.visit_count, h.last_visit_date, f.url, b2.id, "
             "b2.dateAdded, b2.lastModified, b2.parent, ") +
-            tagsSqlFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid "
+            tagsSqlFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, "
+            "b2.guid, b2.position, b2.type, b2.fk "
           "FROM moz_bookmarks b2 "
           "JOIN (SELECT b.fk "
                 "FROM moz_bookmarks b "
@@ -1536,7 +1537,8 @@ PlacesSQLQueryBuilder::SelectAsURI()
           "SELECT b.fk, h.url, COALESCE(b.title, h.title) AS page_title, "
             "h.rev_host, h.visit_count, h.last_visit_date, f.url, b.id, "
             "b.dateAdded, b.lastModified, b.parent, ") +
-            tagsSqlFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid "
+            tagsSqlFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid,"
+            "b.guid, b.position, b.type, b.fk "
           "FROM moz_bookmarks b "
           "JOIN moz_places h ON b.fk = h.id "
           "LEFT OUTER JOIN moz_favicons f ON h.favicon_id = f.id "
@@ -3902,36 +3904,40 @@ nsNavHistory::RowToResult(mozIStorageValueArray* aRow,
   }
 
   if (IsQueryURI(url)) {
-    // special case "place:" URIs: turn them into containers
-
-    // We should never expose the history title for query nodes if the
-    // bookmark-item's title is set to null (the history title may be the
-    // query string without the place: prefix). Thus we call getItemTitle
-    // explicitly. Doing this in the SQL query would be less performant since
-    // it should be done for all results rather than only for queries.
-    if (itemId != -1) {
-      nsNavBookmarks *bookmarks = nsNavBookmarks::GetBookmarksService();
-      NS_ENSURE_TRUE(bookmarks, NS_ERROR_OUT_OF_MEMORY);
-
-      rv = bookmarks->GetItemTitle(itemId, title);
-      NS_ENSURE_SUCCESS(rv, rv);
-    }
-
+    // Special case "place:" URIs: turn them into containers.
     nsRefPtr<nsNavHistoryResultNode> resultNode;
     rv = QueryRowToResult(itemId, url, title, accessCount, time, favicon,
                           getter_AddRefs(resultNode));
     NS_ENSURE_SUCCESS(rv,rv);
 
-    if (aOptions->ResultType() == nsNavHistoryQueryOptions::RESULTS_AS_TAG_QUERY) {
+    if (itemId != -1) {
+      rv = aRow->GetUTF8String(nsNavBookmarks::kGetChildrenIndex_Guid,
+                               resultNode->mBookmarkGuid);
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      // We should never expose the history title for query nodes if the
+      // bookmark-item's title is set to null (the history title may be the
+      // query string without the place: prefix). Thus we call getItemTitle
+      // explicitly. Doing this in the SQL query would be less performant since
+      // it should be done for all results rather than only for queries.
+      nsNavBookmarks *bookmarks = nsNavBookmarks::GetBookmarksService();
+      NS_ENSURE_TRUE(bookmarks, NS_ERROR_OUT_OF_MEMORY);
+
+      rv = bookmarks->GetItemTitle(itemId, resultNode->mTitle);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
+
+    if (itemId != -1 ||
+        aOptions->ResultType() == nsNavHistoryQueryOptions::RESULTS_AS_TAG_QUERY) {
       // RESULTS_AS_TAG_QUERY has date columns
       resultNode->mDateAdded = aRow->AsInt64(kGetInfoIndex_ItemDateAdded);
       resultNode->mLastModified = aRow->AsInt64(kGetInfoIndex_ItemLastModified);
-    }
-    else if (resultNode->IsFolder()) {
-      // If it's a simple folder node (i.e. a shortcut to another folder), apply
-      // our options for it. However, if the parent type was tag query, we do not
-      // apply them, because it would not yield any results.
-      resultNode->GetAsContainer()->mOptions = aOptions;
+      if (resultNode->IsFolder()) {
+        // If it's a simple folder node (i.e. a shortcut to another folder), apply
+        // our options for it. However, if the parent type was tag query, we do not
+        // apply them, because it would not yield any results.
+        resultNode->GetAsContainer()->mOptions = aOptions;
+      }
     }
 
     resultNode.forget(aResult);
@@ -3946,6 +3952,10 @@ nsNavHistory::RowToResult(mozIStorageValueArray* aRow,
       resultNode->mFolderId = parentId;
       resultNode->mDateAdded = aRow->AsInt64(kGetInfoIndex_ItemDateAdded);
       resultNode->mLastModified = aRow->AsInt64(kGetInfoIndex_ItemLastModified);
+
+      rv = aRow->GetUTF8String(nsNavBookmarks::kGetChildrenIndex_Guid,
+                               resultNode->mBookmarkGuid);
+      NS_ENSURE_SUCCESS(rv, rv);
     }
 
     resultNode->mFrecency = aRow->AsInt32(kGetInfoIndex_Frecency);
@@ -4135,7 +4145,8 @@ nsNavHistory::BookmarkIdToResultNode(int64_t aBookmarkId, nsNavHistoryQueryOptio
       "SELECT b.fk, h.url, COALESCE(b.title, h.title), "
              "h.rev_host, h.visit_count, h.last_visit_date, f.url, b.id, "
              "b.dateAdded, b.lastModified, b.parent, "
-             ) + tagsFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid "
+             ) + tagsFragment + NS_LITERAL_CSTRING(", h.frecency, h.hidden, h.guid, "
+             "b.guid, b.position, b.type, b.fk "
       "FROM moz_bookmarks b "
       "JOIN moz_places h ON b.fk = h.id "
       "LEFT JOIN moz_favicons f ON h.favicon_id = f.id "

toolkit/components/places/nsNavHistoryResult.cpp

@@ -3584,6 +3584,9 @@ nsNavHistoryFolderResultNode::OnItemAdded(int64_t aItemId,
     node = new nsNavHistorySeparatorResultNode();
     NS_ENSURE_TRUE(node, NS_ERROR_OUT_OF_MEMORY);
     node->mItemId = aItemId;
+    node->mBookmarkGuid = aGUID;
+    node->mDateAdded = aDateAdded;
+    node->mLastModified = aDateAdded;
   }
 
   node->mBookmarkIndex = aIndex;

toolkit/components/places/tests/unit/xpcshell.ini

@@ -55,6 +55,7 @@ skip-if = os == "android"
 [test_485442_crash_bug_nsNavHistoryQuery_GetUri.js]
 [test_486978_sort_by_date_queries.js]
 [test_536081.js]
+[test_1085291.js]
 [test_adaptive.js]
 # Bug 676989: test hangs consistently on Android
 skip-if = os == "android"