Bug 1193621 - Can't change tag name in library. r=ttaubert a=ritu
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 02 Sep 2015 16:41:01 +0200
changeset 289152 9de325bc66b9104c07e61890917021621a2abb60
parent 289151 452f6270615dd054a237f2ab40164b6d682fb5b3
child 289153 02b5db7eeed56a701c65862344569c3c52fd9666
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersttaubert, ritu
bugs1193621
milestone42.0a2
Bug 1193621 - Can't change tag name in library. r=ttaubert a=ritu
browser/components/places/content/editBookmarkOverlay.js
browser/components/places/tests/browser/browser.ini
browser/components/places/tests/browser/browser_bookmarkProperties_editTagContainer.js
--- a/browser/components/places/content/editBookmarkOverlay.js
+++ b/browser/components/places/content/editBookmarkOverlay.js
@@ -29,16 +29,22 @@ let gEditItemOverlay = {
     // folders), when the pane shows for them it's opened in read-only mode, showing the
     // properties of the target folder.
     let itemId = node ? node.itemId : -1;
     let itemGuid = PlacesUIUtils.useAsyncTransactions && node ?
                      PlacesUtils.getConcreteItemGuid(node) : null;
     let isItem = itemId != -1;
     let isFolderShortcut = isItem &&
       node.type == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT;
+    let isTag = node && PlacesUtils.nodeIsTagQuery(node);
+    if (isTag) {
+      itemId = PlacesUtils.getConcreteItemId(node);
+      // For now we don't have access to the item guid synchronously for tags,
+      // so we'll need to fetch it later.
+    }
     let isURI = node && PlacesUtils.nodeIsURI(node);
     let uri = isURI ? NetUtil.newURI(node.uri) : null;
     let title = node ? node.title : null;
     let isBookmark = isItem && isURI;
     let bulkTagging = !node;
     let uris = bulkTagging ? aInitInfo.uris : null;
     let visibleRows = new Set();
     let isParentReadOnly = false;
@@ -50,17 +56,17 @@ let gEditItemOverlay = {
                             PlacesUIUtils.isContentsReadOnly(parent);
       }
     }
 
     return this._paneInfo = { itemId, itemGuid, isItem,
                               isURI, uri, title,
                               isBookmark, isFolderShortcut, isParentReadOnly,
                               bulkTagging, uris,
-                              visibleRows, postData };
+                              visibleRows, postData, isTag };
   },
 
   get initialized() {
     return this._paneInfo != null;
   },
 
   // Backwards-compatibility getters
   get itemId() {
@@ -84,22 +90,23 @@ let gEditItemOverlay = {
   // Check if the pane is initialized to show only read-only fields.
   get readOnly() {
     // TODO (Bug 1120314): Folder shortcuts are currently read-only due to some
     // quirky implementation details (the most important being the "smart"
     // semantics of node.title that makes hard to edit the right entry).
     // This pane is read-only if:
     //  * the panel is not initialized
     //  * the node is a folder shortcut
-    //  * the node is not bookmarked
-    //  * the node is child of a read-only container and is not a bookmarked URI
+    //  * the node is not bookmarked and not a tag container
+    //  * the node is child of a read-only container and is not a bookmarked
+    //    URI nor a tag container
     return !this.initialized ||
            this._paneInfo.isFolderShortcut ||
-           !this._paneInfo.isItem ||
-           (this._paneInfo.isParentReadOnly && !this._paneInfo.isBookmark);
+           (!this._paneInfo.isItem && !this._paneInfo.isTag) ||
+           (this._paneInfo.isParentReadOnly && !this._paneInfo.isBookmark && !this._paneInfo.isTag);
   },
 
   // the first field which was edited after this panel was initialized for
   // a certain item
   _firstEditedField: "",
 
   _initNamePicker() {
     if (this._paneInfo.bulkTagging)
@@ -513,17 +520,17 @@ let gEditItemOverlay = {
 
     // set the pref
     var prefs = Cc["@mozilla.org/preferences-service;1"].
                 getService(Ci.nsIPrefBranch);
     prefs.setCharPref("browser.bookmarks.editDialog.firstEditField", aNewField);
   },
 
   onNamePickerChange() {
-    if (this.readOnly || !this._paneInfo.isItem)
+    if (this.readOnly || !(this._paneInfo.isItem || this._paneInfo.isTag))
       return;
 
     // Here we update either the item title or its cached static title
     let newTitle = this._namePicker.value;
     if (!newTitle &&
         PlacesUtils.bookmarks.getFolderIdForItem(this._paneInfo.itemId) == PlacesUtils.tagsFolderId) {
       // We don't allow setting an empty title for a tag, restore the old one.
       this._initNamePicker();
@@ -531,19 +538,23 @@ let gEditItemOverlay = {
     else {
       this._mayUpdateFirstEditField("namePicker");
       if (!PlacesUIUtils.useAsyncTransactions) {
         let txn = new PlacesEditItemTitleTransaction(this._paneInfo.itemId,
                                                      newTitle);
         PlacesUtils.transactionManager.doTransaction(txn);
         return;
       }
-      let guid = this._paneInfo.itemGuid;
-      PlacesTransactions.EditTitle({ guid, title: newTitle })
-                        .transact().catch(Components.utils.reportError);
+      Task.spawn(function* () {
+        let guid = this._paneInfo.isTag
+                    ? (yield PlacesUtils.promiseItemGuid(this._paneInfo.itemId))
+                    : this._paneInfo.itemGuid;
+        PlacesTransactions.EditTitle({ guid, title: newTitle })
+                          .transact().catch(Components.utils.reportError);
+      }).catch(Cu.reportError);
     }
   },
 
   onDescriptionFieldChange() {
     if (this.readOnly || !this._paneInfo.isItem)
       return;
 
     let itemId = this._paneInfo.itemId;
--- a/browser/components/places/tests/browser/browser.ini
+++ b/browser/components/places/tests/browser/browser.ini
@@ -21,16 +21,17 @@ support-files =
 [browser_475045.js]
 [browser_555547.js]
 [browser_bookmarklet_windowOpen.js]
 support-files =
   pageopeningwindow.html
 [browser_bookmarkProperties_addFolderDefaultButton.js]
 [browser_bookmarkProperties_addKeywordForThisSearch.js]
 [browser_bookmarkProperties_addLivemark.js]
+[browser_bookmarkProperties_editTagContainer.js]
 [browser_bookmarkProperties_readOnlyRoot.js]
 [browser_bookmarksProperties.js]
 [browser_drag_bookmarks_on_toolbar.js]
 skip-if = e10s # Bug ?????? - test fails - "Number of dragged items should be the same. - Got 0, expected 1"
 [browser_forgetthissite_single.js]
 [browser_history_sidebar_search.js]
 skip-if = e10s && (os == 'linux' || os == 'mac') # Bug 1116457
 [browser_library_batch_delete.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/places/tests/browser/browser_bookmarkProperties_editTagContainer.js
@@ -0,0 +1,71 @@
+"use strict"
+
+add_task(function* () {
+  info("Bug 479348 - Properties on a root should be read-only.");
+  let uri = NetUtil.newURI("http://example.com/");
+  let bm = yield PlacesUtils.bookmarks.insert({
+    url: uri.spec,
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid
+  });
+  registerCleanupFunction(function* () {
+    yield PlacesUtils.bookmarks.remove(bm);
+  });
+
+  PlacesUtils.tagging.tagURI(uri, ["tag1"]);
+
+  let library = yield promiseLibrary();
+  let PlacesOrganizer = library.PlacesOrganizer;
+  registerCleanupFunction(function* () {
+    yield promiseLibraryClosed(library);
+  });
+
+  PlacesOrganizer.selectLeftPaneQuery("Tags");
+  let tree = PlacesOrganizer._places;
+  let tagsContainer = tree.selectedNode;
+  tagsContainer.containerOpen = true;
+  let fooTag = tagsContainer.getChild(0);
+  let tagNode = fooTag;
+  tree.selectNode(fooTag);
+  is(tagNode.title, 'tag1', "tagNode title is correct");
+
+  ok(tree.controller.isCommandEnabled("placesCmd_show:info"),
+     "'placesCmd_show:info' on current selected node is enabled");
+
+  yield withBookmarksDialog(
+    true,
+    function openDialog() {
+      tree.controller.doCommand("placesCmd_show:info");
+    },
+    function* test(dialogWin) {
+      // Check that the dialog is not read-only.
+      ok(!dialogWin.gEditItemOverlay.readOnly, "Dialog should not be read-only");
+
+      // Check that name picker is not read only
+      let namepicker = dialogWin.document.getElementById("editBMPanel_namePicker");
+      ok(!namepicker.readOnly, "Name field should not be read-only");
+      is(namepicker.value, "tag1", "Node title is correct");
+
+      let promiseTitleChangeNotification = promiseBookmarksNotification(
+          "onItemChanged", (itemId, prop, isAnno, val) => prop == "title" && val == "tag2");
+
+      fillBookmarkTextField("editBMPanel_namePicker", "tag2", dialogWin);
+
+      yield promiseTitleChangeNotification;
+
+      is(namepicker.value, "tag2", "Node title has been properly edited");
+
+      // Check the shortcut's title.
+      is(tree.selectedNode.title, "tag2", "The node has the correct title");
+
+      // Check the tags have been edited.
+      let tags = PlacesUtils.tagging.getTagsForURI(uri);
+      is(tags.length, 1, "Found the right number of tags");
+      ok(tags.indexOf("tag2") != -1, "Found the expected tag");
+    }
+  );
+
+  // Check the tag change has been reverted.
+  let tags = PlacesUtils.tagging.getTagsForURI(uri);
+  is(tags.length, 1, "Found the right number of tags");
+  ok(tags.indexOf("tag1") != -1, "Found the expected tag");
+});