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

--HG--
extra : rebase_source : 66eea325825007266e08424630b092b9e8d75b67
This commit is contained in:
Marco Bonardo 2018-01-24 17:55:42 +01:00
Родитель 4af14dc29f
Коммит 52f4c19c6b
4 изменённых файлов: 57 добавлений и 33 удалений

Просмотреть файл

@ -2114,11 +2114,9 @@ nsNavBookmarks::ResultNodeForContainer(int64_t aItemId,
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);
@ -2135,6 +2133,9 @@ nsNavBookmarks::QueryFolderChildren(
"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);
@ -2142,6 +2143,10 @@ nsNavBookmarks::QueryFolderChildren(
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);
@ -2149,7 +2154,7 @@ nsNavBookmarks::QueryFolderChildren(
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);
}
@ -2160,13 +2165,11 @@ nsNavBookmarks::QueryFolderChildren(
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);
@ -2189,14 +2192,10 @@ nsNavBookmarks::ProcessFolderNodeRow(
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;
}
}
@ -2217,7 +2216,9 @@ nsNavBookmarks::ProcessFolderNodeRow(
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);
@ -2232,9 +2233,6 @@ nsNavBookmarks::ProcessFolderNodeRow(
}
else {
// This is a separator.
if (aOptions->ExcludeItems()) {
return NS_OK;
}
node = new nsNavHistorySeparatorResultNode();
node->mItemId = id;
@ -2260,7 +2258,6 @@ nsNavBookmarks::ProcessFolderNodeRow(
nsresult
nsNavBookmarks::QueryFolderChildrenAsync(
nsNavHistoryFolderResultNode* aNode,
int64_t aFolderId,
mozIStoragePendingStatement** _pendingStmt)
{
NS_ENSURE_ARG_POINTER(aNode);
@ -2279,11 +2276,18 @@ nsNavBookmarks::QueryFolderChildrenAsync(
"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;

Просмотреть файл

@ -129,7 +129,6 @@ public:
// 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);
@ -140,9 +139,6 @@ public:
* @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.
@ -153,7 +149,6 @@ public:
* this should be set to -1.
*/
nsresult ProcessFolderNodeRow(mozIStorageValueArray* aRow,
nsNavHistoryQueryOptions* aOriginalOptions,
nsNavHistoryQueryOptions* aOptions,
nsCOMArray<nsNavHistoryResultNode>* aChildren,
int32_t& aCurrentIndex);
@ -168,7 +163,6 @@ public:
* execution.
*/
nsresult QueryFolderChildrenAsync(nsNavHistoryFolderResultNode* aNode,
int64_t aFolderId,
mozIStoragePendingStatement** _pendingStmt);
/**

Просмотреть файл

@ -3156,7 +3156,6 @@ nsNavHistoryFolderResultNode::FillChildren()
// Actually get the folder children from the bookmark service.
nsresult rv = bookmarks->QueryFolderChildren(mTargetFolderItemId,
mOriginalOptions,
mOptions,
&mChildren);
NS_ENSURE_SUCCESS(rv, rv);
@ -3247,8 +3246,7 @@ nsNavHistoryFolderResultNode::FillChildrenAsync()
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
@ -3280,8 +3278,7 @@ nsNavHistoryFolderResultNode::HandleResult(mozIStorageResultSet* aResultSet)
// 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);
@ -3577,7 +3574,9 @@ nsNavHistoryFolderResultNode::OnItemAdded(int64_t aItemId,
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) {

Просмотреть файл

@ -22,28 +22,33 @@ add_task(async function() {
{ 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;
@ -53,6 +58,11 @@ async function test_query(opts, expectedRootCc, expectedFolderCc, expectedQueryC
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) {
@ -63,5 +73,22 @@ async function test_query(opts, expectedRootCc, expectedFolderCc, expectedQueryC
}
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");
}