Bug 1432437 - Remove synchronous changeBookmarkURI. r=standard8
MozReview-Commit-ID: Jy76eKCN7wp
--- a/services/sync/tests/unit/test_bookmark_tracker.js
+++ b/services/sync/tests/unit/test_bookmark_tracker.js
@@ -594,36 +594,37 @@ add_task(async function test_onItemChang
add_task(async function test_onItemChanged_changeBookmarkURI() {
_("Changes to bookmark URIs should be tracked");
try {
await stopTracking();
_("Insert a bookmark");
- let fx_id = PlacesUtils.bookmarks.insertBookmark(
- PlacesUtils.bookmarks.bookmarksMenuFolder,
- CommonUtils.makeURI("http://getfirefox.com"),
- PlacesUtils.bookmarks.DEFAULT_INDEX,
- "Get Firefox!");
- let fx_guid = await engine._store.GUIDForId(fx_id);
- _(`Firefox GUID: ${fx_guid}`);
+ let bm = await PlacesUtils.bookmarks.insert({
+ parentGuid: PlacesUtils.bookmarks.menuGuid,
+ url: "http://getfirefox.com",
+ title: "Get Firefox!"
+ });
+ _(`Firefox GUID: ${bm.guid}`);
_("Set a tracked annotation to make sure we only notify once");
+ let id = await PlacesUtils.promiseItemId(bm.guid);
PlacesUtils.annotations.setItemAnnotation(
- fx_id, PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO, "A test description", 0,
+ id, PlacesSyncUtils.bookmarks.DESCRIPTION_ANNO, "A test description", 0,
PlacesUtils.annotations.EXPIRE_NEVER);
await startTracking();
_("Change the bookmark's URI");
- PlacesUtils.bookmarks.changeBookmarkURI(fx_id,
- CommonUtils.makeURI("https://www.mozilla.org/firefox"));
- await verifyTrackedItems([fx_guid]);
- Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE);
+ bm.url = "https://www.mozilla.org/firefox";
+ bm = await PlacesUtils.bookmarks.update(bm);
+
+ await verifyTrackedItems([bm.guid]);
+ Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE * 2);
} finally {
_("Clean up.");
await cleanup();
}
});
add_task(async function test_onItemTagged() {
_("Items tagged using the synchronous API should be tracked");
--- a/toolkit/components/places/nsINavBookmarksService.idl
+++ b/toolkit/components/places/nsINavBookmarksService.idl
@@ -601,25 +601,16 @@ interface nsINavBookmarksService : nsISu
*
* @throws If aNewIndex is out of bounds.
*/
void setItemIndex(in long long aItemId,
in long aNewIndex,
[optional] in unsigned short aSource);
/**
- * Change the bookmarked URI for a bookmark.
- * This changes which "place" the bookmark points at,
- * which means all annotations, etc are carried along.
- */
- void changeBookmarkURI(in long long aItemId,
- in nsIURI aNewURI,
- [optional] in unsigned short aSource);
-
- /**
* Get the parent folder's id for an item.
*/
long long getFolderIdForItem(in long long aItemId);
/**
* Associates the given keyword with the given bookmark.
*
* Use an empty keyword to clear the keyword associated with the URI.
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -2344,106 +2344,16 @@ nsNavBookmarks::FetchFolderInfo(int64_t
NS_ENSURE_SUCCESS(rv, rv);
}
return NS_OK;
}
NS_IMETHODIMP
-nsNavBookmarks::ChangeBookmarkURI(int64_t aBookmarkId, nsIURI* aNewURI,
- uint16_t aSource)
-{
- NS_ENSURE_ARG_MIN(aBookmarkId, 1);
- NS_ENSURE_ARG(aNewURI);
-
- BookmarkData bookmark;
- nsresult rv = FetchItemInfo(aBookmarkId, bookmark);
- NS_ENSURE_SUCCESS(rv, rv);
- NS_ENSURE_ARG(bookmark.type == TYPE_BOOKMARK);
-
- mozStorageTransaction transaction(mDB->MainConn(), false);
-
- int64_t tagsRootId = TagsRootId();
- bool isTagging = bookmark.grandParentId == tagsRootId;
- int64_t syncChangeDelta = DetermineSyncChangeDelta(aSource);
-
- nsNavHistory* history = nsNavHistory::GetHistoryService();
- NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
- int64_t newPlaceId;
- nsAutoCString newPlaceGuid;
- rv = history->GetOrCreateIdForPage(aNewURI, &newPlaceId, newPlaceGuid);
- NS_ENSURE_SUCCESS(rv, rv);
- if (!newPlaceId)
- return NS_ERROR_INVALID_ARG;
-
- nsCOMPtr<mozIStorageStatement> statement = mDB->GetStatement(
- "UPDATE moz_bookmarks SET "
- "fk = :page_id, lastModified = :date, "
- "syncChangeCounter = syncChangeCounter + :delta "
- "WHERE id = :item_id "
- );
- NS_ENSURE_STATE(statement);
- mozStorageStatementScoper scoper(statement);
-
- rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), newPlaceId);
- NS_ENSURE_SUCCESS(rv, rv);
- bookmark.lastModified = RoundedPRNow();
- rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("date"),
- bookmark.lastModified);
- NS_ENSURE_SUCCESS(rv, rv);
- rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("item_id"), bookmark.id);
- NS_ENSURE_SUCCESS(rv, rv);
- rv = statement->BindInt64ByName(NS_LITERAL_CSTRING("delta"), syncChangeDelta);
- NS_ENSURE_SUCCESS(rv, rv);
- rv = statement->Execute();
- NS_ENSURE_SUCCESS(rv, rv);
-
- if (isTagging) {
- // For consistency with the tagging service behavior, changing a tag entry's
- // URL bumps the change counter for bookmarks with the old and new URIs.
- rv = AddSyncChangesForBookmarksWithURL(bookmark.url, syncChangeDelta);
- NS_ENSURE_SUCCESS(rv, rv);
- rv = AddSyncChangesForBookmarksWithURI(aNewURI, syncChangeDelta);
- NS_ENSURE_SUCCESS(rv, rv);
- }
-
- rv = transaction.Commit();
- NS_ENSURE_SUCCESS(rv, rv);
-
- rv = history->UpdateFrecency(newPlaceId);
- NS_ENSURE_SUCCESS(rv, rv);
-
- // Upon changing the URI for a bookmark, update the frecency for the old
- // place as well.
- rv = history->UpdateFrecency(bookmark.placeId);
- NS_ENSURE_SUCCESS(rv, rv);
-
- nsAutoCString spec;
- rv = aNewURI->GetSpec(spec);
- NS_ENSURE_SUCCESS(rv, rv);
-
- NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
- nsINavBookmarkObserver,
- OnItemChanged(bookmark.id,
- NS_LITERAL_CSTRING("uri"),
- false,
- spec,
- bookmark.lastModified,
- bookmark.type,
- bookmark.parentId,
- bookmark.guid,
- bookmark.parentGuid,
- bookmark.url,
- aSource));
- return NS_OK;
-}
-
-
-NS_IMETHODIMP
nsNavBookmarks::GetFolderIdForItem(int64_t aItemId, int64_t* _parentId)
{
NS_ENSURE_ARG_MIN(aItemId, 1);
NS_ENSURE_ARG_POINTER(_parentId);
BookmarkData bookmark;
nsresult rv = FetchItemInfo(aItemId, bookmark);
NS_ENSURE_SUCCESS(rv, rv);
--- a/toolkit/components/places/tests/legacy/test_bookmarks.js
+++ b/toolkit/components/places/tests/legacy/test_bookmarks.js
@@ -420,46 +420,30 @@ add_task(async function test_bookmarks()
// Workaround possible VM timers issues moving lastModified and dateAdded
// to the past.
lastModified -= 1000;
bs.setItemLastModified(newId10, lastModified);
dateAdded -= 1000;
bs.setItemDateAdded(newId10, dateAdded);
- bs.changeBookmarkURI(newId10, uri("http://foo11.com/"));
-
- // check that lastModified is set after we change the bookmark uri
- lastModified2 = bs.getItemLastModified(newId10);
- info("test changeBookmarkURI");
- info("dateAdded = " + dateAdded);
- info("lastModified = " + lastModified);
- info("lastModified2 = " + lastModified2);
- Assert.ok(is_time_ordered(lastModified, lastModified2));
- Assert.ok(is_time_ordered(dateAdded, lastModified2));
-
- Assert.equal(bookmarksObserver._itemChangedId, newId10);
- Assert.equal(bookmarksObserver._itemChangedProperty, "uri");
- Assert.equal(bookmarksObserver._itemChangedValue, "http://foo11.com/");
- Assert.equal(bookmarksObserver._itemChangedOldValue, "http://foo10.com/");
-
// test getBookmarkURI
- let newId11 = bs.insertBookmark(testRoot, uri("http://foo11.com/"),
+ let newId11 = bs.insertBookmark(testRoot, uri("http://foo10.com/"),
bs.DEFAULT_INDEX, "");
let bmURI = bs.getBookmarkURI(newId11);
- Assert.equal("http://foo11.com/", bmURI.spec);
+ Assert.equal("http://foo10.com/", bmURI.spec);
// test getBookmarkURI with non-bookmark items
try {
bs.getBookmarkURI(testRoot);
do_throw("getBookmarkURI() should throw for non-bookmark items!");
} catch (ex) {}
// test getItemIndex
- let newId12 = bs.insertBookmark(testRoot, uri("http://foo11.com/"), 1, "");
+ let newId12 = bs.insertBookmark(testRoot, uri("http://foo10.com/"), 1, "");
let bmIndex = bs.getItemIndex(newId12);
Assert.equal(1, bmIndex);
// insert a bookmark with title ZZZXXXYYY and then search for it.
// this test confirms that we can find bookmarks that we haven't visited
// (which are "hidden") and that we can find by title.
// see bug #369887 for more details
let newId13 = bs.insertBookmark(testRoot, uri("http://foobarcheese.com/"),
deleted file mode 100644
--- a/toolkit/components/places/tests/legacy/test_changeBookmarkURI.js
+++ /dev/null
@@ -1,62 +0,0 @@
-/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
-/* vim:set ts=2 sw=2 sts=2 et: */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-// Get bookmark service
-var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
- getService(Ci.nsINavBookmarksService);
-
-/**
- * Ensures that the Places APIs recognize that aBookmarkedUri is bookmarked
- * via aBookmarkId and that aUnbookmarkedUri is not bookmarked at all.
- *
- * @param aBookmarkId
- * an item ID whose corresponding URI is aBookmarkedUri
- * @param aBookmarkedUri
- * a bookmarked URI that has a corresponding item ID aBookmarkId
- * @param aUnbookmarkedUri
- * a URI that is not currently bookmarked at all
- */
-async function checkUris(aBookmarkId, aBookmarkedUri, aUnbookmarkedUri) {
- // Ensure that aBookmarkedUri equals some URI that is bookmarked
- let bm = await PlacesUtils.bookmarks.fetch({url: aBookmarkedUri});
-
- Assert.notEqual(bm, null);
- Assert.ok(uri(bm.url).equals(aBookmarkedUri));
-
- // Ensure that the URI corresponding to aBookmarkId equals aBookmarkedUri
- Assert.ok(bmsvc.getBookmarkURI(aBookmarkId).equals(aBookmarkedUri));
-
- // Ensure that aUnbookmarkedUri does not equal any URI that is bookmarked
- bm = await PlacesUtils.bookmarks.fetch({url: aUnbookmarkedUri});
- Assert.equal(bm, null);
-}
-
-// main
-add_task(async function() {
- // Create a folder
- var folderId = bmsvc.createFolder(bmsvc.toolbarFolder,
- "test",
- bmsvc.DEFAULT_INDEX);
-
- // Create 2 URIs
- var uri1 = uri("http://www.dogs.com/");
- var uri2 = uri("http://www.cats.com/");
-
- // Bookmark the first one
- var bookmarkId = bmsvc.insertBookmark(folderId,
- uri1,
- bmsvc.DEFAULT_INDEX,
- "Dogs");
-
- // uri1 is bookmarked via bookmarkId, uri2 is not
- await checkUris(bookmarkId, uri1, uri2);
-
- // Change the URI of the bookmark to uri2
- bmsvc.changeBookmarkURI(bookmarkId, uri2);
-
- // uri2 is now bookmarked via bookmarkId, uri1 is not
- await checkUris(bookmarkId, uri2, uri1);
-});
--- a/toolkit/components/places/tests/legacy/xpcshell.ini
+++ b/toolkit/components/places/tests/legacy/xpcshell.ini
@@ -3,10 +3,9 @@
[DEFAULT]
head = head_legacy.js
firefox-appdir = browser
[test_418643_removeFolderChildren.js]
[test_bookmarks.js]
[test_bookmarks_setNullTitle.js]
-[test_changeBookmarkURI.js]
[test_protectRoots.js]
--- a/toolkit/components/places/tests/unit/test_sync_utils.js
+++ b/toolkit/components/places/tests/unit/test_sync_utils.js
@@ -698,38 +698,37 @@ add_task(async function test_pullChanges
"Tagging service should update cache after updating tag folder");
let changes = await PlacesSyncUtils.bookmarks.pullChanges();
deepEqual(Object.keys(changes).sort(),
[firstItem.recordId, secondItem.recordId].sort(),
"Should include tagged bookmarks after updating tag folder");
await setChangesSynced(changes);
}
- info("Change tag entry URI using Bookmarks.changeBookmarkURI");
+ info("Change tag entry URL using Bookmarks.update");
{
- let tagId = PlacesUtils.bookmarks.getIdForItemAt(tagFolderId, 0);
- PlacesUtils.bookmarks.changeBookmarkURI(tagId, uri("https://bugzilla.org"));
+ let tagGuid = await PlacesUtils.promiseItemGuid(
+ PlacesUtils.bookmarks.getIdForItemAt(tagFolderId, 0));
+ await PlacesUtils.bookmarks.update({
+ guid: tagGuid,
+ url: "https://bugzilla.org",
+ });
let changes = await PlacesSyncUtils.bookmarks.pullChanges();
deepEqual(Object.keys(changes).sort(),
[firstItem.recordId, secondItem.recordId, untaggedItem.recordId].sort(),
"Should include tagged bookmarks after changing tag entry URI");
assertTagForURLs("tricky", ["https://bugzilla.org/", "https://mozilla.org/"],
"Should remove tag entry for old URI");
await setChangesSynced(changes);
- }
-
- info("Change tag entry URL using Bookmarks.update");
- {
- let tagGuid = await PlacesUtils.promiseItemGuid(
- PlacesUtils.bookmarks.getIdForItemAt(tagFolderId, 0));
+
await PlacesUtils.bookmarks.update({
guid: tagGuid,
url: "https://example.com",
});
- let changes = await PlacesSyncUtils.bookmarks.pullChanges();
+ changes = await PlacesSyncUtils.bookmarks.pullChanges();
deepEqual(Object.keys(changes).sort(),
[untaggedItem.recordId].sort(),
"Should include tagged bookmarks after changing tag entry URL");
assertTagForURLs("tricky", ["https://example.com/", "https://mozilla.org/"],
"Should remove tag entry for old URL");
await setChangesSynced(changes);
}