Bug 1209518 - Fix highlighting of cut bookmarks. r=mak
authorMark Banner <standard8@mozilla.com>
Mon, 21 May 2018 16:55:26 +0100
changeset 419246 cc2b79a70a8c9c9b85b466a0c20fe5951bff1b5f
parent 419245 f51587925bf1d822ee5fe44c6a1266a3c64b2303
child 419247 ac1c5c363e29e16c99c87a978d1db82b45a787ed
child 419270 1a42a5c3a00ed2a74bd3873c1a97c95d0f7cfca5
push id34031
push usernbeleuzu@mozilla.com
push dateTue, 22 May 2018 09:47:31 +0000
treeherdermozilla-central@ac1c5c363e29 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1209518
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1209518 - Fix highlighting of cut bookmarks. r=mak Fix the ownership check to correctly check if we're cutting or copying. MozReview-Commit-ID: 1eLt4Fyy2nE
browser/components/places/content/controller.js
browser/components/places/tests/browser/browser.ini
browser/components/places/tests/browser/browser_paste_resets_cut_highlights.js
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -1091,17 +1091,17 @@ PlacesController.prototype = {
     // Track the exected action in the xferable.  This must be the last flavor
     // since it's the least preferred one.
     // Enqueue a unique instance identifier to distinguish operations across
     // concurrent instances of the application.
     addData(PlacesUtils.TYPE_X_MOZ_PLACE_ACTION, aAction + "," + this.profileName);
 
     if (hasData) {
       this.clipboard.setData(xferable,
-                             this.cutNodes.length > 0 ? this : null,
+                             aAction == "cut" ? this : null,
                              Ci.nsIClipboard.kGlobalClipboard);
     }
   },
 
   _cutNodes: [],
   get cutNodes() {
     return this._cutNodes;
   },
--- a/browser/components/places/tests/browser/browser.ini
+++ b/browser/components/places/tests/browser/browser.ini
@@ -65,16 +65,18 @@ subsuite = clipboard
 [browser_library_panel_leak.js]
 [browser_library_search.js]
 [browser_library_views_liveupdate.js]
 [browser_markPageAsFollowedLink.js]
 [browser_panelview_bookmarks_delete.js]
 [browser_paste_bookmarks.js]
 subsuite = clipboard
 [browser_paste_into_tags.js]
+[browser_paste_resets_cut_highlights.js]
+subsuite = clipboard
 [browser_remove_bookmarks.js]
 subsuite = clipboard
 [browser_sidebar_open_bookmarks.js]
 [browser_sidebarpanels_click.js]
 skip-if = (os == "mac" && debug) # Bug 1423667
 [browser_sort_in_library.js]
 [browser_stayopenmenu.js]
 skip-if = os == "mac" && debug # bug 1400323
new file mode 100644
--- /dev/null
+++ b/browser/components/places/tests/browser/browser_paste_resets_cut_highlights.js
@@ -0,0 +1,88 @@
+/* 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;
+
+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;
+});
+
+add_task(async function paste() {
+  info("Selecting BookmarksToolbar in the left pane");
+  PlacesOrganizer.selectLeftPaneBuiltIn("BookmarksToolbar");
+
+  let bookmarks = await PlacesUtils.bookmarks.insertTree({
+    guid: PlacesUtils.bookmarks.toolbarGuid,
+    children: [{
+      url: TEST_URL,
+      title: "0",
+    }, {
+      url: TEST_URL1,
+      title: "1",
+    }]
+  });
+
+  Assert.equal(ContentTree.view.view.rowCount, 2,
+    "Should have the right amount of items in the view");
+
+  ContentTree.view.selectItems([bookmarks[0].guid]);
+
+  await promiseClipboard(() => {
+    info("Cutting selection");
+    ContentTree.view.controller.cut();
+  }, PlacesUtils.TYPE_X_MOZ_PLACE);
+
+  assertItemsHighlighted(1);
+
+  ContentTree.view.selectItems([bookmarks[1].guid]);
+
+  info("Pasting clipboard");
+  await ContentTree.view.controller.paste();
+
+  assertItemsHighlighted(0);
+
+  // And now repeat the other way around to make sure.
+
+  await promiseClipboard(() => {
+    info("Cutting selection");
+    ContentTree.view.controller.cut();
+  }, PlacesUtils.TYPE_X_MOZ_PLACE);
+
+  assertItemsHighlighted(1);
+
+  ContentTree.view.selectItems([bookmarks[0].guid]);
+
+  info("Pasting clipboard");
+  await ContentTree.view.controller.paste();
+
+  assertItemsHighlighted(0);
+});
+
+function assertItemsHighlighted(expectedItems) {
+  let column = ContentTree.view.view._tree.columns[0];
+  // Check the properties of the cells to make sure nothing has a cut highlight.
+  let highlighedItems = 0;
+  for (let i = 0; i < ContentTree.view.view.rowCount; i++) {
+    if (ContentTree.view.view.getCellProperties(i, column).includes("cutting")) {
+      highlighedItems++;
+    }
+  }
+
+  Assert.equal(highlighedItems, expectedItems,
+    "Should have the correct amount of items highlighed");
+}