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

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

Index: chrome/browser/resources/bookmark_manager/js/main.js
diff --git a/chrome/browser/resources/bookmark_manager/js/main.js b/chrome/browser/resources/bookmark_manager/js/main.js
index acfc3d7db701566ae61ea65a88c2c33d5ebf04f4..f8c7638ab56b53bf918c5bc22b89edbfe4cb685c 100644
--- a/chrome/browser/resources/bookmark_manager/js/main.js
+++ b/chrome/browser/resources/bookmark_manager/js/main.js
@@ -398,7 +398,8 @@ function updateOpenCommand(e, command, singularId, pluralId, commandDisabled) {
     // 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);
   }
 
@@ -841,8 +842,14 @@ function getLinkController() {
  * @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;
+  }
 }
 
 /**
@@ -851,6 +858,8 @@ function getSelectedBookmarkNodes(opt_target) {
  */
 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;
@@ -896,10 +905,12 @@ function openBookmarks(kind, opt_eventTarget) {
 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);
+  }
 }
 
 /**
@@ -918,7 +929,7 @@ function updateSearchResults() {
  */
 function deleteBookmarks(opt_target) {
   var selectedIds = getSelectedBookmarkIds(opt_target);
-  if (!selectedIds.length)
+  if (!selectedIds || !selectedIds.length)
     return;
 
   var filteredIds = getFilteredSelectedBookmarkIds(opt_target);
@@ -1232,6 +1243,8 @@ function getFilteredSelectedBookmarkIds(opt_target) {
   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)))