diff --git a/services/sync/tests/unit/test_bookmark_tracker.js b/services/sync/tests/unit/test_bookmark_tracker.js index da06d0fa7a6d..b98c82d908c9 100644 --- a/services/sync/tests/unit/test_bookmark_tracker.js +++ b/services/sync/tests/unit/test_bookmark_tracker.js @@ -455,18 +455,6 @@ add_task(async function test_onItemAdded() { let syncBmkGUID = await PlacesUtils.promiseItemGuid(syncBmkID); await verifyTrackedItems([syncFolderGUID, syncBmkGUID]); Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE); - - await resetTracker(); - await startTracking(); - - _("Insert a separator using the sync API"); - let index = (await PlacesUtils.bookmarks.fetch(syncFolderGUID)).index; - let syncSepID = PlacesUtils.bookmarks.insertSeparator( - PlacesUtils.bookmarks.bookmarksMenuFolder, - index); - let syncSepGUID = await PlacesUtils.promiseItemGuid(syncSepID); - await verifyTrackedItems(["menu", syncSepGUID]); - Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE); } finally { _("Clean up."); await cleanup(); diff --git a/toolkit/components/places/nsINavBookmarksService.idl b/toolkit/components/places/nsINavBookmarksService.idl index 27bff76906a5..bbda69b3eb73 100644 --- a/toolkit/components/places/nsINavBookmarksService.idl +++ b/toolkit/components/places/nsINavBookmarksService.idl @@ -456,27 +456,6 @@ interface nsINavBookmarksService : nsISupports in long aIndex, [optional] in unsigned short aSource); - /** - * Inserts a bookmark separator into the given folder at the given index. - * The separator can be removed using removeChildAt(). - * @param aParentId - * The id of the parent folder - * @param aIndex - * The separator's index under folder, or DEFAULT_INDEX to append - * @param [optional] aGuid - * The GUID to be set for the new item. If not set, a new GUID is - * generated. Unless you've a very sound reason, such as an undo - * manager implementation, do not pass this argument. - * @param [optional] aSource - * The change source, forwarded to all bookmark observers. Defaults - * to SOURCE_DEFAULT. - * @return The ID of the new separator. - * @throws if aGuid is malformed. - */ - long long insertSeparator(in long long aParentId, in long aIndex, - [optional] in ACString aGuid, - [optional] in unsigned short aSource); - /** * Set the title for an item. * @param aItemId diff --git a/toolkit/components/places/nsNavBookmarks.cpp b/toolkit/components/places/nsNavBookmarks.cpp index 738bf29df28c..3ba59a7c4000 100644 --- a/toolkit/components/places/nsNavBookmarks.cpp +++ b/toolkit/components/places/nsNavBookmarks.cpp @@ -889,60 +889,6 @@ nsNavBookmarks::CreateContainerWithID(int64_t aItemId, } -NS_IMETHODIMP -nsNavBookmarks::InsertSeparator(int64_t aParent, - int32_t aIndex, - const nsACString& aGUID, - uint16_t aSource, - int64_t* aNewItemId) -{ - NS_ENSURE_ARG_MIN(aParent, 1); - NS_ENSURE_ARG_MIN(aIndex, nsINavBookmarksService::DEFAULT_INDEX); - NS_ENSURE_ARG_POINTER(aNewItemId); - - if (!aGUID.IsEmpty() && !IsValidGUID(aGUID)) - return NS_ERROR_INVALID_ARG; - - // Get the correct index for insertion. This also ensures the parent exists. - int32_t index, folderCount; - int64_t grandParentId; - nsAutoCString folderGuid; - nsresult rv = FetchFolderInfo(aParent, &folderCount, folderGuid, &grandParentId); - NS_ENSURE_SUCCESS(rv, rv); - - mozStorageTransaction transaction(mDB->MainConn(), false); - - if (aIndex == nsINavBookmarksService::DEFAULT_INDEX || - aIndex >= folderCount) { - index = folderCount; - } - else { - index = aIndex; - // Create space for the insertion. - rv = AdjustIndices(aParent, index, INT32_MAX, 1); - NS_ENSURE_SUCCESS(rv, rv); - } - - *aNewItemId = -1; - nsAutoCString guid(aGUID); - PRTime dateAdded = RoundedPRNow(); - rv = InsertBookmarkInDB(-1, SEPARATOR, aParent, index, EmptyCString(), dateAdded, - 0, folderGuid, grandParentId, nullptr, aSource, - aNewItemId, guid); - NS_ENSURE_SUCCESS(rv, rv); - - rv = transaction.Commit(); - NS_ENSURE_SUCCESS(rv, rv); - - NOTIFY_BOOKMARKS_OBSERVERS(mCanNotify, mObservers, DontSkip, - OnItemAdded(*aNewItemId, aParent, index, TYPE_SEPARATOR, - nullptr, EmptyCString(), dateAdded, guid, - folderGuid, aSource)); - - return NS_OK; -} - - NS_IMPL_ISUPPORTS(nsNavBookmarks::RemoveFolderTransaction, nsITransaction) NS_IMETHODIMP diff --git a/toolkit/components/places/tests/bookmarks/test_458683.js b/toolkit/components/places/tests/bookmarks/test_458683.js index fbb8e31ebed4..639cea277581 100644 --- a/toolkit/components/places/tests/bookmarks/test_458683.js +++ b/toolkit/components/places/tests/bookmarks/test_458683.js @@ -15,14 +15,7 @@ const ITEM_URL = "http://test.mozilla.org/"; const TAG_NAME = "testTag"; function validateResults() { - var query = PlacesUtils.history.getNewQuery(); - query.setFolders([PlacesUtils.bookmarks.toolbarFolder], 1); - var options = PlacesUtils.history.getNewQueryOptions(); - var result = PlacesUtils.history.executeQuery(query, options); - - var toolbar = result.root; - toolbar.containerOpen = true; - + let toolbar = PlacesUtils.getFolderContents(PlacesUtils.toolbarFolderId).root; // test for our bookmark Assert.equal(toolbar.childCount, 1); for (var i = 0; i < toolbar.childCount; i++) { @@ -51,34 +44,45 @@ add_task(async function() { // create a tag PlacesUtils.tagging.tagURI(Services.io.newURI(ITEM_URL), [TAG_NAME]); // get tag folder id - var options = PlacesUtils.history.getNewQueryOptions(); - var query = PlacesUtils.history.getNewQuery(); - query.setFolders([PlacesUtils.bookmarks.tagsFolder], 1); - var result = PlacesUtils.history.executeQuery(query, options); - var tagRoot = result.root; - tagRoot.containerOpen = true; + let tagRoot = PlacesUtils.getFolderContents(PlacesUtils.tagsFolderId).root; Assert.equal(tagRoot.childCount, 1); - var tagNode = tagRoot.getChild(0) - .QueryInterface(Ci.nsINavHistoryContainerResultNode); - let tagItemId = tagNode.itemId; + let tagItemGuid = PlacesUtils.asContainer(tagRoot.getChild(0)).bookmarkGuid; tagRoot.containerOpen = false; - // Currently these use the old API as the new API doesn't support inserting - // invalid items into the tag folder. + function insert({type, parentGuid}) { + return PlacesUtils.withConnectionWrapper("test_458683: insert", async db => { + await db.executeCached( + `INSERT INTO moz_bookmarks (type, parent, position, guid) + VALUES (:type, + (SELECT id FROM moz_bookmarks WHERE guid = :parentGuid), + (SELECT MAX(position) + 1 FROM moz_bookmarks WHERE parent = (SELECT id FROM moz_bookmarks WHERE guid = :parentGuid)), + GENERATE_GUID())`, {type, parentGuid}); + }); + } // add a separator and a folder inside tag folder - PlacesUtils.bookmarks.insertSeparator(tagItemId, - PlacesUtils.bookmarks.DEFAULT_INDEX); - PlacesUtils.bookmarks.createFolder(tagItemId, - "test folder", - PlacesUtils.bookmarks.DEFAULT_INDEX); + // We must insert these manually, because the new bookmarking API doesn't + // support inserting invalid items into the tag folder. + await insert({ + parentGuid: tagItemGuid, + type: PlacesUtils.bookmarks.TYPE_SEPARATOR + }); + await insert({ + parentGuid: tagItemGuid, + type: PlacesUtils.bookmarks.TYPE_FOLDER + }); // add a separator and a folder inside tag root - PlacesUtils.bookmarks.insertSeparator(PlacesUtils.bookmarks.tagsFolder, - PlacesUtils.bookmarks.DEFAULT_INDEX); - PlacesUtils.bookmarks.createFolder(PlacesUtils.bookmarks.tagsFolder, - "test tags root folder", - PlacesUtils.bookmarks.DEFAULT_INDEX); + await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.tagsGuid, + type: PlacesUtils.bookmarks.TYPE_SEPARATOR + }); + await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.tagsGuid, + title: "test tags root folder", + type: PlacesUtils.bookmarks.TYPE_FOLDER + }); + // sanity validateResults(); diff --git a/toolkit/components/places/tests/bookmarks/test_bookmarks_getRecent.js b/toolkit/components/places/tests/bookmarks/test_bookmarks_getRecent.js index 2c559b1e21e5..e53976377909 100644 --- a/toolkit/components/places/tests/bookmarks/test_bookmarks_getRecent.js +++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_getRecent.js @@ -35,8 +35,10 @@ add_task(async function getRecent_returns_recent_bookmarks() { PlacesUtils.tagging.tagURI(uri(bm4.url), ["Test Tag"]); // Add a separator. - PlacesUtils.bookmarks.insertSeparator(PlacesUtils.unfiledBookmarksFolderId, - PlacesUtils.bookmarks.DEFAULT_INDEX); + await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.unfiledGuid, + type: PlacesUtils.bookmarks.TYPE_SEPARATOR + }); // Add a query bookmark. let queryURL = `place:folder=${PlacesUtils.bookmarksMenuFolderId}&queryType=1`; diff --git a/toolkit/components/places/tests/bookmarks/test_sync_fields.js b/toolkit/components/places/tests/bookmarks/test_sync_fields.js index 2b37dfb6e6ca..c6e626277e51 100644 --- a/toolkit/components/places/tests/bookmarks/test_sync_fields.js +++ b/toolkit/components/places/tests/bookmarks/test_sync_fields.js @@ -75,12 +75,14 @@ class TestCases { await PlacesTestUtils.markBookmarksAsSynced(); } - info("Test 3: separators"); - try { - await this.testSeparators(); - } finally { - info("Reset sync fields after test 3"); - await PlacesTestUtils.markBookmarksAsSynced(); + if ("insertSeparator" in this) { + info("Test 3: separators"); + try { + await this.testSeparators(); + } finally { + info("Reset sync fields after test 3"); + await PlacesTestUtils.markBookmarksAsSynced(); + } } } @@ -265,12 +267,6 @@ class SyncTestCases extends TestCases { return PlacesUtils.promiseItemGuid(id); } - async insertSeparator(parentGuid, index) { - let parentId = await PlacesUtils.promiseItemId(parentGuid); - let id = PlacesUtils.bookmarks.insertSeparator(parentId, index); - return PlacesUtils.promiseItemGuid(id); - } - async moveItem(guid, newParentGuid, index) { let id = await PlacesUtils.promiseItemId(guid); let newParentId = await PlacesUtils.promiseItemId(newParentGuid); diff --git a/toolkit/components/places/tests/legacy/test_bookmarks.js b/toolkit/components/places/tests/legacy/test_bookmarks.js index 4223e5e6f284..71849dc311c8 100644 --- a/toolkit/components/places/tests/legacy/test_bookmarks.js +++ b/toolkit/components/places/tests/legacy/test_bookmarks.js @@ -273,13 +273,12 @@ add_task(async function test_bookmarks() { let tmpFolder = bs.createFolder(testRoot, "tmp", 2); // test removeFolderChildren - // 1) add/remove each child type (bookmark, separator, folder) + // 1) add/remove each child type (bookmark, folder) tmpFolder = bs.createFolder(testRoot, "removeFolderChildren", bs.DEFAULT_INDEX); bs.insertBookmark(tmpFolder, uri("http://foo9.com/"), bs.DEFAULT_INDEX, ""); bs.createFolder(tmpFolder, "subfolder", bs.DEFAULT_INDEX); - bs.insertSeparator(tmpFolder, bs.DEFAULT_INDEX); - // 2) confirm that folder has 3 children + // 2) confirm that folder has 2 children let options = hs.getNewQueryOptions(); let query = hs.getNewQuery(); query.setFolders([tmpFolder], 1); @@ -287,7 +286,7 @@ add_task(async function test_bookmarks() { let result = hs.executeQuery(query, options); let rootNode = result.root; rootNode.containerOpen = true; - Assert.equal(rootNode.childCount, 3); + Assert.equal(rootNode.childCount, 2); rootNode.containerOpen = false; } catch (ex) { do_throw("test removeFolderChildren() - querying for children failed: " + ex); @@ -532,9 +531,6 @@ function testSimpleFolderResult() { let beforeInsert = Date.now() * 1000 - 1; Assert.ok(beforeInsert > 0); - // insert a separator - let sep = bs.insertSeparator(parent, bs.DEFAULT_INDEX); - // re-set item title separately so can test nodes' last modified let item = bs.insertBookmark(parent, uri("about:blank"), bs.DEFAULT_INDEX, ""); @@ -554,24 +550,19 @@ function testSimpleFolderResult() { let result = hs.executeQuery(query, options); let rootNode = result.root; rootNode.containerOpen = true; - Assert.equal(rootNode.childCount, 4); + Assert.equal(rootNode.childCount, 3); let node = rootNode.getChild(0); - Assert.ok(node.dateAdded > 0); - Assert.equal(node.lastModified, node.dateAdded); - Assert.equal(node.itemId, sep); - Assert.equal(node.title, ""); - node = rootNode.getChild(1); Assert.equal(node.itemId, item); Assert.ok(node.dateAdded > 0); Assert.ok(node.lastModified > 0); Assert.equal(node.title, "test bookmark"); - node = rootNode.getChild(2); + node = rootNode.getChild(1); Assert.equal(node.itemId, folder); Assert.equal(node.title, "test folder"); Assert.ok(node.dateAdded > 0); Assert.ok(node.lastModified > 0); - node = rootNode.getChild(3); + node = rootNode.getChild(2); Assert.equal(node.itemId, folderLongName); Assert.equal(node.title, longName.substring(0, TITLE_LENGTH_MAX)); Assert.ok(node.dateAdded > 0); @@ -583,7 +574,7 @@ function testSimpleFolderResult() { Assert.equal(bookmarksObserver._itemChangedProperty, "title"); Assert.equal(bookmarksObserver._itemChangedValue, longName.substring(0, TITLE_LENGTH_MAX)); - node = rootNode.getChild(3); + node = rootNode.getChild(2); Assert.equal(node.title, longName.substring(0, TITLE_LENGTH_MAX)); rootNode.containerOpen = false;