DeletableItemList: Make 'X' button delete only the one list item that contains it.

chrome/browser/resources/options/deletable_item_list.js

Index: chrome/browser/resources/options/deletable_item_list.js
diff --git a/chrome/browser/resources/options/deletable_item_list.js b/chrome/browser/resources/options/deletable_item_list.js
index 4f223bad57802ee00c8472335ac9d6207863a788..d625dc5a7ca705e0e27b6957a40019c28818e22a 100644
--- a/chrome/browser/resources/options/deletable_item_list.js
+++ b/chrome/browser/resources/options/deletable_item_list.js
@@ -136,18 +136,15 @@ cr.define('options', function() {
 
       var target = e.target;
       if (target.classList.contains('row-delete-button')) {
+        // Because the close buttons do not give any visual indication that they
+        // would behave differently when multiple items are selected, delete
+        // only the one list item that contains the close button being clicked,
+        // regardless of how many other items are selected. If the user wants to
+        // delete a lot of stuff at once, he would have to press the delete key.

[Evan Stade, 2014/07/31 15:09:26]

I think this comment is too much. Better to make this part of the CL
description.

[engedy, 2014/07/31 16:36:53]

Moved it to the CL description, and replaced it with a much shorter one. WDYT? I
am happy to remove it entire if you still do not like it.

         var listItem = this.getListItemAncestor(target);
-        var selected = this.selectionModel.selectedIndexes;
-
-        // Check if the list item that contains the close button being clicked
-        // is not in the list of selected items. Only delete this item in that
-        // case.
         var idx = this.getIndexOfListItem(listItem);
-        if (selected.indexOf(idx) == -1) {
-          this.deleteItemAtIndex(idx);
-        } else {
-          this.deleteSelectedItems_();
-        }
+        this.deleteItemAtIndex(idx);
+        this.selectionModel.clear();

[engedy, 2014/07/31 09:30:22]

What is the motivation for clearing the selection? (Even though it happens in
most all cases automatically even without this line, as we rebuild the list of
items.)

[Evan Stade, 2014/07/31 15:09:27]

It's my impression that that is the expected behavior --- any time I left click
without holding ctrl or shift, the selection will be cleared.

also based on your comment, it sounds like consistency is another good reason

[engedy, 2014/07/31 16:36:53]

Sorry, turns out my above statement was incorrect:
 1.) We currently have some lists where the selection does not go away when an
item gets deleted (e.g. Content Settings exceptions).
 2.) I have just discovered lines 102--113 in this file, which implements the
complete opposite of what we have just discussed. :-)

Actually, I now think that *not* clearing the selection here would actually make
sense. This way, the 'X' buttons would consistently have absolutely nothing to
do with the current selection: the selection would not affect which item is
deleted, and the selection would be not affected by deletions. I think the
current visual treatment does not contradict this too much.

Then, if a subclass implements deletion in such a way that the underlying model
/ selection model is reset, that would still be consistently done so. (And might
even be considered an erroneous implementation.)

WDYT?

       }
     },