Bug 1432746 - Some folders in the Library may not show their bookmark children. r=standard8
Don't use the parent node options when creating new folder nodes, since they should retain
their original options.
Additionally, we can filter nodes in the queries rather than building a lot of nodes that
will be filtered out.
MozReview-Commit-ID: MmlGDe5QgV
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -2124,69 +2124,72 @@ nsNavBookmarks::ResultNodeForContainer(i
NS_ADDREF(*aNode);
return NS_OK;
}
nsresult
nsNavBookmarks::QueryFolderChildren(
int64_t aFolderId,
- nsNavHistoryQueryOptions* aOriginalOptions,
nsNavHistoryQueryOptions* aOptions,
nsCOMArray<nsNavHistoryResultNode>* aChildren)
{
- NS_ENSURE_ARG_POINTER(aOriginalOptions);
NS_ENSURE_ARG_POINTER(aOptions);
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, 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 "
+ "AND (NOT :excludeItems OR "
+ "b.type = :folder OR "
+ "h.url_hash BETWEEN hash('place', 'prefix_lo') AND hash('place', 'prefix_hi')) "
"ORDER BY b.position ASC"
);
NS_ENSURE_STATE(stmt);
mozStorageStatementScoper scoper(stmt);
nsresult rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("parent"), aFolderId);
NS_ENSURE_SUCCESS(rv, rv);
+ rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("folder"), TYPE_FOLDER);
+ NS_ENSURE_SUCCESS(rv, rv);
+ rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("excludeItems"), aOptions->ExcludeItems());
+ NS_ENSURE_SUCCESS(rv, rv);
nsCOMPtr<mozIStorageValueArray> row = do_QueryInterface(stmt, &rv);
NS_ENSURE_SUCCESS(rv, rv);
int32_t index = -1;
bool hasResult;
while (NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) {
- rv = ProcessFolderNodeRow(row, aOriginalOptions, aOptions, aChildren, index);
+ rv = ProcessFolderNodeRow(row, aOptions, aChildren, index);
NS_ENSURE_SUCCESS(rv, rv);
}
return NS_OK;
}
nsresult
nsNavBookmarks::ProcessFolderNodeRow(
mozIStorageValueArray* aRow,
- nsNavHistoryQueryOptions* aOriginalOptions,
nsNavHistoryQueryOptions* aOptions,
nsCOMArray<nsNavHistoryResultNode>* aChildren,
int32_t& aCurrentIndex)
{
NS_ENSURE_ARG_POINTER(aRow);
- NS_ENSURE_ARG_POINTER(aOriginalOptions);
NS_ENSURE_ARG_POINTER(aOptions);
NS_ENSURE_ARG_POINTER(aChildren);
// The results will be in order of aCurrentIndex. Even if we don't add a node
// because it was excluded, we need to count its index, so do that before
// doing anything else.
aCurrentIndex++;
@@ -2199,24 +2202,20 @@ nsNavBookmarks::ProcessFolderNodeRow(
RefPtr<nsNavHistoryResultNode> node;
if (itemType == TYPE_BOOKMARK) {
nsNavHistory* history = nsNavHistory::GetHistoryService();
NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
rv = history->RowToResult(aRow, aOptions, getter_AddRefs(node));
NS_ENSURE_SUCCESS(rv, rv);
-
uint32_t nodeType;
node->GetType(&nodeType);
- if ((nodeType == nsINavHistoryResultNode::RESULT_TYPE_QUERY &&
- aOptions->ExcludeQueries()) ||
- (nodeType != nsINavHistoryResultNode::RESULT_TYPE_QUERY &&
- nodeType != nsINavHistoryResultNode::RESULT_TYPE_FOLDER_SHORTCUT &&
- aOptions->ExcludeItems())) {
+ if (nodeType == nsINavHistoryResultNode::RESULT_TYPE_QUERY &&
+ aOptions->ExcludeQueries()) {
return NS_OK;
}
}
else if (itemType == TYPE_FOLDER) {
// ExcludeReadOnlyFolders currently means "ExcludeLivemarks" (to be fixed in
// bug 1072833)
if (aOptions->ExcludeReadOnlyFolders()) {
if (IsLivemark(id))
@@ -2227,34 +2226,33 @@ nsNavBookmarks::ProcessFolderNodeRow(
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, aOriginalOptions, id);
+ // Don't use options from the parent to build the new folder node, it will
+ // inherit those later when it's inserted in the result.
+ node = new nsNavHistoryFolderResultNode(title, new nsNavHistoryQueryOptions(), id);
rv = aRow->GetUTF8String(kGetChildrenIndex_Guid, node->mBookmarkGuid);
NS_ENSURE_SUCCESS(rv, rv);
node->GetAsFolder()->mTargetFolderGuid = node->mBookmarkGuid;
rv = aRow->GetInt64(nsNavHistory::kGetInfoIndex_ItemDateAdded,
reinterpret_cast<int64_t*>(&node->mDateAdded));
NS_ENSURE_SUCCESS(rv, rv);
rv = aRow->GetInt64(nsNavHistory::kGetInfoIndex_ItemLastModified,
reinterpret_cast<int64_t*>(&node->mLastModified));
NS_ENSURE_SUCCESS(rv, rv);
}
else {
// This is a separator.
- if (aOptions->ExcludeItems()) {
- return NS_OK;
- }
node = new nsNavHistorySeparatorResultNode();
node->mItemId = id;
rv = aRow->GetUTF8String(kGetChildrenIndex_Guid, node->mBookmarkGuid);
NS_ENSURE_SUCCESS(rv, rv);
rv = aRow->GetInt64(nsNavHistory::kGetInfoIndex_ItemDateAdded,
reinterpret_cast<int64_t*>(&node->mDateAdded));
NS_ENSURE_SUCCESS(rv, rv);
@@ -2270,17 +2268,16 @@ nsNavBookmarks::ProcessFolderNodeRow(
NS_ENSURE_TRUE(aChildren->AppendObject(node), NS_ERROR_OUT_OF_MEMORY);
return NS_OK;
}
nsresult
nsNavBookmarks::QueryFolderChildrenAsync(
nsNavHistoryFolderResultNode* aNode,
- int64_t aFolderId,
mozIStoragePendingStatement** _pendingStmt)
{
NS_ENSURE_ARG_POINTER(aNode);
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
@@ -2289,21 +2286,28 @@ nsNavBookmarks::QueryFolderChildrenAsync
nsCOMPtr<mozIStorageAsyncStatement> stmt = mDB->GetAsyncStatement(
"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 "
+ "AND (NOT :excludeItems OR "
+ "b.type = :folder OR "
+ "h.url_hash BETWEEN hash('place', 'prefix_lo') AND hash('place', 'prefix_hi')) "
"ORDER BY b.position ASC"
);
NS_ENSURE_STATE(stmt);
- nsresult rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("parent"), aFolderId);
+ nsresult rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("parent"), aNode->mTargetFolderItemId);
+ NS_ENSURE_SUCCESS(rv, rv);
+ rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("folder"), TYPE_FOLDER);
+ NS_ENSURE_SUCCESS(rv, rv);
+ rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("excludeItems"), aNode->mOptions->ExcludeItems());
NS_ENSURE_SUCCESS(rv, rv);
nsCOMPtr<mozIStoragePendingStatement> pendingStmt;
rv = stmt->ExecuteAsync(aNode, getter_AddRefs(pendingStmt));
NS_ENSURE_SUCCESS(rv, rv);
NS_IF_ADDREF(*_pendingStmt = pendingStmt);
return NS_OK;
--- a/toolkit/components/places/nsNavBookmarks.h
+++ b/toolkit/components/places/nsNavBookmarks.h
@@ -124,56 +124,50 @@ public:
nsresult ResultNodeForContainer(int64_t aID,
nsNavHistoryQueryOptions* aOptions,
nsNavHistoryResultNode** aNode);
// Find all the children of a folder, using the given query and options.
// For each child, a ResultNode is created and added to |children|.
// The results are ordered by folder position.
nsresult QueryFolderChildren(int64_t aFolderId,
- nsNavHistoryQueryOptions* aOriginalOptions,
nsNavHistoryQueryOptions* aOptions,
nsCOMArray<nsNavHistoryResultNode>* children);
/**
* Turns aRow into a node and appends it to aChildren if it is appropriate to
* do so.
*
* @param aRow
* A Storage statement (in the case of synchronous execution) or row of
* a result set (in the case of asynchronous execution).
- * @param aOriginalOptions
- * The original options of the parent folder node. These are the
- * options used to define the parent node.
* @param aOptions
* The options of the parent folder node. These are the options used
* to fill the parent node.
* @param aChildren
* The children of the parent folder node.
* @param aCurrentIndex
* The index of aRow within the results. When called on the first row,
* this should be set to -1.
*/
nsresult ProcessFolderNodeRow(mozIStorageValueArray* aRow,
- nsNavHistoryQueryOptions* aOriginalOptions,
nsNavHistoryQueryOptions* aOptions,
nsCOMArray<nsNavHistoryResultNode>* aChildren,
int32_t& aCurrentIndex);
/**
* The async version of QueryFolderChildren.
*
* @param aNode
* The folder node that will receive the children.
* @param _pendingStmt
* The Storage pending statement that will be used to control async
* execution.
*/
nsresult QueryFolderChildrenAsync(nsNavHistoryFolderResultNode* aNode,
- int64_t aFolderId,
mozIStoragePendingStatement** _pendingStmt);
/**
* @return index of the new folder in aIndex, whether it was passed in or
* generated by autoincrement.
*
* @note If aFolder is -1, uses the autoincrement id for folder index.
* @note aTitle will be truncated to TITLE_LENGTH_MAX
--- a/toolkit/components/places/nsNavHistoryResult.cpp
+++ b/toolkit/components/places/nsNavHistoryResult.cpp
@@ -3151,17 +3151,16 @@ nsNavHistoryFolderResultNode::FillChildr
NS_ASSERTION(mChildren.Count() == 0,
"We are trying to fill children when there already are some");
nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
NS_ENSURE_TRUE(bookmarks, NS_ERROR_OUT_OF_MEMORY);
// Actually get the folder children from the bookmark service.
nsresult rv = bookmarks->QueryFolderChildren(mTargetFolderItemId,
- mOriginalOptions,
mOptions,
&mChildren);
NS_ENSURE_SUCCESS(rv, rv);
// PERFORMANCE: it may be better to also fill any child folders at this point
// so that we can draw tree twisties without doing a separate query later.
// If we don't end up drawing twisties a lot, it doesn't matter. If we do
// this, we should wrap everything in a transaction here on the bookmark
@@ -3242,18 +3241,17 @@ nsNavHistoryFolderResultNode::FillChildr
// ProcessFolderNodeChild, called in HandleResult, increments this for every
// result row it processes. Initialize it here as we begin async execution.
mAsyncBookmarkIndex = -1;
nsNavBookmarks* bmSvc = nsNavBookmarks::GetBookmarksService();
NS_ENSURE_TRUE(bmSvc, NS_ERROR_OUT_OF_MEMORY);
nsresult rv =
- bmSvc->QueryFolderChildrenAsync(this, mTargetFolderItemId,
- getter_AddRefs(mAsyncPendingStmt));
+ bmSvc->QueryFolderChildrenAsync(this, getter_AddRefs(mAsyncPendingStmt));
NS_ENSURE_SUCCESS(rv, rv);
// Register with the result for updates. All updates during async execution
// will cause it to be restarted.
EnsureRegisteredAsFolderObserver();
return NS_OK;
}
@@ -3275,18 +3273,17 @@ nsNavHistoryFolderResultNode::HandleResu
if (!bmSvc) {
CancelAsyncOpen(false);
return NS_ERROR_OUT_OF_MEMORY;
}
// Consume all the currently available rows of the result set.
nsCOMPtr<mozIStorageRow> row;
while (NS_SUCCEEDED(aResultSet->GetNextRow(getter_AddRefs(row))) && row) {
- nsresult rv = bmSvc->ProcessFolderNodeRow(row, mOriginalOptions,
- mOptions, &mChildren,
+ nsresult rv = bmSvc->ProcessFolderNodeRow(row, mOptions, &mChildren,
mAsyncBookmarkIndex);
if (NS_FAILED(rv)) {
CancelAsyncOpen(false);
return rv;
}
}
return NS_OK;
@@ -3572,17 +3569,19 @@ nsNavHistoryFolderResultNode::OnItemAdde
nsNavHistory* history = nsNavHistory::GetHistoryService();
NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
rv = history->BookmarkIdToResultNode(aItemId, mOptions, getter_AddRefs(node));
NS_ENSURE_SUCCESS(rv, rv);
}
else if (aItemType == nsINavBookmarksService::TYPE_FOLDER) {
nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
NS_ENSURE_TRUE(bookmarks, NS_ERROR_OUT_OF_MEMORY);
- rv = bookmarks->ResultNodeForContainer(aItemId, mOptions, getter_AddRefs(node));
+ rv = bookmarks->ResultNodeForContainer(aItemId,
+ new nsNavHistoryQueryOptions(),
+ getter_AddRefs(node));
NS_ENSURE_SUCCESS(rv, rv);
}
else if (aItemType == nsINavBookmarksService::TYPE_SEPARATOR) {
node = new nsNavHistorySeparatorResultNode();
NS_ENSURE_TRUE(node, NS_ERROR_OUT_OF_MEMORY);
node->mItemId = aItemId;
node->mBookmarkGuid = aGUID;
node->mDateAdded = aDateAdded;
--- a/toolkit/components/places/tests/queries/test_options_inherit.js
+++ b/toolkit/components/places/tests/queries/test_options_inherit.js
@@ -17,51 +17,78 @@ add_task(async function() {
type: PlacesUtils.bookmarks.TYPE_FOLDER,
children: [
{ title: "query",
url: "place:queryType=" + Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS,
},
{ title: "bm",
url: "http://example.com",
},
+ {
+ type: PlacesUtils.bookmarks.TYPE_SEPARATOR,
+ },
]
},
{ title: "bm",
url: "http://example.com",
},
+ {
+ type: PlacesUtils.bookmarks.TYPE_SEPARATOR,
+ },
]
});
- await test_query({}, 2, 2, 3);
- await test_query({ expandQueries: false }, 2, 2, 0);
+ await test_query({}, 3, 3, 3);
+ await test_query({ expandQueries: false }, 3, 3, 0);
await test_query({ excludeItems: true }, 1, 1, 0);
await test_query({ excludeItems: true, expandQueries: false }, 1, 1, 0);
await test_query({ excludeItems: true, excludeQueries: true }, 1, 0, 0);
});
-
async function test_query(opts, expectedRootCc, expectedFolderCc, expectedQueryCc) {
let query = PlacesUtils.history.getNewQuery();
query.setFolders([PlacesUtils.unfiledBookmarksFolderId], 1);
let options = PlacesUtils.history.getNewQueryOptions();
for (const [o, v] of Object.entries(opts)) {
- info(`setting ${o} to ${v}`);
+ info(`Setting ${o} to ${v}`);
options[o] = v;
}
let root = PlacesUtils.history.executeQuery(query, options).root;
root.containerOpen = true;
Assert.equal(root.childCount, expectedRootCc, "Checking root child count");
if (root.childCount > 0) {
let folder = root.getChild(0);
Assert.equal(folder.title, "folder", "Found the expected folder");
+
+ // Check the folder uri doesn't reflect the root options, since those
+ // options are inherited and not part of this node declaration.
+ checkURIOptions(folder.uri);
+
PlacesUtils.asContainer(folder).containerOpen = true;
Assert.equal(folder.childCount, expectedFolderCc, "Checking folder child count");
if (folder.childCount) {
let placeQuery = folder.getChild(0);
PlacesUtils.asQuery(placeQuery).containerOpen = true;
Assert.equal(placeQuery.childCount, expectedQueryCc, "Checking query child count");
placeQuery.containerOpen = false;
}
folder.containerOpen = false;
}
+ let f = await PlacesUtils.bookmarks.insert({
+ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+ type: PlacesUtils.bookmarks.TYPE_FOLDER
+ });
+ checkURIOptions(root.getChild(root.childCount - 1).uri);
+ await PlacesUtils.bookmarks.remove(f);
+
root.containerOpen = false;
}
+
+function checkURIOptions(uri) {
+ info("Checking options for uri " + uri);
+ let folderOptions = { };
+ PlacesUtils.history.queryStringToQueries(uri, {}, {}, folderOptions);
+ folderOptions = folderOptions.value;
+ Assert.equal(folderOptions.excludeItems, false, "ExcludeItems should not be changed");
+ Assert.equal(folderOptions.excludeQueries, false, "ExcludeQueries should not be changed");
+ Assert.equal(folderOptions.expandQueries, true, "ExpandQueries should not be changed");
+}