Bookmark manager: Pass in explicit targets to the delete and rename commands.

chrome/browser/resources/bookmark_manager/js/main.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 (function() {
 /** @const */ var BookmarkList = bmm.BookmarkList;
 /** @const */ var BookmarkTree = bmm.BookmarkTree;
 /** @const */ var Command = cr.ui.Command;
 /** @const */ var LinkKind = cr.LinkKind;
 /** @const */ var ListItem = cr.ui.ListItem;
@@ skipping 1000 matching lines @@
 function computeParentFolderForNewItem() {
   if (document.activeElement == bmm.tree)
     return bmm.list.parentId;
   var selectedItem = bmm.list.selectedItem;
   return selectedItem && bmm.isFolder(selectedItem) ?
       selectedItem.id : bmm.list.parentId;
 }
 
 /**
  * Callback for rename folder and edit command. This starts editing for
- * selected item.
+ * the passed in target, or the selected item.
+ * @param {EventTarget=} opt_target The target to start editing. If absent or
+ *     null, the selected item will be edited instead.
  */
-function editSelectedItem() {
-  if (document.activeElement == bmm.tree) {
+function editItem(opt_target) {
+  if ((opt_target || document.activeElement) == bmm.tree) {
     bmm.tree.selectedItem.editing = true;
   } else {
     var li = bmm.list.getListItem(bmm.list.selectedItem);
     if (li)
       li.editing = true;
   }
 }
 
 /**
  * Callback for the new folder command. This creates a new folder and starts
@@ skipping 264 matching lines @@
     case 'open-incognito-window-command':
       recordUserAction('OpenIncognito');
       openBookmarks(LinkKind.INCOGNITO,
           assertInstanceof(e.target, HTMLElement));
       break;
 
     case 'delete-from-folders-menu-command':
       target = bmm.tree;
     case 'delete-command':
       recordUserAction('Delete');
-      deleteBookmarks(target);
+      deleteBookmarks(target || assertInstanceof(e.target, HTMLElement));
       break;
 
     case 'copy-from-folders-menu-command':
       target = bmm.tree;
     case 'copy-command':
       recordUserAction('Copy');
       chrome.bookmarkManagerPrivate.copy(getSelectedBookmarkIds(target),
                                          updatePasteCommand);
       break;
 
@@ skipping 19 matching lines @@
     case 'paste-from-context-menu-command':
       pasteBookmark(getSelectedId());
       break;
 
     case 'sort-command':
       recordUserAction('Sort');
       chrome.bookmarkManagerPrivate.sortChildren(bmm.list.parentId);
       break;
 
     case 'rename-folder-command':
-      editSelectedItem();
+      editItem(assertInstanceof(e.target, HTMLElement));
       break;
 
     case 'rename-folder-from-folders-menu-command':
-      bmm.tree.selectedItem.editing = true;
-      break;
-
+      target = bmm.tree;
+      // Fall through.
     case 'edit-command':
       recordUserAction('Edit');
-      editSelectedItem();
+      editItem(target);

[Dan Beam, 2015/04/17 19:26:33]

what happens if target is undefined?  seems entirely possible here...

[Bernhard Bauer, 2015/04/17 21:46:51]

Yup, that's intended here. editItem() will fall back to using the currently
selected item (via document.activeElement), which matches the behavior of the
old editEditSelectedItem().

[Dan Beam, 2015/04/17 23:35:16]

ok, thanks for explaining

       break;

[Dan Beam, 2015/04/17 19:26:33]

nit: maybe:

  case 'edit-command':
    recordUserAction('Edit');
    // Fall through.

  case 'rename-folder-from-folders-menu-command':
    target = bmm.tree;
    // Fall through.

  case 'rename-folder-command':
    editItem(target || assertInstanceof(e.target, HTMLElement));
    break;

[Bernhard Bauer, 2015/04/17 21:46:51]

That won't work, because for edit we want to pass undefined (see my comment
above). What I do need to change though is to not record the user action for
rename-folder-from-folders-menu, at which point I might just directly call
editItem() and break in each of the cases. WDYT?

[Dan Beam, 2015/04/17 23:35:16]

yeah, that's fine

[Bernhard Bauer, 2015/04/20 07:40:17]

Ok, done.

 
     case 'new-folder-from-folders-menu-command':
       target = bmm.tree;
     case 'new-folder-command':
       recordUserAction('NewFolder');
       newFolder(target);
       break;
 
     case 'add-new-bookmark-command':
       recordUserAction('AddPage');
@@ skipping 147 matching lines @@
 
   cr.ui.FocusOutlineManager.forDocument(document);
   initializeSplitter();
   bmm.addBookmarkModelListeners();
   dnd.init(selectItemsAfterUserAction);
   bmm.tree.reload();
 }
 
 initializeBookmarkManager();
 })();