Bug 1386513 - Handle cases of pasted bookmarks not being in the database, to fix copying bookmarks across browser instances. r?mak
MozReview-Commit-ID: Lv2DT0WQGhZ
--- a/browser/components/places/PlacesUIUtils.jsm
+++ b/browser/components/places/PlacesUIUtils.jsm
@@ -577,17 +577,18 @@ this.PlacesUIUtils = {
*
* @return a Places Transaction that can be transacted for performing the
* move/insert command.
*/
getTransactionForData(aData, aType, aNewParentGuid, aIndex, aCopy) {
if (!this.SUPPORTED_FLAVORS.includes(aData.type))
throw new Error(`Unsupported '${aData.type}' data type`);
- if ("itemGuid" in aData) {
+ if ("itemGuid" in aData && "instanceId" in aData &&
+ aData.instanceId == PlacesUtils.instanceId) {
if (!this.PLACES_FLAVORS.includes(aData.type))
throw new Error(`itemGuid unexpectedly set on ${aData.type} data`);
let info = { guid: aData.itemGuid,
newParentGuid: aNewParentGuid,
newIndex: aIndex };
if (aCopy) {
info.excludingAnnotation = "Places/SmartBookmark";
--- a/browser/components/places/tests/browser/browser.ini
+++ b/browser/components/places/tests/browser/browser.ini
@@ -48,16 +48,18 @@ subsuite = clipboard
[browser_library_middleclick.js]
[browser_library_move_bookmarks.js]
[browser_library_open_leak.js]
[browser_library_openFlatContainer.js]
[browser_library_panel_leak.js]
[browser_library_search.js]
[browser_library_views_liveupdate.js]
[browser_markPageAsFollowedLink.js]
+[browser_paste_bookmarks.js]
+subsuite = clipboard
[browser_paste_into_tags.js]
subsuite = clipboard
[browser_sidebarpanels_click.js]
skip-if = true # temporarily disabled for breaking the treeview - bug 658744
[browser_sort_in_library.js]
[browser_stayopenmenu.js]
[browser_toolbar_drop_text.js]
[browser_toolbar_overflow.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/places/tests/browser/browser_paste_bookmarks.js
@@ -0,0 +1,103 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const TEST_URL = "http://example.com/";
+const TEST_URL1 = "https://example.com/otherbrowser";
+
+var PlacesOrganizer;
+var ContentTree;
+var bookmark;
+var bookmarkId;
+
+add_task(async function setup() {
+ await PlacesUtils.bookmarks.eraseEverything();
+ let organizer = await promiseLibrary();
+
+ registerCleanupFunction(async function() {
+ await promiseLibraryClosed(organizer);
+ await PlacesUtils.bookmarks.eraseEverything();
+ });
+
+ PlacesOrganizer = organizer.PlacesOrganizer;
+ ContentTree = organizer.ContentTree;
+
+ info("Selecting BookmarksToolbar in the left pane");
+ PlacesOrganizer.selectLeftPaneQuery("BookmarksToolbar");
+
+ bookmark = await PlacesUtils.bookmarks.insert({
+ parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+ url: TEST_URL,
+ title: "0"
+ });
+ bookmarkId = await PlacesUtils.promiseItemId(bookmark.guid);
+
+ ContentTree.view.selectItems([bookmarkId]);
+
+ await promiseClipboard(() => {
+ info("Copying selection");
+ ContentTree.view.controller.cut();
+ }, PlacesUtils.TYPE_X_MOZ_PLACE);
+});
+
+add_task(async function paste() {
+ info("Selecting UnfiledBookmarks in the left pane");
+ PlacesOrganizer.selectLeftPaneQuery("UnfiledBookmarks");
+
+ info("Pasting clipboard");
+ await ContentTree.view.controller.paste();
+
+ let tree = await PlacesUtils.promiseBookmarksTree(PlacesUtils.bookmarks.unfiledGuid);
+
+ Assert.equal(tree.children.length, 1,
+ "Should be one bookmark in the unfiled folder.");
+ Assert.equal(tree.children[0].title, "0",
+ "Should have the correct title");
+ Assert.equal(tree.children[0].uri, TEST_URL,
+ "Should have the correct URL");
+
+ await PlacesUtils.bookmarks.remove(tree.children[0].guid);
+});
+
+add_task(async function paste_from_different_instance() {
+ let xferable = Cc["@mozilla.org/widget/transferable;1"]
+ .createInstance(Ci.nsITransferable);
+ xferable.init(null);
+
+ // Fake data on the clipboard to pretend this is from a different instance
+ // of Firefox.
+ let data = {
+ "title": "test",
+ "id": 32,
+ "instanceId": "FAKEFAKEFAKE",
+ "itemGuid": "ZBf_TYkrYGvW",
+ "parent": 452,
+ "dateAdded": 1464866275853000,
+ "lastModified": 1507638113352000,
+ "type": "text/x-moz-place",
+ "uri": TEST_URL1
+ };
+ data = JSON.stringify(data);
+
+ xferable.addDataFlavor(PlacesUtils.TYPE_X_MOZ_PLACE);
+ xferable.setTransferData(PlacesUtils.TYPE_X_MOZ_PLACE, PlacesUtils.toISupportsString(data),
+ data.length * 2);
+
+ Services.clipboard.setData(xferable, null, Ci.nsIClipboard.kGlobalClipboard);
+
+ info("Pasting clipboard");
+
+ await ContentTree.view.controller.paste();
+
+ let tree = await PlacesUtils.promiseBookmarksTree(PlacesUtils.bookmarks.unfiledGuid);
+
+ Assert.equal(tree.children.length, 1,
+ "Should be one bookmark in the unfiled folder.");
+ Assert.equal(tree.children[0].title, "test",
+ "Should have the correct title");
+ Assert.equal(tree.children[0].uri, TEST_URL1,
+ "Should have the correct URL");
+
+ await PlacesUtils.bookmarks.remove(tree.children[0].guid);
+});
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -130,16 +130,19 @@ async function notifyKeywordChange(url,
* Whether the node represents a livemark.
*/
function serializeNode(aNode, aIsLivemark) {
let data = {};
data.title = aNode.title;
data.id = aNode.itemId;
data.livemark = aIsLivemark;
+ // Add an instanceId so we can tell which instance of an FF session the data
+ // is coming from.
+ data.instanceId = PlacesUtils.instanceId;
let guid = aNode.bookmarkGuid;
if (guid) {
data.itemGuid = guid;
if (aNode.parent)
data.parent = aNode.parent.itemId;
let grandParent = aNode.parent && aNode.parent.parent;
if (grandParent)
@@ -2010,16 +2013,21 @@ XPCOMUtils.defineLazyGetter(PlacesUtils,
XPCOMUtils.defineLazyGetter(this, "bundle", function() {
const PLACES_STRING_BUNDLE_URI = "chrome://places/locale/places.properties";
return Cc["@mozilla.org/intl/stringbundle;1"].
getService(Ci.nsIStringBundleService).
createBundle(PLACES_STRING_BUNDLE_URI);
});
+// This is just used as a reasonably-random value for copy & paste / drag operations.
+XPCOMUtils.defineLazyGetter(PlacesUtils, "instanceId", () => {
+ return PlacesUtils.history.makeGuid();
+});
+
/**
* Setup internal databases for closing properly during shutdown.
*
* 1. Places initiates shutdown.
* 2. Before places can move to the step where it closes the low-level connection,
* we need to make sure that we have closed `conn`.
* 3. Before we can close `conn`, we need to make sure that all external clients
* have stopped using `conn`.