# HG changeset patch # User Mark Banner # Date 1513281386 21600 # Node ID 2254a714e53d7f09efe5b4dc8443f71b14f7a8cb # Parent e7aeb25af76febf91e5ca2eb2b4aafc6943d1daa Bug 1425339 - Allow Places tree's selectItems() function to handle guids as well as ids. r=mak MozReview-Commit-ID: LmZ7AYXBsJ4 diff --git a/browser/components/places/content/controller.js b/browser/components/places/content/controller.js --- a/browser/components/places/content/controller.js +++ b/browser/components/places/content/controller.js @@ -725,19 +725,25 @@ PlacesController.prototype = { let performed = PlacesUIUtils.showBookmarkDialog({ action: "add", type: aType, defaultInsertionPoint: ip, hiddenRows: [ "folderPicker" ] }, window.top); if (performed) { // Select the new item. - let insertedNodeId = PlacesUtils.bookmarks - .getIdForItemAt(ip.itemId, await ip.getIndex()); - this._view.selectItems([insertedNodeId], false); + // TODO (Bug 1425555): When we remove places transactions, we might be + // able to improve showBookmarkDialog to return the guid direct, and + // avoid the fetch. + let insertedNode = await PlacesUtils.bookmarks.fetch({ + parentGuid: ip.guid, + index: await ip.getIndex() + }); + + this._view.selectItems([insertedNode.guid], false); } }, /** * Create a new Bookmark separator somewhere. */ async newSeparator() { var ip = this._view.insertionPoint; @@ -752,19 +758,18 @@ PlacesController.prototype = { let insertedNodeId = PlacesUtils.bookmarks .getIdForItemAt(ip.itemId, index); this._view.selectItems([insertedNodeId], false); return; } let txn = PlacesTransactions.NewSeparator({ parentGuid: ip.guid, index }); let guid = await txn.transact(); - let itemId = await PlacesUtils.promiseItemId(guid); // Select the new item. - this._view.selectItems([itemId], false); + this._view.selectItems([guid], false); }, /** * Opens a dialog for moving the selected nodes. */ async moveSelectedBookmarks() { let args = { // The guid of the folder to move bookmarks to. This will only be @@ -1297,20 +1302,17 @@ PlacesController.prototype = { } catch (ex) { // No supported data exists or nodes unwrap failed, just bail out. return; } let itemsToSelect = []; if (PlacesUIUtils.useAsyncTransactions) { let doCopy = action == "copy"; - let guidsToSelect = await handleTransferItems(items, ip, doCopy, this._view); - - let guidsToIdMap = await PlacesUtils.promiseManyItemIds(guidsToSelect); - itemsToSelect = Array.from(guidsToIdMap.values()); + itemsToSelect = await handleTransferItems(items, ip, doCopy, this._view); } else { let transactions = []; let insertionIndex = await ip.getIndex(); for (let index = insertionIndex, i = 0; i < items.length; ++i) { if (ip.isTag) { // Pasting into a tag container means tagging the item, regardless of // the requested action. let tagTxn = new PlacesTagURITransaction(NetUtil.newURI(items[i].uri), diff --git a/browser/components/places/content/tree.xml b/browser/components/places/content/tree.xml --- a/browser/components/places/content/tree.xml +++ b/browser/components/places/content/tree.xml @@ -545,18 +545,20 @@ { - tree.selectItems([bookmarkIds.get(bookmarks[0].guid)]); + tree.selectItems([bookmarks[0].guid]); // Delete the bookmark to put something in the undo history. // Rather than calling cmd_delete, we call the remove directly, so that we // can await on it finishing, and be guaranteed that there's something // in the history. await tree.controller.remove("Remove Selection"); - tree.selectItems([bookmarkIds.get(bookmarks[1].guid)]); + tree.selectItems([bookmarks[1].guid]); // Now open the bookmarks dialog and cancel it. await withBookmarksDialog( true, function openDialog() { tree.controller.doCommand("placesCmd_show:info"); }, async function test(dialogWin) { @@ -81,17 +76,17 @@ add_task(async function test_cancel_with add_task(async function test_cancel_with_changes() { if (!PlacesUIUtils.useAsyncTransactions) { Assert.ok(true, "Skipping test as async transactions are turned off"); return; } await withSidebarTree("bookmarks", async (tree) => { - tree.selectItems([bookmarkIds.get(bookmarks[1].guid)]); + tree.selectItems([bookmarks[1].guid]); // Now open the bookmarks dialog and cancel it. await withBookmarksDialog( true, function openDialog() { tree.controller.doCommand("placesCmd_show:info"); }, async function test(dialogWin) { diff --git a/browser/components/places/tests/browser/browser_bookmark_folder_moveability.js b/browser/components/places/tests/browser/browser_bookmark_folder_moveability.js --- a/browser/components/places/tests/browser/browser_bookmark_folder_moveability.js +++ b/browser/components/places/tests/browser/browser_bookmark_folder_moveability.js @@ -19,72 +19,68 @@ add_task(async function() { await withSidebarTree("bookmarks", async function(tree) { info("Test a regular folder"); let folder = await PlacesUtils.bookmarks.insert({ parentGuid: root.guid, title: "", type: PlacesUtils.bookmarks.TYPE_FOLDER, }); let folderId = await PlacesUtils.promiseItemId(folder.guid); - tree.selectItems([folderId]); + tree.selectItems([folder.guid]); Assert.equal(tree.selectedNode.bookmarkGuid, folder.guid, "Selected the expected node"); Assert.equal(tree.selectedNode.type, 6, "node is a folder"); Assert.ok(PlacesControllerDragHelper.canMoveNode(tree.selectedNode, tree), "can move regular folder node"); info("Test a folder shortcut"); let shortcut = await PlacesUtils.bookmarks.insert({ parentGuid: root.guid, title: "bar", url: `place:folder=${folderId}` }); - let shortcutId = await PlacesUtils.promiseItemId(shortcut.guid); - tree.selectItems([shortcutId]); + tree.selectItems([shortcut.guid]); Assert.equal(tree.selectedNode.bookmarkGuid, shortcut.guid, "Selected the expected node"); Assert.equal(tree.selectedNode.type, 9, "node is a folder shortcut"); Assert.equal(PlacesUtils.getConcreteItemGuid(tree.selectedNode), folder.guid, "shortcut node guid and concrete guid match"); Assert.ok(PlacesControllerDragHelper.canMoveNode(tree.selectedNode, tree), "can move folder shortcut node"); info("Test a query"); let bookmark = await PlacesUtils.bookmarks.insert({ parentGuid: root.guid, title: "", url: "http://foo.com", }); - let bookmarkId = await PlacesUtils.promiseItemId(bookmark.guid); - tree.selectItems([bookmarkId]); + tree.selectItems([bookmark.guid]); Assert.equal(tree.selectedNode.bookmarkGuid, bookmark.guid, "Selected the expected node"); let query = await PlacesUtils.bookmarks.insert({ parentGuid: root.guid, title: "bar", url: `place:terms=foo` }); - let queryId = await PlacesUtils.promiseItemId(query.guid); - tree.selectItems([queryId]); + tree.selectItems([query.guid]); Assert.equal(tree.selectedNode.bookmarkGuid, query.guid, "Selected the expected node"); Assert.ok(PlacesControllerDragHelper.canMoveNode(tree.selectedNode, tree), "can move query node"); info("Test a tag container"); PlacesUtils.tagging.tagURI(Services.io.newURI(bookmark.url.href), ["bar"]); // Add the tags root query. let tagsQuery = await PlacesUtils.bookmarks.insert({ parentGuid: root.guid, title: "", url: "place:type=" + Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY, }); - let tagsQueryId = await PlacesUtils.promiseItemId(tagsQuery.guid); - tree.selectItems([tagsQueryId]); + tree.selectItems([tagsQuery.guid]); PlacesUtils.asQuery(tree.selectedNode).containerOpen = true; Assert.equal(tree.selectedNode.childCount, 1, "has tags"); let tagNode = tree.selectedNode.getChild(0); Assert.ok(!PlacesControllerDragHelper.canMoveNode(tagNode, tree), "should not be able to move tag container node"); tree.selectedNode.containerOpen = false; info("Test that special folders and cannot be moved but other shortcuts can."); @@ -98,18 +94,17 @@ add_task(async function() { selectShortcutForRootId(tree, id); Assert.ok(!PlacesControllerDragHelper.canMoveNode(tree.selectedNode, tree), "shouldn't be able to move default shortcuts to roots"); let s = await PlacesUtils.bookmarks.insert({ parentGuid: root.guid, title: "bar", url: `place:folder=${id}`, }); - let sid = await PlacesUtils.promiseItemId(s.guid); - tree.selectItems([sid]); + tree.selectItems([s.guid]); Assert.equal(tree.selectedNode.bookmarkGuid, s.guid, "Selected the expected node"); Assert.ok(PlacesControllerDragHelper.canMoveNode(tree.selectedNode, tree), "should be able to move user-created shortcuts to roots"); } }); }); diff --git a/browser/components/places/tests/browser/browser_bookmarksProperties.js b/browser/components/places/tests/browser/browser_bookmarksProperties.js --- a/browser/components/places/tests/browser/browser_bookmarksProperties.js +++ b/browser/components/places/tests/browser/browser_bookmarksProperties.js @@ -53,39 +53,35 @@ var gTests = []; gTests.push({ desc: "Bug 462662 - Pressing Enter to select tag from autocomplete closes bookmarks properties dialog", sidebar: SIDEBAR_BOOKMARKS_ID, action: ACTION_EDIT, itemType: null, window: null, _bookmark: null, - _itemId: null, _cleanShutdown: false, async setup() { // Add a bookmark in unsorted bookmarks folder. this._bookmark = await add_bookmark(TEST_URL); Assert.ok(this._bookmark, "Correctly added a bookmark"); - this._itemId = await PlacesUtils.promiseItemId(this._bookmark.guid); - Assert.ok(this._itemId > 0, "Got an item id for the bookmark"); - // Add a tag to this bookmark. PlacesUtils.tagging.tagURI(PlacesUtils._uri(TEST_URL), ["testTag"]); var tags = PlacesUtils.tagging.getTagsForURI(PlacesUtils._uri(TEST_URL)); Assert.equal(tags[0], "testTag", "Correctly added a tag"); }, selectNode(tree) { - tree.selectItems([PlacesUtils.unfiledBookmarksFolderId]); + tree.selectItems([PlacesUtils.bookmarks.unfiledGuid]); PlacesUtils.asContainer(tree.selectedNode).containerOpen = true; - tree.selectItems([this._itemId]); - Assert.equal(tree.selectedNode.itemId, this._itemId, "Bookmark has been selected"); + tree.selectItems([this._bookmark.guid]); + Assert.equal(tree.selectedNode.bookmarkGuid, this._bookmark.guid, "Bookmark has been selected"); }, async run() { // open tags autocomplete and press enter var tagsField = this.window.document.getElementById("editBMPanel_tagsField"); var self = this; let unloadPromise = new Promise(resolve => { @@ -164,39 +160,35 @@ gTests.push({ gTests.push({ desc: "Bug 476020 - Pressing Esc while having the tag autocomplete open closes the bookmarks panel", sidebar: SIDEBAR_BOOKMARKS_ID, action: ACTION_EDIT, itemType: null, window: null, _bookmark: null, - _itemId: null, _cleanShutdown: false, async setup() { // Add a bookmark in unsorted bookmarks folder. this._bookmark = await add_bookmark(TEST_URL); Assert.ok(this._bookmark, "Correctly added a bookmark"); - this._itemId = await PlacesUtils.promiseItemId(this._bookmark.guid); - Assert.ok(this._itemId > 0, "Got an item id for the bookmark"); - // Add a tag to this bookmark. PlacesUtils.tagging.tagURI(PlacesUtils._uri(TEST_URL), ["testTag"]); var tags = PlacesUtils.tagging.getTagsForURI(PlacesUtils._uri(TEST_URL)); Assert.equal(tags[0], "testTag", "Correctly added a tag"); }, selectNode(tree) { - tree.selectItems([PlacesUtils.unfiledBookmarksFolderId]); + tree.selectItems([PlacesUtils.bookmarks.unfiledGuid]); PlacesUtils.asContainer(tree.selectedNode).containerOpen = true; - tree.selectItems([this._itemId]); - Assert.equal(tree.selectedNode.itemId, this._itemId, "Bookmark has been selected"); + tree.selectItems([this._bookmark.guid]); + Assert.equal(tree.selectedNode.bookmarkGuid, this._bookmark.guid, "Bookmark has been selected"); }, async run() { // open tags autocomplete and press enter var tagsField = this.window.document.getElementById("editBMPanel_tagsField"); var self = this; let hiddenPromise = new Promise(resolve => { diff --git a/browser/components/places/tests/browser/browser_controller_onDrop.js b/browser/components/places/tests/browser/browser_controller_onDrop.js --- a/browser/components/places/tests/browser/browser_controller_onDrop.js +++ b/browser/components/places/tests/browser/browser_controller_onDrop.js @@ -70,17 +70,17 @@ async function run_drag_test(startBookma // Reset the stubs so that previous test runs don't count against us. PlacesUIUtils.getTransactionForData.reset(); PlacesTransactions.batch.reset(); let dragBookmark = bookmarks[startBookmarkIndex]; await withSidebarTree("bookmarks", async (tree) => { - tree.selectItems([PlacesUtils.unfiledBookmarksFolderId]); + tree.selectItems([PlacesUtils.bookmarks.unfiledGuid]); PlacesUtils.asContainer(tree.selectedNode).containerOpen = true; // Simulating a drag-drop with a tree view turns out to be really difficult // as you can't get a node for the source/target. Hence, we fake the // insertion point and drag data and call the function direct. let ip = new InsertionPoint({ parentId: await PlacesUtils.promiseItemId(PlacesUtils.bookmarks.unfiledGuid), parentGuid: newParentGuid, diff --git a/browser/components/places/tests/browser/browser_controller_onDrop_tagFolder.js b/browser/components/places/tests/browser/browser_controller_onDrop_tagFolder.js --- a/browser/components/places/tests/browser/browser_controller_onDrop_tagFolder.js +++ b/browser/components/places/tests/browser/browser_controller_onDrop_tagFolder.js @@ -50,17 +50,17 @@ async function run_drag_test(startBookma // Reset the stubs so that previous test runs don't count against us. PlacesTransactions.Tag.reset(); PlacesTransactions.batch.reset(); let dragBookmark = bookmarks[startBookmarkIndex]; await withSidebarTree("bookmarks", async (tree) => { - tree.selectItems([PlacesUtils.unfiledBookmarksFolderId]); + tree.selectItems([PlacesUtils.bookmarks.unfiledGuid]); PlacesUtils.asContainer(tree.selectedNode).containerOpen = true; // Simulating a drag-drop with a tree view turns out to be really difficult // as you can't get a node for the source/target. Hence, we fake the // insertion point and drag data and call the function direct. let ip = new InsertionPoint({ isTag: true, tagName: TAG_NAME, diff --git a/browser/components/places/tests/browser/browser_cutting_bookmarks.js b/browser/components/places/tests/browser/browser_cutting_bookmarks.js --- a/browser/components/places/tests/browser/browser_cutting_bookmarks.js +++ b/browser/components/places/tests/browser/browser_cutting_bookmarks.js @@ -54,24 +54,22 @@ add_task(async function() { }); var selectBookmarksIn = async function(organizer, bookmarks, aLeftPaneQuery) { let PlacesOrganizer = organizer.PlacesOrganizer; let ContentTree = organizer.ContentTree; info("Selecting " + aLeftPaneQuery + " in the left pane"); PlacesOrganizer.selectLeftPaneQuery(aLeftPaneQuery); - let ids = []; for (let {guid} of bookmarks) { let bookmark = await PlacesUtils.bookmarks.fetch(guid); is(bookmark.parentGuid, PlacesOrganizer._places.selectedNode.targetFolderGuid, "Bookmark has the right parent"); - ids.push(await PlacesUtils.promiseItemId(bookmark.guid)); } info("Selecting the bookmarks in the right pane"); - ContentTree.view.selectItems(ids); + ContentTree.view.selectItems(bookmarks.map(bm => bm.guid)); for (let node of ContentTree.view.selectedNodes) { is(node.bookmarkIndex, node.title, "Found the expected bookmark in the expected position"); } }; diff --git a/browser/components/places/tests/browser/browser_paste_bookmarks.js b/browser/components/places/tests/browser/browser_paste_bookmarks.js --- a/browser/components/places/tests/browser/browser_paste_bookmarks.js +++ b/browser/components/places/tests/browser/browser_paste_bookmarks.js @@ -26,19 +26,18 @@ add_task(async function paste() { info("Selecting BookmarksToolbar in the left pane"); PlacesOrganizer.selectLeftPaneQuery("BookmarksToolbar"); let bookmark = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.toolbarGuid, url: TEST_URL, title: "0" }); - let bookmarkId = await PlacesUtils.promiseItemId(bookmark.guid); - ContentTree.view.selectItems([bookmarkId]); + ContentTree.view.selectItems([bookmark.guid]); await promiseClipboard(() => { info("Cutting selection"); ContentTree.view.controller.cut(); }, PlacesUtils.TYPE_X_MOZ_PLACE); info("Selecting UnfiledBookmarks in the left pane"); PlacesOrganizer.selectLeftPaneQuery("UnfiledBookmarks"); @@ -81,41 +80,32 @@ add_task(async function paste_check_inde children: copyChildren }); let targetBookmarks = await PlacesUtils.bookmarks.insertTree({ guid: PlacesUtils.bookmarks.unfiledGuid, children: targetChildren }); - let bookmarkIds = await PlacesUtils.promiseManyItemIds([ + ContentTree.view.selectItems([ copyBookmarks[0].guid, copyBookmarks[3].guid, copyBookmarks[6].guid, - copyBookmarks[9].guid - ]); - - ContentTree.view.selectItems([ - bookmarkIds.get(copyBookmarks[0].guid), - bookmarkIds.get(copyBookmarks[3].guid), - bookmarkIds.get(copyBookmarks[6].guid), - bookmarkIds.get(copyBookmarks[9].guid), + copyBookmarks[9].guid, ]); await promiseClipboard(() => { info("Cutting multiple selection"); ContentTree.view.controller.cut(); }, PlacesUtils.TYPE_X_MOZ_PLACE); info("Selecting UnfiledBookmarks in the left pane"); PlacesOrganizer.selectLeftPaneQuery("UnfiledBookmarks"); - let insertionBookmarkId = await PlacesUtils.promiseItemId(targetBookmarks[4].guid); - - ContentTree.view.selectItems([insertionBookmarkId]); + ContentTree.view.selectItems([targetBookmarks[4].guid]); info("Pasting clipboard"); await ContentTree.view.controller.paste(); let tree = await PlacesUtils.promiseBookmarksTree(PlacesUtils.bookmarks.unfiledGuid); const expectedBookmarkOrder = [ targetBookmarks[0].guid, @@ -157,38 +147,29 @@ add_task(async function paste_check_inde }); } let copyBookmarks = await PlacesUtils.bookmarks.insertTree({ guid: PlacesUtils.bookmarks.toolbarGuid, children: copyChildren }); - let bookmarkIds = await PlacesUtils.promiseManyItemIds([ + ContentTree.view.selectItems([ copyBookmarks[0].guid, copyBookmarks[3].guid, copyBookmarks[6].guid, - copyBookmarks[9].guid - ]); - - ContentTree.view.selectItems([ - bookmarkIds.get(copyBookmarks[0].guid), - bookmarkIds.get(copyBookmarks[3].guid), - bookmarkIds.get(copyBookmarks[6].guid), - bookmarkIds.get(copyBookmarks[9].guid), + copyBookmarks[9].guid, ]); await promiseClipboard(() => { info("Cutting multiple selection"); ContentTree.view.controller.cut(); }, PlacesUtils.TYPE_X_MOZ_PLACE); - let insertionBookmarkId = await PlacesUtils.promiseItemId(copyBookmarks[4].guid); - - ContentTree.view.selectItems([insertionBookmarkId]); + ContentTree.view.selectItems([copyBookmarks[4].guid]); info("Pasting clipboard"); await ContentTree.view.controller.paste(); let tree = await PlacesUtils.promiseBookmarksTree(PlacesUtils.bookmarks.toolbarGuid); // Although we've inserted at index 4, we've taken out two items below it, so // we effectively insert after the third item. diff --git a/browser/components/places/tests/browser/browser_sidebarpanels_click.js b/browser/components/places/tests/browser/browser_sidebarpanels_click.js --- a/browser/components/places/tests/browser/browser_sidebarpanels_click.js +++ b/browser/components/places/tests/browser/browser_sidebarpanels_click.js @@ -37,18 +37,17 @@ add_task(async function test_sidebarpane title: "test", url: TEST_URL, }); }, prepare() { }, async selectNode(tree) { - let bookmarkId = await PlacesUtils.promiseItemId(this._bookmark.guid); - tree.selectItems([bookmarkId]); + tree.selectItems([this._bookmark.guid]); }, cleanup(aCallback) { return PlacesUtils.bookmarks.remove(this._bookmark); }, sidebarName: BOOKMARKS_SIDEBAR_ID, treeName: BOOKMARKS_SIDEBAR_TREE_ID, desc: "Bookmarks sidebar test" }); diff --git a/browser/components/places/tests/browser/browser_views_iconsupdate.js b/browser/components/places/tests/browser/browser_views_iconsupdate.js --- a/browser/components/places/tests/browser/browser_views_iconsupdate.js +++ b/browser/components/places/tests/browser/browser_views_iconsupdate.js @@ -100,20 +100,19 @@ function getNodeForToolbarItem(guid) { /** * Get a rect for a bookmark in the bookmarks sidebar * * @param guid * GUID of the item to search. * @returns DOM Node of the element. */ async function getRectForSidebarItem(guid) { - let itemId = await PlacesUtils.promiseItemId(guid); let sidebar = document.getElementById("sidebar"); let tree = sidebar.contentDocument.getElementById("bookmarks-view"); - tree.selectItems([itemId]); + tree.selectItems([guid]); let rect = {}; [rect.left, rect.top, rect.width, rect.height] = tree.treeBoxObject .selectionRegion .getRects(); // Adjust the position for the sidebar. rect.left += sidebar.getBoundingClientRect().left; rect.top += sidebar.getBoundingClientRect().top; return rect; diff --git a/browser/components/places/tests/chrome/test_bug1163447_selectItems_through_shortcut.xul b/browser/components/places/tests/chrome/test_bug1163447_selectItems_through_shortcut.xul --- a/browser/components/places/tests/chrome/test_bug1163447_selectItems_through_shortcut.xul +++ b/browser/components/places/tests/chrome/test_bug1163447_selectItems_through_shortcut.xul @@ -67,21 +67,20 @@ type: bmu.TYPE_FOLDER, title: "folder within unfiled" }); // Setup the places tree contents. let tree = document.getElementById("tree"); tree.place = "place:folder=TOOLBAR"; - // Select the folder via the selectItems(itemId) API being tested - let itemId = await PlacesUtils.promiseItemId(folder.guid); - tree.selectItems([itemId]); + // Select the folder via the selectItems(folder.guid) API being tested + tree.selectItems([folder.guid]); - is(tree.selectedNode && tree.selectedNode.itemId, itemId, "The node was selected through the shortcut"); + is(tree.selectedNode && tree.selectedNode.bookmarkGuid, folder.guid, "The node was selected through the shortcut"); // Cleanup await bmu.eraseEverything(); })().catch(err => { ok(false, `Uncaught error: ${err}`); }).then(SimpleTest.finish); } diff --git a/browser/components/places/tests/chrome/test_selectItems_on_nested_tree.xul b/browser/components/places/tests/chrome/test_selectItems_on_nested_tree.xul --- a/browser/components/places/tests/chrome/test_selectItems_on_nested_tree.xul +++ b/browser/components/places/tests/chrome/test_selectItems_on_nested_tree.xul @@ -72,15 +72,14 @@ title: "bookmark" }); // Setup the places tree contents. let tree = document.getElementById("tree"); tree.place = "place:folder=UNFILED_BOOKMARKS"; // Select the last bookmark. - let itemId = await PlacesUtils.promiseItemId(bm.guid); - tree.selectItems([itemId]); - is (tree.selectedNode.itemId, itemId, "The right node was selected"); + tree.selectItems([bm.guid]); + is (tree.selectedNode.bookmarkGuid, bm.guid, "The right node was selected"); })().then(SimpleTest.finish); } ]]>