Pressing 'Delete' key in bookmark manager's search box deletes bookmark.

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 380 matching lines @@
        not used.
  * @param {boolean} commandDisabled Whether the menu item should be disabled
        no matter what bookmarks are selected.
  */
 function updateOpenCommand(e, command, singularId, pluralId, commandDisabled) {
   if (singularId) {
     // The command label reflects the selection which might not reflect
     // how many bookmarks will be opened. For example if you right click an
     // empty area in a folder with 1 bookmark the text should still say "all".
     var selectedNodes = getSelectedBookmarkNodes(e.target).filter(notNewNode);
-    var singular = selectedNodes.length == 1 && !bmm.isFolder(selectedNodes[0]);
+    var singular = selectedNodes && selectedNodes.length == 1 &&
+                   !bmm.isFolder(selectedNodes[0]);
     command.label = loadTimeData.getString(singular ? singularId : pluralId);
   }
 
   if (commandDisabled) {
     command.disabled = true;
     e.canExecute = false;
     return;
   }
 
   getUrlsForOpenCommands(assertInstanceof(e.target, HTMLElement)).then(
@@ skipping 71 matching lines @@
       break;
 
     case 'sort-command':
       e.canExecute = !bmm.list.isSearch() &&
           bmm.list.dataModel && bmm.list.dataModel.length > 1 &&
           !isUnmodifiable(bmm.tree.getBookmarkNodeById(bmm.list.parentId));
       break;
 
     case 'undo-command':
       // If the search box is active, pass the undo command through
       // (fixes http://crbug.com/278112). Otherwise, because

[Bernhard Bauer, 2015/04/15 10:11:57]

(Read my comments bottom-up for context)

Here is where we disable the undo command if the search field is active.

BTW, another possible yak shaving, I mean improvement: Checking whether a
command can run happens by dispatching an event on the active DOM element. If
the search field is active, it would get the first chance to handle this event,
so we could move these checks to a separate event handler that would be
registered on the search field.

       // the global undo command has no visible UI, always enable it, and
       // just make it a no-op if undo is not possible.
       e.canExecute = e.currentTarget.activeElement !== $('term');
       break;
 
     default:
       canExecuteForList(e);
       if (!e.defaultPrevented)
         canExecuteForTree(e);
       break;
@@ skipping 330 matching lines @@
 }
 
 /**
  * Returns the selected bookmark nodes of the provided tree or list.
  * If |opt_target| is not provided or null the active element is used.
  * Only call this if the list or the tree is focused.
  * @param {EventTarget=} opt_target The target list or tree.
  * @return {!Array} Array of bookmark nodes.
  */
 function getSelectedBookmarkNodes(opt_target) {
-  return (opt_target || document.activeElement) == bmm.tree ?
-      bmm.tree.selectedFolders : bmm.list.selectedItems;
+  if (opt_target) {

[Bernhard Bauer, 2015/04/15 10:11:57]

I don't really understand what's going on here. You are changing this method to
return null if the target is null and there is no active element? That doesn't
really match the JSDoc comment, which states that the method will never return
null (per the exclamation mark in the return type; see
http://usejsdoc.org/tags-type.html). Above that, it also says that the method
should only be called if the list or the tree is focused, so that case shouldn't
even be happening.

Also, looking at it in a bigger context, wouldn't this mean that we would
silently swallow a Delete key press instead of applying it to the text field? I
think what we should instead do is not try to handle the delete command at all
if the search field is active, similar to what we do for the undo command (see
my other comment).

+    return (opt_target == bmm.tree) ? bmm.tree.selectedFolders :
+                                      bmm.list.selectedItems;
+  } else if (document.activeElement == bmm.tree ||
+             document.activeElement == bmm.list) {
+    return (document.activeElement == bmm.tree) ? bmm.tree.selectedFolders :
+                                                  bmm.list.selectedItems;
+  }
 }
 
 /**
  * @param {EventTarget=} opt_target The target list or tree.
  * @return {!Array<string>} An array of the selected bookmark IDs.
  */
 function getSelectedBookmarkIds(opt_target) {
   var selectedNodes = getSelectedBookmarkNodes(opt_target);
+  if (!selectedNodes || !selectedNodes.length)
+    return;
   selectedNodes.sort(function(a, b) { return a.index - b.index });
   return selectedNodes.map(function(node) {
     return node.id;
   });
 }
 
 /**
  * @param {BookmarkTreeNode} node The node to test.
  * @return {boolean} Whether the given node is unmodifiable.
  */
@@ skipping 25 matching lines @@
     chrome.bookmarkManagerPrivate.recordLaunch();
   });
 }
 
 /**
  * Opens an item in the list.
  */
 function openItem() {
   var bookmarkNodes = getSelectedBookmarkNodes();
   // If we double clicked or pressed enter on a single folder, navigate to it.
-  if (bookmarkNodes.length == 1 && bmm.isFolder(bookmarkNodes[0]))
+  if (bookmarkNodes && bookmarkNodes.length == 1 &&
+      bmm.isFolder(bookmarkNodes[0])) {
     navigateTo(bookmarkNodes[0].id);
-  else
+  } else {
     openBookmarks(LinkKind.FOREGROUND_TAB);
+  }
 }
 
 /**
  * Refreshes search results after delete or undo-delete.
  * This ensures children of deleted folders do not remain in results
  */
 function updateSearchResults() {
   if (bmm.list.isSearch())
     bmm.list.reload();
 }
 
 /**
  * Deletes the selected bookmarks. The bookmarks are saved in memory in case
  * the user needs to undo the deletion.
  * @param {EventTarget=} opt_target The deleter of bookmarks.
  */
 function deleteBookmarks(opt_target) {
   var selectedIds = getSelectedBookmarkIds(opt_target);
-  if (!selectedIds.length)
+  if (!selectedIds || !selectedIds.length)
     return;
 
   var filteredIds = getFilteredSelectedBookmarkIds(opt_target);
   lastDeleted = {nodes: [], target: opt_target || document.activeElement};
 
   function performDelete() {
     // Only remove filtered ids.
     chrome.bookmarkManagerPrivate.removeTrees(filteredIds);
     $('undo-delete-command').canExecuteChange();
     $('undo-delete-from-folders-menu-command').canExecuteChange();
@@ skipping 293 matching lines @@
 
 /**
  * @param {EventTarget=} opt_target A target to get bookmark IDs from.
  * @return {Array<string>} An array of bookmarks IDs.
  */
 function getFilteredSelectedBookmarkIds(opt_target) {
   // Remove duplicates from filteredIds and return.
   var filteredIds = [];
   // Selected nodes to iterate through for matches.
   var nodes = getSelectedBookmarkNodes(opt_target);
+  if (!nodes)
+    return filteredIds;
 
   for (var i = 0; i < nodes.length; i++)
     if (!hasSelectedAncestor(bmm.tree.getBookmarkNodeById(nodes[i].parentId)))
       filteredIds.splice(0, 0, nodes[i].id);
 
   return filteredIds;
 }
 
 /**
  * Handler for the command event. This is used for context menu of list/tree
@@ skipping 261 matching lines @@
 
   cr.ui.FocusOutlineManager.forDocument(document);
   initializeSplitter();
   bmm.addBookmarkModelListeners();
   dnd.init(selectItemsAfterUserAction);
   bmm.tree.reload();
 }
 
 initializeBookmarkManager();
 })();