Bug 210876 – Web Inspector: Storage: cannot select multiple local storage entries

• Component: Web Inspector
• Status: RESOLVED FIXED

[Jon Lee]

Split off from bug 209867.

I'd like to be able to delete multiple key/values pairs in local storage in WI.

This bug tracks the following requests:
- Allow selection of multiple rows
- Command-A to select all rows

Devin says:
> These both should be solved if we switch from `WI.DataGrid` to `WI.Table`.  We'd need to either add logic to `WI.Table` to support editing by default, or add custom logic to `WI.DOMStorageContentView` for editing.

[Devin Rousso]

Created attachment 397299 [details]
Patch

[Devin Rousso]

Comment on attachment 397299 [details]
Patch

>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 773117f5462c7f2a5074eea64fa1687b0c0a143e..9f69ab78771905448fb61a6b79643b30c7c332a8 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,82 @@
>[phone]  Devin Rousso  <[e-mail]>
>+
>+        Web Inspector: Storage: cannot select multiple local storage entries
>+        https://bugs.webkit.org/show_bug.cgi?id=210876
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Support multiple selection using `WI.DataGrid`.
>+
>+        * UserInterface/Views/DataGrid.js:
>+        (WI.DataGrid):
>+        (WI.DataGrid.prototype.get allowsMultipleSelection): Added.
>+        (WI.DataGrid.prototype.set allowsMultipleSelection): Added.
>+        (WI.DataGrid.prototype.get selectedNode):
>+        (WI.DataGrid.prototype.set selectedNode):
>+        (WI.DataGrid.prototype.get selectedDataGridNodes): Added.
>+        (WI.DataGrid.prototype._keyDown):
>+        (WI.DataGrid.prototype.selectNodes):
>+        (WI.DataGrid.prototype._mouseDownInDataTable):
>+        (WI.DataGrid.prototype._contextMenuInDataTable):
>+        (WI.DataGrid.prototype.handleCopyEvent):
>+        (WI.DataGrid.prototype._copyRow):
>+        (WI.DataGrid.prototype._copyTable):
>+        (WI.DataGrid.prototype._hasCopyableData):
>+        (WI.DataGrid.prototype.selectDataGridNodeInternal): Added.
>+        (WI.DataGrid.prototype.deselectDataGridNodeInternal): Added.
>+        (WI.DataGrid.prototype._dispatchSelectedNodeChangedEvent): Added.
>+        (WI.DataGrid.prototype.dataGridNodeForSelectionItem): Added.
>+        (WI.DataGrid.prototype.selectionItemForDataGridNode): Added.
>+        (WI.DataGrid.prototype.selectionControllerSelectionDidChange): Added.
>+        (WI.DataGrid.prototype.selectionControllerFirstSelectableItem): Added.
>+        (WI.DataGrid.prototype.selectionControllerLastSelectableItem): Added.
>+        (WI.DataGrid.prototype.selectionControllerPreviousSelectableItem): Added.
>+        (WI.DataGrid.prototype.selectionControllerNextSelectableItem): Added.
>+        * UserInterface/Views/DataGridNode.js:
>+        (WI.DataGridNode.prototype.select):
>+        (WI.DataGridNode.prototype.deselect):
>+        Replace `selectedNode` with a `WI.SelectionController` that behaves like a `WI.TreeOutline`.
>+        Use the `WI.SelectionController.Reason` to ensure that `WI.PlaceholderDataGridNode` are not
>+        selected when selecting all nodes. Ensure that `WI.PlaceholderDataGridNode` are not copied.
>+        Prefer to use `_rows` instead of `children` as the latter is not sorted/filtered.
>+
>+        * UserInterface/Controllers/SelectionController.js:
>+        (WI.SelectionController.createTreeComparator): Added.
>+        (WI.SelectionController.createListComparator): Added.
>+        Create `static` helper functions for common comparators.
>+
>+        (WI.SelectionController.prototype.deselectItem):
>+        (WI.SelectionController.prototype.selectAll):
>+        (WI.SelectionController.prototype.removeSelectedItems):
>+        (WI.SelectionController.prototype.handleItemMouseDown):
>+        (WI.SelectionController.prototype._selectItemsFromArrowKey):
>+        (WI.SelectionController.prototype._firstSelectableItem):
>+        (WI.SelectionController.prototype._lastSelectableItem):
>+        (WI.SelectionController.prototype._previousSelectableItem):
>+        (WI.SelectionController.prototype._nextSelectableItem):
>+        (WI.SelectionController.prototype._addRange):
>+        (WI.SelectionController.prototype._deleteRange):
>+        Introduce a `WI.SelectionController.Reason` which is used to tell the `_delegate` about why
>+        it's being asked for information.
>+
>+        * UserInterface/Views/DOMStorageContentView.js:
>+        (WI.DOMStorageContentView):
>+        (WI.DOMStorageContentView.prototype._deleteCallback):
>+        Support multiple selection, including deleting multiple rows at once.
>+
>+        * UserInterface/Views/Table.js:
>+        (WI.Table):
>+        * UserInterface/Views/TreeOutline.js:
>+        (WI.TreeOutline):
>+        (WI.TreeOutline.prototype.selectionControllerNumberOfItems): Deleted.
>+        Removed unused `selectionControllerNumberOfItems`.
>+
>+        * UserInterface/Views/ProfileView.js:
>+        (WI.ProfileView):
>+        (WI.ProfileView.prototype._dataGridNodeSelected):
>+        Maintain a `_selectedDataGridNode` so that `oldSelectedNode` doesn't have to be included
>+        when dispatching `WI.DataGrid.Event.SelectedNodeChanged`.
>+
> 2020-04-22  Devin Rousso  <[e-mail]>
> 
>         Web Inspector: Storage: unable to filter cookies
>diff --git a/Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js b/Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js
>index dbb0ae7081e4ac4f386a1e6ef351d9181c022cff..9409204956f2dd250e1c59ab9085868fdc5b5131 100644
>--- a/Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js
>+++ b/Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js
>@@ -48,6 +48,66 @@ WI.SelectionController = class SelectionController extends WI.Object
>         console.assert(this._delegate.selectionControllerPreviousSelectableItem, "SelectionController delegate must implement selectionControllerPreviousSelectableItem.");
>     }
> 
>+    // Static
>+
>+    static createTreeComparator(itemForRepresentedObject)
>+    {
>+        return (a, b) => {
>+            // Translate represented objects to TreeElements, which have the
>+            // hierarchical information needed to perform the comparison.
>+            a = itemForRepresentedObject(a);
>+            b = itemForRepresentedObject(b);
>+            if (!a || !b)
>+                return 0;
>+
>+            let getLevel = (item) => {
>+                let level = 0;
>+                while (item = item.parent)
>+                    level++;
>+                return level;
>+            };
>+
>+            let compareSiblings = (s, t) => {
>+                return s.parent.children.indexOf(s) - s.parent.children.indexOf(t);
>+            };
>+
>+            if (a.parent === b.parent)
>+                return compareSiblings(a, b);
>+
>+            let aLevel = getLevel(a);
>+            let bLevel = getLevel(b);
>+            while (aLevel > bLevel) {
>+                if (a.parent === b)
>+                    return 1;
>+                a = a.parent;
>+                aLevel--;
>+            }
>+            while (bLevel > aLevel) {
>+                if (b.parent === a)
>+                    return -1;
>+                b = b.parent;
>+                bLevel--;
>+            }
>+
>+            while (a.parent !== b.parent) {
>+                a = a.parent;
>+                b = b.parent;
>+            }
>+
>+            console.assert(a.parent === b.parent, "Missing common ancestor for TreeElements.", a, b);
>+            return compareSiblings(a, b);
>+        };
>+    }
>+
>+    static createListComparator(indexForRepresentedObject)
>+    {
>+        console.assert(indexForRepresentedObject);
>+
>+        return (a, b) => {
>+            return indexForRepresentedObject(a) - indexForRepresentedObject(b);
>+        };
>+    }
>+
>     // Public
> 
>     get delegate() { return this._delegate; }
>@@ -129,17 +189,19 @@ WI.SelectionController = class SelectionController extends WI.Object
>             this._lastSelectedItem = null;
> 
>             if (newItems.size) {
>+                const reason = WI.SelectionController.Reason.Extend;
>+
>                 // Find selected item closest to deselected item.
>                 let previous = item;
>                 let next = item;
>                 while (!this._lastSelectedItem && previous && next) {
>-                    previous = this._previousSelectableItem(previous);
>+                    previous = this._previousSelectableItem(previous, reason);
>                     if (this.hasSelectedItem(previous)) {
>                         this._lastSelectedItem = previous;
>                         break;
>                     }
> 
>-                    next = this._nextSelectableItem(next);
>+                    next = this._nextSelectableItem(next, reason);
>                     if (this.hasSelectedItem(next)) {
>                         this._lastSelectedItem = next;
>                         break;
>@@ -159,10 +221,10 @@ WI.SelectionController = class SelectionController extends WI.Object
>         if (!this._allowsMultipleSelection)
>             return;
> 
>-        this.reset();
>+        const reason = WI.SelectionController.Reason.All;
> 
>         let newItems = new Set;
>-        this._addRange(newItems, this._firstSelectableItem(), this._lastSelectableItem());
>+        this._addRange(newItems, this._firstSelectableItem(reason), this._lastSelectableItem(reason));
>         this.selectItems(newItems);
>     }
> 
>@@ -176,22 +238,24 @@ WI.SelectionController = class SelectionController extends WI.Object
>         if (!this._selectedItems.size)
>             return;
> 
>+        const reason = WI.SelectionController.Reason.Extend;
>+
>         let orderedSelection = Array.from(this._selectedItems).sort(this._comparator);
> 
>         // Try selecting the item preceding the selection.
>         let firstSelectedItem = orderedSelection[0];
>-        let itemToSelect = this._previousSelectableItem(firstSelectedItem);
>+        let itemToSelect = this._previousSelectableItem(firstSelectedItem, reason);
>         if (!itemToSelect) {
>             // If no item exists before the first item in the selection, try selecting
>             // a deselected item (hole) within the selection.
>             itemToSelect = firstSelectedItem;
>             while (itemToSelect && this.hasSelectedItem(itemToSelect))
>-                itemToSelect = this._nextSelectableItem(itemToSelect);
>+                itemToSelect = this._nextSelectableItem(itemToSelect, reason);
> 
>             if (!itemToSelect || this.hasSelectedItem(itemToSelect)) {
>                 // If the selection contains no holes, try selecting the item
>                 // following the selection.
>-                itemToSelect = this._nextSelectableItem(orderedSelection.lastValue);
>+                itemToSelect = this._nextSelectableItem(orderedSelection.lastValue, reason);
>             }
>         }
> 
>@@ -265,7 +329,7 @@ WI.SelectionController = class SelectionController extends WI.Object
>         // through the clicked item to be selected.
>         if (!newItems.size) {
>             this._lastSelectedItem = item;
>-            this._shiftAnchorItem = this._firstSelectableItem();
>+            this._shiftAnchorItem = this._firstSelectableItem(WI.SelectionController.Reason.Direct);
> 
>             this._addRange(newItems, this._shiftAnchorItem, this._lastSelectedItem);
>             this._updateSelectedItems(newItems);
>@@ -320,16 +384,18 @@ WI.SelectionController = class SelectionController extends WI.Object
> 
>     _selectItemsFromArrowKey(goingUp, shiftKey)
>     {
>+        let extendSelection = shiftKey && this._allowsMultipleSelection;
>+        let reason = (extendSelection && this._selectedItems.size) ? WI.SelectionController.Reason.Extend : WI.SelectionController.Reason.Direct;
>+
>         if (!this._selectedItems.size) {
>-            this.selectItem(goingUp ? this._lastSelectableItem() : this._firstSelectableItem());
>+            this.selectItem(goingUp ? this._lastSelectableItem(reason) : this._firstSelectableItem(reason));
>             return;
>         }
> 
>-        let item = goingUp ? this._previousSelectableItem(this._lastSelectedItem) : this._nextSelectableItem(this._lastSelectedItem);
>+        let item = goingUp ? this._previousSelectableItem(this._lastSelectedItem, reason) : this._nextSelectableItem(this._lastSelectedItem, reason);
>         if (!item)
>             return;
> 
>-        let extendSelection = shiftKey && this._allowsMultipleSelection;
>         if (!extendSelection || !this.hasSelectedItem(item)) {
>             this.selectItem(item, extendSelection);
>             return;
>@@ -338,7 +404,7 @@ WI.SelectionController = class SelectionController extends WI.Object
>         // Since the item in the direction of movement is selected, we are either
>         // extending the selection into the item, or deselecting. Determine which
>         // by checking whether the item opposite the anchor item is selected.
>-        let priorItem = goingUp ? this._nextSelectableItem(this._lastSelectedItem) : this._previousSelectableItem(this._lastSelectedItem);
>+        let priorItem = goingUp ? this._nextSelectableItem(this._lastSelectedItem, reason) : this._previousSelectableItem(this._lastSelectedItem, reason);
>         if (!priorItem || !this.hasSelectedItem(priorItem)) {
>             this.deselectItem(this._lastSelectedItem);
>             return;
>@@ -354,28 +420,28 @@ WI.SelectionController = class SelectionController extends WI.Object
>             }
> 
>             this._lastSelectedItem = item;
>-            item = goingUp ? this._previousSelectableItem(item) : this._nextSelectableItem(item);
>+            item = goingUp ? this._previousSelectableItem(item, reason) : this._nextSelectableItem(item, reason);
>         }
>     }
> 
>-    _firstSelectableItem()
>+    _firstSelectableItem(reason)
>     {
>-        return this._delegate.selectionControllerFirstSelectableItem(this);
>+        return this._delegate.selectionControllerFirstSelectableItem(this, reason);
>     }
> 
>-    _lastSelectableItem()
>+    _lastSelectableItem(reason)
>     {
>-        return this._delegate.selectionControllerLastSelectableItem(this);
>+        return this._delegate.selectionControllerLastSelectableItem(this, reason);
>     }
> 
>-    _previousSelectableItem(item)
>+    _previousSelectableItem(item, reason)
>     {
>-        return this._delegate.selectionControllerPreviousSelectableItem(this, item);
>+        return this._delegate.selectionControllerPreviousSelectableItem(this, item, reason);
>     }
> 
>-    _nextSelectableItem(item)
>+    _nextSelectableItem(item, reason)
>     {
>-        return this._delegate.selectionControllerNextSelectableItem(this, item);
>+        return this._delegate.selectionControllerNextSelectableItem(this, item, reason);
>     }
> 
>     _updateSelectedItems(items)
>@@ -394,12 +460,14 @@ WI.SelectionController = class SelectionController extends WI.Object
> 
>     _addRange(items, firstItem, lastItem)
>     {
>+        const reason = WI.SelectionController.Reason.Extend;
>+
>         let current = firstItem;
>         while (current) {
>             items.add(current);
>             if (current === lastItem)
>                 break;
>-            current = this._nextSelectableItem(current);
>+            current = this._nextSelectableItem(current, reason);
>         }
> 
>         console.assert(!lastItem || items.has(lastItem), "End of range could not be reached.");
>@@ -407,14 +475,22 @@ WI.SelectionController = class SelectionController extends WI.Object
> 
>     _deleteRange(items, firstItem, lastItem)
>     {
>+        const reason = WI.SelectionController.Reason.Extend;
>+
>         let current = firstItem;
>         while (current) {
>             items.delete(current);
>             if (current === lastItem)
>                 break;
>-            current = this._nextSelectableItem(current);
>+            current = this._nextSelectableItem(current, reason);
>         }
> 
>         console.assert(!lastItem || !items.has(lastItem), "End of range could not be reached.");
>     }
> };
>+
>+WI.SelectionController.Reason = {
>+    Direct: Symbol("selection-reason-direct"),
>+    Extend: Symbol("selection-reason-extend"),
>+    All: Symbol("selection-reason-all"),
>+};
>diff --git a/Source/WebInspectorUI/UserInterface/Views/DOMStorageContentView.js b/Source/WebInspectorUI/UserInterface/Views/DOMStorageContentView.js
>index 57599a7db6d2edd5c609e61ddf8bfcdf3a6c41ef..35e4bb4b0e8620ec633b16dab9ef3719e07d1101 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/DOMStorageContentView.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/DOMStorageContentView.js
>@@ -48,6 +48,7 @@ WI.DOMStorageContentView = class DOMStorageContentView extends WI.ContentView
>         });
>         this._dataGrid.sortOrder = WI.DataGrid.SortOrder.Ascending;
>         this._dataGrid.sortColumnIdentifier = "key";
>+        this._dataGrid.allowsMultipleSelection = true;
>         this._dataGrid.createSettings("dom-storage-content-view");
>         this._dataGrid.addEventListener(WI.DataGrid.Event.SortChanged, this._sortDataGrid, this);
>         this.addSubview(this._dataGrid);
>@@ -193,13 +194,14 @@ WI.DOMStorageContentView = class DOMStorageContentView extends WI.ContentView
>         this._dataGrid.sortNodesImmediately(comparator);
>     }
> 
>-    _deleteCallback(node)
>+    _deleteCallback()
>     {
>-        if (!node || node.isPlaceholderNode)
>-            return;
>-
>-        this._dataGrid.removeChild(node);
>-        this.representedObject.removeItem(node.data["key"]);
>+        for (let dataGridNode of this._dataGrid.selectedDataGridNodes) {
>+            if (dataGridNode.isPlaceholderNode)
>+                continue;
>+            this._dataGrid.removeChild(dataGridNode);
>+            this.representedObject.removeItem(dataGridNode.data["key"]);
>+        }
>     }
> 
>     _editingCallback(editingNode, columnIdentifier, oldText, newText, moveDirection)
>diff --git a/Source/WebInspectorUI/UserInterface/Views/DataGrid.js b/Source/WebInspectorUI/UserInterface/Views/DataGrid.js
>index bc5ba75fb5412a1549e3253e6c827ce51c87b343..8b126288ab4bcb7a2a106acec82029dab3f8f60b 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/DataGrid.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/DataGrid.js
>@@ -44,7 +44,6 @@ WI.DataGrid = class DataGrid extends WI.View
>         this._rows = [];
> 
>         this.children = [];
>-        this.selectedNode = null;
>         this.expandNodesWhenArrowing = false;
>         this.root = true;
>         this.hasChildren = false;
>@@ -68,6 +67,13 @@ WI.DataGrid = class DataGrid extends WI.View
>         this._filterDelegate = null;
>         this._filterDidModifyNodeWhileProcessingItems = false;
> 
>+        let itemForRepresentedObject = this.dataGridNodeForSelectionItem.bind(this);
>+        let selectionComparator = WI.SelectionController.createTreeComparator(itemForRepresentedObject);
>+        this._selectionController = new WI.SelectionController(this, selectionComparator);
>+
>+        this._processingSelectionChange = false;
>+        this._suppressNextSelectionDidChangeEvent = false;
>+
>         this.element.className = "data-grid";
>         this.element.tabIndex = 0;
>         this.element.addEventListener("keydown", this._keyDown.bind(this), false);
>@@ -318,6 +324,45 @@ WI.DataGrid = class DataGrid extends WI.View
>         this._updateScrollListeners();
>     }
> 
>+    get allowsMultipleSelection()
>+    {
>+        return this._selectionController.allowsMultipleSelection;
>+    }
>+
>+    set allowsMultipleSelection(flag)
>+    {
>+        this._selectionController.allowsMultipleSelection = flag;
>+    }
>+
>+    get selectedNode()
>+    {
>+        return this.dataGridNodeForSelectionItem(this._selectionController.lastSelectedItem);
>+    }
>+
>+    set selectedNode(dataGridNode)
>+    {
>+        if (dataGridNode)
>+            this._selectionController.selectItem(this.selectionItemForDataGridNode(dataGridNode));
>+        else
>+            this._selectionController.deselectAll();
>+    }
>+
>+    get selectedDataGridNodes()
>+    {
>+        if (this.allowsMultipleSelection) {
>+            let selectedDataGridNodes = [];
>+            for (let item of this._selectionController.selectedItems)
>+                selectedDataGridNodes.push(this.dataGridNodeForSelectionItem(item));
>+            return selectedDataGridNodes;
>+        }
>+
>+        let selectedNode = this.selectedNode;
>+        if (selectedNode)
>+            return [selectedNode];
>+
>+        return [];
>+    }
>+
>     get filterText() { return this._filterText; }
> 
>     set filterText(x)
>@@ -1366,64 +1411,70 @@ WI.DataGrid = class DataGrid extends WI.View
> 
>     _keyDown(event)
>     {
>-        if (!this.selectedNode || event.shiftKey || event.metaKey || event.ctrlKey || this._editing)
>+        if (this._editing)
>             return;
> 
>-        let isRTL = WI.resolvedLayoutDirection() === WI.LayoutDirection.RTL;
>+        let isRTL = WI.resolveLayoutDirectionForElement(this.element) === WI.LayoutDirection.RTL;
>+        let expandKeyIdentifier = isRTL ? "Left" : "Right";
>+        let collapseKeyIdentifier = isRTL ? "Right" : "Left";
> 
>         var handled = false;
>         var nextSelectedNode;
>-        if (event.keyIdentifier === "Up" && !event.altKey) {
>-            nextSelectedNode = this.selectedNode.traversePreviousNode(true);
>-            while (nextSelectedNode && !nextSelectedNode.selectable)
>-                nextSelectedNode = nextSelectedNode.traversePreviousNode(true);
>-            handled = nextSelectedNode ? true : false;
>-        } else if (event.keyIdentifier === "Down" && !event.altKey) {
>-            nextSelectedNode = this.selectedNode.traverseNextNode(true);
>-            while (nextSelectedNode && !nextSelectedNode.selectable)
>-                nextSelectedNode = nextSelectedNode.traverseNextNode(true);
>-            handled = nextSelectedNode ? true : false;
>-        } else if ((!isRTL && event.keyIdentifier === "Left") || (isRTL && event.keyIdentifier === "Right")) {
>-            if (this.selectedNode.expanded) {
>-                if (event.altKey)
>-                    this.selectedNode.collapseRecursively();
>-                else
>-                    this.selectedNode.collapse();
>-                handled = true;
>-            } else if (this.selectedNode.parent && !this.selectedNode.parent.root) {
>-                handled = true;
>-                if (this.selectedNode.parent.selectable) {
>-                    nextSelectedNode = this.selectedNode.parent;
>-                    handled = nextSelectedNode ? true : false;
>-                } else if (this.selectedNode.parent)
>-                    this.selectedNode.parent.collapse();
>-            }
>-        } else if ((!isRTL && event.keyIdentifier === "Right") || (isRTL && event.keyIdentifier === "Left")) {
>-            if (!this.selectedNode.revealed) {
>-                this.selectedNode.reveal();
>-                handled = true;
>-            } else if (this.selectedNode.hasChildren) {
>-                handled = true;
>+
>+        if (this.selectedNode) {
>+            if (event.keyIdentifier === collapseKeyIdentifier) {
>                 if (this.selectedNode.expanded) {
>-                    nextSelectedNode = this.selectedNode.children[0];
>-                    handled = nextSelectedNode ? true : false;
>-                } else {
>                     if (event.altKey)
>-                        this.selectedNode.expandRecursively();
>+                        this.selectedNode.collapseRecursively();
>                     else
>-                        this.selectedNode.expand();
>+                        this.selectedNode.collapse();
>+                    handled = true;
>+                } else if (this.selectedNode.parent && !this.selectedNode.parent.root) {
>+                    handled = true;
>+                    if (this.selectedNode.parent.selectable) {
>+                        nextSelectedNode = this.selectedNode.parent;
>+                        while (nextSelectedNode && !nextSelectedNode.selectable)
>+                            nextSelectedNode = nextSelectedNode.parent;
>+                        handled = nextSelectedNode ? true : false;
>+                    } else if (this.selectedNode.parent)
>+                        this.selectedNode.parent.collapse();
>+                }
>+            } else if (event.keyIdentifier === expandKeyIdentifier) {
>+                if (!this.selectedNode.revealed) {
>+                    this.selectedNode.reveal();
>+                    handled = true;
>+                } else if (this.selectedNode.hasChildren) {
>+                    handled = true;
>+                    if (this.selectedNode.expanded) {
>+                        nextSelectedNode = this.selectedNode.children[0];
>+                        while (nextSelectedNode && !nextSelectedNode.selectable)
>+                            nextSelectedNode = nextSelectedNode.nextSibling;
>+                        handled = nextSelectedNode ? true : false;
>+                    } else {
>+                        if (event.altKey)
>+                            this.selectedNode.expandRecursively();
>+                        else
>+                            this.selectedNode.expand();
>+                    }
>+                }
>+            } else if (event.keyCode === 8 /* Backspace */ || event.keyCode === 46 /* Delete */) {
>+                if (this._deleteCallback) {
>+                    handled = true;
>+                    this._deleteCallback(this.selectedNode);
>+                }
>+            } else if (isEnterKey(event)) {
>+                if (this._editCallback) {
>+                    handled = true;
>+                    this._startEditing(this.selectedNode.element.children[0]);
>                 }
>             }
>-        } else if (event.keyCode === 8 || event.keyCode === 46) {
>-            if (this._deleteCallback) {
>-                handled = true;
>-                this._deleteCallback(this.selectedNode);
>-            }
>-        } else if (isEnterKey(event)) {
>-            if (this._editCallback) {
>-                handled = true;
>-                this._startEditing(this.selectedNode.element.children[0]);
>-            }
>+        }
>+
>+        if (!handled) {
>+            handled = this._selectionController.handleKeyDown(event);
>+
>+            if (handled)
>+                nextSelectedNode = this.selectedNode;
>         }
> 
>         if (nextSelectedNode) {
>@@ -1462,6 +1513,24 @@ WI.DataGrid = class DataGrid extends WI.View
>         // This is the root, do nothing.
>     }
> 
>+    selectNodes(nodes)
>+    {
>+        if (!nodes.length)
>+            return;
>+
>+        if (nodes.length === 1) {
>+            this.selectedNode = nodes[0];
>+            return;
>+        }
>+
>+        console.assert(this.allowsMultipleSelection, "Cannot select multiple DataGridNode with multiple selection disabled.");
>+        if (!this.allowsMultipleSelection)
>+            return;
>+
>+        let selectableObjects = nodes.map((node) => this.selectionItemForDataGridNode(node));
>+        this._selectionController.selectItems(new Set(selectableObjects));
>+    }
>+
>     dataGridNodeFromNode(target)
>     {
>         var rowElement = target.closest("tr");
>@@ -1578,23 +1647,16 @@ WI.DataGrid = class DataGrid extends WI.View
> 
>     _mouseDownInDataTable(event)
>     {
>-        var gridNode = this.dataGridNodeFromNode(event.target);
>-        if (!gridNode) {
>-            if (this.selectedNode)
>-                this.selectedNode.deselect();
>+        let dataGridNode = this.dataGridNodeFromNode(event.target);
>+        if (!dataGridNode) {
>+            this._selectionController.deselectAll();
>             return;
>         }
> 
>-        if (!gridNode.selectable || gridNode.isEventWithinDisclosureTriangle(event))
>+        if (!dataGridNode.selectable || dataGridNode.isEventWithinDisclosureTriangle(event))
>             return;
> 
>-        if (event.metaKey) {
>-            if (gridNode.selected)
>-                gridNode.deselect();
>-            else
>-                gridNode.select();
>-        } else
>-            gridNode.select();
>+        this._selectionController.handleItemMouseDown(this.selectionItemForDataGridNode(dataGridNode), event);
>     }
> 
>     _contextMenuInHeader(event)
>@@ -1666,7 +1728,7 @@ WI.DataGrid = class DataGrid extends WI.View
> 
>         if (gridNode) {
>             if (gridNode.selectable && gridNode.copyable && !gridNode.isEventWithinDisclosureTriangle(event)) {
>-                contextMenu.appendItem(WI.UIString("Copy Row"), this._copyRow.bind(this, event.target));
>+                contextMenu.appendItem(WI.UIString("Copy Row"), this._copyRow.bind(this, gridNode));
>                 contextMenu.appendItem(WI.UIString("Copy Table"), this._copyTable.bind(this));
> 
>                 if (this.dataGrid._editCallback) {
>@@ -1746,22 +1808,30 @@ WI.DataGrid = class DataGrid extends WI.View
> 
>     handleCopyEvent(event)
>     {
>-        if (!this.selectedNode || !window.getSelection().isCollapsed)
>+        if (!window.getSelection().isCollapsed)
>             return;
> 
>-        var copyText = this._copyTextForDataGridNode(this.selectedNode);
>-        event.clipboardData.setData("text/plain", copyText);
>+        let copyData = [];
>+        for (let dataGridNode of this.selectedDataGridNodes) {
>+            if (!dataGridNode.copyable || dataGridNode.isPlaceholderNode)
>+                continue;
>+            copyData.push(this._copyTextForDataGridNode(dataGridNode));
>+        }
>+
>+        if (!copyData.length)
>+            return;
>+
>+        event.clipboardData.setData("text/plain", copyData.join("\n"));
>         event.stopPropagation();
>         event.preventDefault();
>     }
> 
>-    _copyRow(target)
>+    _copyRow(dataGridNode)
>     {
>-        var gridNode = this.dataGridNodeFromNode(target);
>-        if (!gridNode)
>+        if (!dataGridNode.copyable || dataGridNode.isPlaceholderNode)
>             return;
> 
>-        var copyText = this._copyTextForDataGridNode(gridNode);
>+        let copyText = this._copyTextForDataGridNode(dataGridNode);
>         InspectorFrontendHost.copyText(copyText);
>     }
> 
>@@ -1769,19 +1839,28 @@ WI.DataGrid = class DataGrid extends WI.View
>     {
>         let copyData = [];
>         copyData.push(this._copyTextForDataGridHeaders());
>-        for (let gridNode of this.children) {
>-            if (!gridNode.copyable)
>+        for (let dataGridNode of this._rows) {
>+            if (!dataGridNode.copyable || dataGridNode.isPlaceholderNode)
>                 continue;
>-            copyData.push(this._copyTextForDataGridNode(gridNode));
>+            copyData.push(this._copyTextForDataGridNode(dataGridNode));
>         }
> 
>+        if (!copyData.length)
>+            return;
>+
>         InspectorFrontendHost.copyText(copyData.join("\n"));
>     }
> 
>     _hasCopyableData()
>     {
>-        let gridNode = this.children[0];
>-        return gridNode && gridNode.selectable && gridNode.copyable;
>+        const skipHidden = true;
>+        const stayWithin = null;
>+        const dontPopulate = true;
>+
>+        let dataGridNode = this._rows[0];
>+        while (dataGridNode && (!dataGridNode.selectable || !dataGridNode.copyable || dataGridNode.isPlaceholderNode))
>+            dataGridNode = dataGridNode.traverseNextNode(skipHidden, stayWithin, dontPopulate);
>+        return !!dataGridNode;
>     }
> 
>     resizerDragStarted(resizer)
>@@ -1904,6 +1983,36 @@ WI.DataGrid = class DataGrid extends WI.View
>         this._applyFilterToNodesTask.start();
>     }
> 
>+    selectDataGridNodeInternal(dataGridNode, suppressSelectedEvent)
>+    {
>+        if (this._processingSelectionChange)
>+            return;
>+
>+        this._suppressNextSelectionDidChangeEvent = suppressSelectedEvent;
>+
>+        this._selectionController.selectItem(this.selectionItemForDataGridNode(dataGridNode));
>+    }
>+
>+    deselectDataGridNodeInternal(dataGridNode, suppressDeselectedEvent)
>+    {
>+        if (this._processingSelectionChange)
>+            return;
>+
>+        this._suppressNextSelectionDidChangeEvent = suppressDeselectedEvent;
>+
>+        this._selectionController.deselectItem(this.selectionItemForDataGridNode(dataGridNode));
>+    }
>+
>+    _dispatchSelectedNodeChangedEvent()
>+    {
>+        if (this._suppressNextSelectionDidChangeEvent) {
>+            this._suppressNextSelectionDidChangeEvent = false;
>+            return;
>+        }
>+
>+        this.dispatchEventToListeners(WI.DataGrid.Event.SelectedNodeChanged);
>+    }
>+
>     // YieldableTask delegate
> 
>     yieldableTaskWillProcessItem(task, node)
>@@ -1927,6 +2036,95 @@ WI.DataGrid = class DataGrid extends WI.View
>     {
>         this._applyFilterToNodesTask = null;
>     }
>+
>+    // SelectionController delegate
>+
>+    dataGridNodeForSelectionItem(item)
>+    {
>+        console.assert(!item || item instanceof WI.DataGridNode);
>+        return item;
>+    }
>+
>+    selectionItemForDataGridNode(dataGridNode)
>+    {
>+        console.assert(!dataGridNode || dataGridNode instanceof WI.DataGridNode);
>+        return dataGridNode;
>+    }
>+
>+    selectionControllerSelectionDidChange(selectionController, deselectedItems, selectedItems)
>+    {
>+        this._processingSelectionChange = true;
>+
>+        for (let item of deselectedItems) {
>+            let dataGridNode = this.dataGridNodeForSelectionItem(item);
>+            dataGridNode?.deselect();
>+        }
>+
>+        for (let item of selectedItems) {
>+            let dataGridNode = this.dataGridNodeForSelectionItem(item);
>+            dataGridNode?.select();
>+        }
>+
>+        this._dispatchSelectedNodeChangedEvent();
>+
>+        this._processingSelectionChange = false;
>+    }
>+
>+    selectionControllerFirstSelectableItem(controller, reason)
>+    {
>+        let firstChild = this._rows[0];
>+        let item = this.selectionItemForDataGridNode(firstChild);
>+        if (firstChild.selectable && (!firstChild.isPlaceholderNode || reason !== WI.SelectionController.Reason.All))
>+            return item;
>+        return this.selectionControllerNextSelectableItem(controller, item, reason);
>+    }
>+
>+    selectionControllerLastSelectableItem(controller, reason)
>+    {
>+        let lastChild = this._rows.lastValue;
>+        while (lastChild.expanded && lastChild.children.length)
>+            lastChild = lastChild.children.lastValue;
>+
>+        let item = this.selectionItemForDataGridNode(lastChild);
>+        if (lastChild.selectable && (!lastChild.isPlaceholderNode || reason !== WI.SelectionController.Reason.All))
>+            return item;
>+        return this.selectionControllerPreviousSelectableItem(controller, item, reason);
>+    }
>+
>+    selectionControllerPreviousSelectableItem(controller, item, reason)
>+    {
>+        let dataGridNode = this.dataGridNodeForSelectionItem(item);
>+        console.assert(dataGridNode, "Missing DataGridNode for selection item.", item);
>+        if (!dataGridNode)
>+            return null;
>+
>+        const skipUnrevealed = true;
>+        const dontPopulate = true;
>+        while (dataGridNode = dataGridNode.traversePreviousNode(skipUnrevealed, dontPopulate)) {
>+            if (dataGridNode.selectable && (!dataGridNode.isPlaceholderNode || reason !== WI.SelectionController.Reason.All))
>+                return this.selectionItemForDataGridNode(dataGridNode);
>+        }
>+
>+        return null;
>+    }
>+
>+    selectionControllerNextSelectableItem(controller, item, reason)
>+    {
>+        let dataGridNode = this.dataGridNodeForSelectionItem(item);
>+        console.assert(dataGridNode, "Missing DataGridNode for selection item.", item);
>+        if (!dataGridNode)
>+            return null;
>+
>+        const skipUnrevealed = true;
>+        const stayWithin = null;
>+        const dontPopulate = true;
>+        while (dataGridNode = dataGridNode.traverseNextNode(skipUnrevealed, stayWithin, dontPopulate)) {
>+            if (dataGridNode.selectable && (!dataGridNode.isPlaceholderNode || reason !== WI.SelectionController.Reason.All))
>+                return this.selectionItemForDataGridNode(dataGridNode);
>+        }
>+
>+        return null;
>+    }
> };
> 
> WI.DataGrid.Event = {
>diff --git a/Source/WebInspectorUI/UserInterface/Views/DataGridNode.js b/Source/WebInspectorUI/UserInterface/Views/DataGridNode.js
>index 640601145c1666ca6826eac4d5a1d1bc7d6969b4..acf8de3cef8cd51aa78e2a1b90531ebb368786c2 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/DataGridNode.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/DataGridNode.js
>@@ -566,18 +566,10 @@ WI.DataGridNode = class DataGridNode extends WI.Object
>         if (!this.dataGrid || !this.selectable || this.selected)
>             return;
> 
>-        let oldSelectedNode = this.dataGrid.selectedNode;
>-        if (oldSelectedNode)
>-            oldSelectedNode.deselect(true);
>-
>         this._selected = true;
>-        this.dataGrid.selectedNode = this;
>+        this._element?.classList.add("selected");
> 
>-        if (this._element)
>-            this._element.classList.add("selected");
>-
>-        if (!suppressSelectedEvent)
>-            this.dataGrid.dispatchEventToListeners(WI.DataGrid.Event.SelectedNodeChanged, {oldSelectedNode});
>+        this.dataGrid.selectDataGridNodeInternal(this, suppressSelectedEvent);
>     }
> 
>     revealAndSelect(suppressSelectedEvent)
>@@ -588,17 +580,13 @@ WI.DataGridNode = class DataGridNode extends WI.Object
> 
>     deselect(suppressDeselectedEvent)
>     {
>-        if (!this.dataGrid || this.dataGrid.selectedNode !== this || !this.selected)
>+        if (!this.dataGrid || !this.selectable || !this.selected)
>             return;
> 
>         this._selected = false;
>-        this.dataGrid.selectedNode = null;
>+        this._element?.classList.remove("selected");
> 
>-        if (this._element)
>-            this._element.classList.remove("selected");
>-
>-        if (!suppressDeselectedEvent)
>-            this.dataGrid.dispatchEventToListeners(WI.DataGrid.Event.SelectedNodeChanged, {oldSelectedNode: this});
>+        this.dataGrid.deselectDataGridNodeInternal(this, suppressDeselectedEvent);
>     }
> 
>     traverseNextNode(skipHidden, stayWithin, dontPopulate, info)
>diff --git a/Source/WebInspectorUI/UserInterface/Views/ProfileView.js b/Source/WebInspectorUI/UserInterface/Views/ProfileView.js
>index a23c5157f61fcacb465f09b8b7fe22da22306391..224d051fee494abafc1c05de64649020b7691505 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/ProfileView.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/ProfileView.js
>@@ -69,6 +69,8 @@ WI.ProfileView = class ProfileView extends WI.ContentView
>         this._dataGrid.sortOrder = WI.DataGrid.SortOrder.Descending;
>         this._dataGrid.createSettings("profile-view");
> 
>+        this._selectedDataGridNode = null;
>+
>         // Currently we create a new ProfileView for each CallingContextTree, so
>         // to share state between them, use a common shared data object.
>         this._sharedData = extraArguments;
>@@ -211,18 +213,19 @@ WI.ProfileView = class ProfileView extends WI.ContentView
> 
>     _dataGridNodeSelected(event)
>     {
>-        let oldSelectedNode = event.data.oldSelectedNode;
>-        if (oldSelectedNode) {
>-            this._removeGuidanceElement(WI.ProfileView.GuidanceType.Selected, oldSelectedNode);
>-            oldSelectedNode.forEachChildInSubtree((node) => this._removeGuidanceElement(WI.ProfileView.GuidanceType.Selected, node));
>+        if (this._selectedDataGridNode) {
>+            this._removeGuidanceElement(WI.ProfileView.GuidanceType.Selected, this._selectedDataGridNode);
>+            this._selectedDataGridNode.forEachChildInSubtree((node) => this._removeGuidanceElement(WI.ProfileView.GuidanceType.Selected, node));
>         }
> 
>-        let newSelectedNode = this._dataGrid.selectedNode;
>-        if (newSelectedNode) {
>-            this._removeGuidanceElement(WI.ProfileView.GuidanceType.Selected, newSelectedNode);
>-            newSelectedNode.forEachChildInSubtree((node) => this._appendGuidanceElement(WI.ProfileView.GuidanceType.Selected, node, newSelectedNode));
>+        this._selectedDataGridNode = this._dataGrid.selectedNode;
> 
>-            this._sharedData.selectedNodeHash = newSelectedNode.callingContextTreeNode.hash;
>+
>+        if (this._selectedDataGridNode) {
>+            this._removeGuidanceElement(WI.ProfileView.GuidanceType.Selected, this._selectedDataGridNode);
>+            this._selectedDataGridNode.forEachChildInSubtree((node) => this._appendGuidanceElement(WI.ProfileView.GuidanceType.Selected, node, this._selectedDataGridNode));
>+
>+            this._sharedData.selectedNodeHash = this._selectedDataGridNode.callingContextTreeNode.hash;
>         }
>     }
> 
>diff --git a/Source/WebInspectorUI/UserInterface/Views/Table.js b/Source/WebInspectorUI/UserInterface/Views/Table.js
>index 8833df73068c7a7346217b900e673f7ab89179b9..28f432629ea4ad87752750618829f9dd0f920d49 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/Table.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/Table.js
>@@ -86,7 +86,8 @@ WI.Table = class Table extends WI.View
>         this._columnWidths = null; // Calculated in _resizeColumnsAndFiller.
>         this._fillerHeight = 0; // Calculated in _resizeColumnsAndFiller.
> 
>-        this._selectionController = new WI.SelectionController(this, (a, b) => this._indexForRepresentedObject(a) - this._indexForRepresentedObject(b));
>+        let selectionComparator = WI.SelectionController.createListComparator(this._indexForRepresentedObject.bind(this));
>+        this._selectionController = new WI.SelectionController(this, selectionComparator);
> 
>         this._resizers = [];
>         this._currentResizer = null;
>diff --git a/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js b/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>index 8120cd15b55369a5cf8236c4aa55b07f8c4c6a14..507a178d54be17c3699338f57bb1c6caa8c4c255 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/TreeOutline.js
>@@ -57,53 +57,9 @@ WI.TreeOutline = class TreeOutline extends WI.Object
> 
>         this._cachedNumberOfDescendants = 0;
> 
>-        let comparator = (a, b) => {
>-            function getLevel(treeElement) {
>-                let level = 0;
>-                while (treeElement = treeElement.parent)
>-                    level++;
>-                return level;
>-            }
>-
>-            function compareSiblings(s, t) {
>-                return s.parent.children.indexOf(s) - s.parent.children.indexOf(t);
>-            }
>-
>-            // Translate represented objects to TreeElements, which have the
>-            // hierarchical information needed to perform the comparison.
>-            a = this.getCachedTreeElement(a);
>-            b = this.getCachedTreeElement(b);
>-            if (!a || !b)
>-                return 0;
>-
>-            if (a.parent === b.parent)
>-                return compareSiblings(a, b);
>-
>-            let aLevel = getLevel(a);
>-            let bLevel = getLevel(b);
>-            while (aLevel > bLevel) {
>-                if (a.parent === b)
>-                    return 1;
>-                a = a.parent;
>-                aLevel--;
>-            }
>-            while (bLevel > aLevel) {
>-                if (b.parent === a)
>-                    return -1;
>-                b = b.parent;
>-                bLevel--;
>-            }
>-
>-            while (a.parent !== b.parent) {
>-                a = a.parent;
>-                b = b.parent;
>-            }
>-
>-            console.assert(a.parent === b.parent, "Missing common ancestor for TreeElements.", a, b);
>-            return compareSiblings(a, b);
>-        };
>-
>-        this._selectionController = new WI.SelectionController(this, comparator);
>+        let itemForRepresentedObject = this.getCachedTreeElement.bind(this);
>+        let selectionComparator = WI.SelectionController.createTreeComparator(itemForRepresentedObject);
>+        this._selectionController = new WI.SelectionController(this, selectionComparator);
> 
>         this._itemWasSelectedByUser = false;
>         this._processingSelectionChange = false;
>@@ -770,11 +726,6 @@ WI.TreeOutline = class TreeOutline extends WI.Object
> 
>     // SelectionController delegate
> 
>-    selectionControllerNumberOfItems(controller)
>-    {
>-        return this._cachedNumberOfDescendants;
>-    }
>-
>     selectionControllerSelectionDidChange(controller, deselectedItems, selectedItems)
>     {
>         this._processingSelectionChange = true;

[Blaze Burg]

Comment on attachment 397299 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=397299&action=review

r=me, nice work!

> Source/WebInspectorUI/ChangeLog:59
> +        Introduce a `WI.SelectionController.Reason` which is used to tell the `_delegate` about why

I think this should be called Operation not reason. It's possible to select new elements in 3 operations: direct selection, all, and range extension.

> Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js:53
> +
> +    static createTreeComparator(itemForRepresentedObject)

This comparator is begging to be tested somehow. At the very least, I'd like a comment to explain the general heuristic, which seems to be to "find the first common ancestor and compare the ordinal of a and b under the common parent".

> Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js:63
> +            let getLevel = (item) => {

Nit: depth?

> Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js:192
> +                const reason = WI.SelectionController.Reason.Extend;

So.. Reason.Extend also covers shrinking the range?

> Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js:224
> +        const reason = WI.SelectionController.Reason.All;

Why no reset()?

> Source/WebInspectorUI/UserInterface/Views/DataGrid.js:71
> +        let selectionComparator = WI.SelectionController.createTreeComparator(itemForRepresentedObject);

This is elegant. Good job.

> Source/WebInspectorUI/UserInterface/Views/DataGrid.js:1419
> +        let collapseKeyIdentifier = isRTL ? "Right" : "Left";

I'm unsure if this is correct. Does Mac system UI reverse the keys for collapsing when in RTL?

> Source/WebInspectorUI/UserInterface/Views/DataGrid.js:1438
> +                        handled = nextSelectedNode ? true : false;

Nit: !!nextSelectedNode (I realise it's moved from earlier)

> Source/WebInspectorUI/UserInterface/Views/DataGrid.js:2051
> +        return dataGridNode;

lol @ this code

[Devin Rousso]

Comment on attachment 397299 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=397299&action=review

>> Source/WebInspectorUI/ChangeLog:59
>> +        Introduce a `WI.SelectionController.Reason` which is used to tell the `_delegate` about why
> 
> I think this should be called Operation not reason. It's possible to select new elements in 3 operations: direct selection, all, and range extension.

Yeah that's a better name :)

>> Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js:53
>> +    static createTreeComparator(itemForRepresentedObject)
> 
> This comparator is begging to be tested somehow. At the very least, I'd like a comment to explain the general heuristic, which seems to be to "find the first common ancestor and compare the ordinal of a and b under the common parent".

This code has just been moved from `WI.TreeOutline` (and from `WI.Table` below) so the logic hasn't changed at all.

I can add some test cases to LayoutTests/inspector/tree-outline/tree-outline-selection.html for the parent/child cases.

>> Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js:192
>> +                const reason = WI.SelectionController.Reason.Extend;
> 
> So.. Reason.Extend also covers shrinking the range?

Yeah.  I consider that to be a kind of "negative" extension.

>> Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js:224
>> +        const reason = WI.SelectionController.Reason.All;
> 
> Why no reset()?

`reset` will clear the list of selected items, meaning that we won't deselect any previously selected items.  This wasn't a problem before as `selectAll` would always select everything, whereas now we ignore `WI.PlaceholderDataGridNode` when `selectAll`-ing.

>> Source/WebInspectorUI/UserInterface/Views/DataGrid.js:1419
>> +        let collapseKeyIdentifier = isRTL ? "Right" : "Left";
> 
> I'm unsure if this is correct. Does Mac system UI reverse the keys for collapsing when in RTL?

Yes, I believe so.  Keep in mind that the UI is also reversed, so the disclosure arrow will point in the other direction :)

>> Source/WebInspectorUI/UserInterface/Views/DataGrid.js:1438
>> +                        handled = nextSelectedNode ? true : false;
> 
> Nit: !!nextSelectedNode (I realise it's moved from earlier)

lol sure :P

>> Source/WebInspectorUI/UserInterface/Views/DataGrid.js:2051
>> +        return dataGridNode;
> 
> lol @ this code

I did this just in case we make a change in the future.  Plus I think it makes the code easier to read/follow, instead of having to implicitly know that the `WI.DataGridNode` is its own `representedObject`.

[Devin Rousso]

Created attachment 397403 [details]
Patch

[EWS]

Committed r260613: <https://trac.webkit.org/changeset/260613>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 397403 [details].

[Radar WebKit Bug Importer]

<rdar://problem/62273396>