Bug 1434261 - Disable user actions on the places trees not in the library window to avoid accidential/unintentional actions. r?mak draft
authorMark Banner <standard8@mozilla.com>
Wed, 21 Feb 2018 17:51:23 +0000
changeset 759013 ed5cb9ff7327fe6ee1f3c2307a13c76eaa6608db
parent 758927 6661c077325c35af028f1cdaa660f673cbea39be
push id100255
push userbmo:standard8@mozilla.com
push dateFri, 23 Feb 2018 16:17:29 +0000
reviewersmak
bugs1434261
milestone60.0a1
Bug 1434261 - Disable user actions on the places trees not in the library window to avoid accidential/unintentional actions. r?mak MozReview-Commit-ID: ILo09hwWRsZ
browser/components/places/content/controller.js
browser/components/places/content/editBookmarkOverlay.xul
browser/components/places/content/tree.xml
browser/components/places/content/treeView.js
browser/components/places/tests/browser/browser.ini
browser/components/places/tests/browser/browser_bookmarkProperties_no_user_actions.js
browser/components/preferences/selectBookmark.xul
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -80,30 +80,38 @@ function PlacesController(aView) {
 }
 
 PlacesController.prototype = {
   /**
    * The places view.
    */
   _view: null,
 
+  // This is used in certain views to disable user actions on the places tree
+  // views. This avoids accidental deletion/modification when the user is not
+  // actually organising the trees.
+  disableUserActions: false,
+
   QueryInterface: XPCOMUtils.generateQI([
     Ci.nsIClipboardOwner
   ]),
 
   // nsIClipboardOwner
   LosingOwnership: function PC_LosingOwnership(aXferable) {
     this.cutNodes = [];
   },
 
   terminate: function PC_terminate() {
     this._releaseClipboardOwnership();
   },
 
   supportsCommand: function PC_supportsCommand(aCommand) {
+    if (this.disableUserActions) {
+      return false;
+    }
     // Non-Places specific commands that we also support
     switch (aCommand) {
     case "cmd_undo":
     case "cmd_redo":
     case "cmd_cut":
     case "cmd_copy":
     case "cmd_paste":
     case "cmd_delete":
--- a/browser/components/places/content/editBookmarkOverlay.xul
+++ b/browser/components/places/content/editBookmarkOverlay.xul
@@ -90,16 +90,17 @@
             <tree id="editBMPanel_folderTree"
                   flex="1"
                   class="placesTree"
                   type="places"
                   height="150"
                   minheight="150"
                   editable="true"
                   onselect="gEditItemOverlay.onFolderTreeSelect();"
+                  disableUserActions="true"
                   hidecolumnpicker="true">
               <treecols>
                 <treecol anonid="title" flex="1" primary="true" hideheader="true"/>
               </treecols>
               <treechildren flex="1"/>
             </tree>
 
             <hbox id="editBMPanel_newFolderBox">
--- a/browser/components/places/content/tree.xml
+++ b/browser/components/places/content/tree.xml
@@ -39,16 +39,21 @@
         }
         this.view = null;
       ]]></destructor>
 
       <property name="controller"
                 readonly="true"
                 onget="return this._controller"/>
 
+      <property name="disableUserActions"
+                onget="return this.getAttribute('disableUserActions') == 'true';"
+                onset="if (val) this.setAttribute('disableUserActions', 'true');
+                       else this.removeAttribute('disableUserActions'); return val;"/>
+
       <!-- overriding -->
       <property name="view">
         <getter><![CDATA[
           try {
             return this.treeBoxObject.view.wrappedJSObject || null;
           } catch (e) {
             return null;
           }
@@ -105,16 +110,17 @@
           if (this.flatList) {
             let onOpenFlatContainer = this.onOpenFlatContainer;
             if (onOpenFlatContainer)
               callback = new Function("aContainer", onOpenFlatContainer);
           }
 
           if (!this._controller) {
             this._controller = new PlacesController(this);
+            this._controller.disableUserActions = this.disableUserActions;
             this.controllers.appendController(this._controller);
           }
 
           let treeView = new PlacesTreeView(this.flatList, callback, this._controller);
 
           // Observer removal is done within the view itself.  When the tree
           // goes away, treeboxobject calls view.setTree(null), which then
           // calls removeObserver.
@@ -732,16 +738,22 @@
           win = win.parent;
         }
       ]]></handler>
 
       <handler event="dragstart"><![CDATA[
         if (event.target.localName != "treechildren")
           return;
 
+        if (this.disableUserActions) {
+          event.preventDefault();
+          event.stopPropagation();
+          return;
+        }
+
         let nodes = this.selectedNodes;
         for (let i = 0; i < nodes.length; i++) {
           let node = nodes[i];
 
           // Disallow dragging the root node of a tree.
           if (!node.parent) {
             event.preventDefault();
             event.stopPropagation();
--- a/browser/components/places/content/treeView.js
+++ b/browser/components/places/content/treeView.js
@@ -1384,16 +1384,20 @@ PlacesTreeView.prototype = {
     return this._result.sortingMode !=
            Ci.nsINavHistoryQueryOptions.SORT_BY_NONE;
   },
 
   canDrop: function PTV_canDrop(aRow, aOrientation, aDataTransfer) {
     if (!this._result)
       throw Cr.NS_ERROR_UNEXPECTED;
 
+    if (this._controller.disableUserActions) {
+      return false;
+    }
+
     // Drop position into a sorted treeview would be wrong.
     if (this.isSorted())
       return false;
 
     let ip = this._getInsertionPoint(aRow, aOrientation);
     return ip && PlacesControllerDragHelper.canDrop(ip, aDataTransfer);
   },
 
@@ -1469,16 +1473,20 @@ PlacesTreeView.prototype = {
     return new InsertionPoint({
       parentId: PlacesUtils.getConcreteItemId(container),
       parentGuid: PlacesUtils.getConcreteItemGuid(container),
       index, orientation, tagName, dropNearNode
     });
   },
 
   drop: function PTV_drop(aRow, aOrientation, aDataTransfer) {
+    if (this._controller.disableUserActions) {
+      return;
+    }
+
     // We are responsible for translating the |index| and |orientation|
     // parameters into a container id and index within the container,
     // since this information is specific to the tree view.
     let ip = this._getInsertionPoint(aRow, aOrientation);
     if (ip) {
       PlacesControllerDragHelper.onDrop(ip, aDataTransfer, this._tree.element)
                                 .catch(Components.utils.reportError)
                                 .then(() => {
--- a/browser/components/places/tests/browser/browser.ini
+++ b/browser/components/places/tests/browser/browser.ini
@@ -38,16 +38,18 @@ skip-if = (os == 'win' && ccov) # Bug 14
 skip-if = (os == 'win' && ccov) # Bug 1423667
 [browser_bookmarkProperties_addLivemark.js]
 skip-if = (os == 'win' && ccov) # Bug 1423667
 [browser_bookmarkProperties_bookmarkAllTabs.js]
 skip-if = (os == 'win' && ccov) # Bug 1423667
 [browser_bookmarkProperties_cancel.js]
 skip-if = (os == 'win' && ccov) # Bug 1423667
 [browser_bookmarkProperties_editTagContainer.js]
+[browser_bookmarkProperties_no_user_actions.js]
+skip-if = (os == 'win' && ccov) # Bug 1423667
 [browser_bookmarkProperties_readOnlyRoot.js]
 skip-if = (os == 'win' && ccov) # Bug 1423667
 [browser_bookmarksProperties.js]
 skip-if = (os == 'win' && ccov) # Bug 1423667
 [browser_check_correct_controllers.js]
 skip-if = (os == 'win' && ccov) # Bug 1423667
 [browser_click_bookmarks_on_toolbar.js]
 [browser_controller_onDrop_sidebar.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/places/tests/browser/browser_bookmarkProperties_no_user_actions.js
@@ -0,0 +1,82 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+"use strict";
+
+const TEST_URL = "about:buildconfig";
+
+add_task(async function test_change_title_from_BookmarkStar() {
+  await PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid,
+    url: TEST_URL,
+    title: "Before Edit"
+  });
+
+  let tab = await BrowserTestUtils.openNewForegroundTab({
+    gBrowser,
+    opening: TEST_URL,
+    waitForStateStop: true
+  });
+
+  registerCleanupFunction(async () => {
+    await BrowserTestUtils.removeTab(tab);
+    await PlacesUtils.bookmarks.eraseEverything();
+  });
+
+  let bookmarkPanel = document.getElementById("editBookmarkPanel");
+  let shownPromise = promisePopupShown(bookmarkPanel);
+
+  let bookmarkStar = BookmarkingUI.star;
+  bookmarkStar.click();
+
+  await shownPromise;
+
+  window.gEditItemOverlay.toggleFolderTreeVisibility();
+
+  let folderTree = document.getElementById("editBMPanel_folderTree");
+
+  // canDrop should always return false.
+  let bookmarkWithId = JSON.stringify(Object.assign({
+    url: "http://example.com",
+    title: "Fake BM",
+  }));
+
+  let dt = {
+    dropEffect: "move",
+    mozCursor: "auto",
+    mozItemCount: 1,
+    types: [ PlacesUtils.TYPE_X_MOZ_PLACE ],
+    mozTypesAt(i) {
+      return this.types;
+    },
+    mozGetDataAt(i) {
+      return bookmarkWithId;
+    }
+  };
+
+  Assert.ok(!folderTree.view.canDrop(1, Ci.nsITreeView.DROP_BEFORE, dt),
+    "Should not be able to drop a bookmark");
+
+  // User Actions should be disabled.
+  const userActions = [
+    "cmd_undo",
+    "cmd_redo",
+    "cmd_cut",
+    "cmd_copy",
+    "cmd_paste",
+    "cmd_delete",
+    "cmd_selectAll",
+    // Anything starting with placesCmd_ should also be disabled.
+    "placesCmd_"
+  ];
+  for (let action of userActions) {
+    Assert.ok(!folderTree.view._controller.supportsCommand(action),
+      `${action} should be disabled for the folder tree in bookmarks properties`);
+  }
+
+  let hiddenPromise = promisePopupHidden(bookmarkPanel);
+  let doneButton = document.getElementById("editBookmarkPanelDoneButton");
+  doneButton.click();
+  await hiddenPromise;
+});
--- a/browser/components/preferences/selectBookmark.xul
+++ b/browser/components/preferences/selectBookmark.xul
@@ -11,33 +11,34 @@
 
 <?xul-overlay href="chrome://browser/content/places/placesOverlay.xul"?>
 
 <!DOCTYPE dialog SYSTEM "chrome://browser/locale/preferences/selectBookmark.dtd">
 
 <dialog id="selectBookmarkDialog"
         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
         title="&selectBookmark.title;" style="width: 32em;"
-        persist="screenX screenY width height" screenX="24" screenY="24"      
-        onload="SelectBookmarkDialog.init();" 
+        persist="screenX screenY width height" screenX="24" screenY="24"
+        onload="SelectBookmarkDialog.init();"
         ondialogaccept="SelectBookmarkDialog.accept();">
 
   <script type="application/javascript"
           src="chrome://browser/content/preferences/selectBookmark.js"/>
-  
+
   <description>&selectBookmark.label;</description>
 
   <separator class="thin"/>
 
-  <tree id="bookmarks" flex="1" type="places" 
-        style="height: 15em;" 
+  <tree id="bookmarks" flex="1" type="places"
+        style="height: 15em;"
         hidecolumnpicker="true"
         seltype="single"
         ondblclick="SelectBookmarkDialog.onItemDblClick();"
-        onselect="SelectBookmarkDialog.selectionChanged();">
+        onselect="SelectBookmarkDialog.selectionChanged();"
+        disableUserActions="true">
     <treecols>
       <treecol id="title" flex="1" primary="true" hideheader="true"/>
     </treecols>
     <treechildren id="bookmarksChildren" flex="1"/>
   </tree>
 
   <separator class="thin"/>