Web Inspector: REGRESSION(r238602): Elements: deleting multiple DOM nodes doesn't...
authormattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Dec 2018 22:29:07 +0000 (22:29 +0000)
committermattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Dec 2018 22:29:07 +0000 (22:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192116
<rdar://problem/46344339>

Reviewed by Devin Rousso.

* UserInterface/Controllers/SelectionController.js:
(WI.SelectionController.prototype.removeSelectedItems):
Finding a new index to select should go through the delegate instead of
naively advancing the index.

* UserInterface/Views/DOMTreeElement.js:
(WI.DOMTreeElement.prototype._populateNodeContextMenu):
(WI.DOMTreeElement.prototype.ondelete): Deleted.
The menu item for removing the DOM node is now managed by the parent
DOMTreeOutline, since its UI and behavior now depend on whether there
are multiple elements selected.

* UserInterface/Views/DOMTreeOutline.js:
(WI.DOMTreeOutline.prototype.populateContextMenu):
(WI.DOMTreeOutline.prototype.ondelete.level):
(WI.DOMTreeOutline.prototype.ondelete):
Implement `ondelete` to remove selected DOM nodes using the delete and
backspace keys. Also used by the DOMTreeOutline's context menu handler.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239178 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js
Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js
Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.js

index dda4c8a..a56b368 100644 (file)
@@ -1,5 +1,32 @@
 2018-12-13  Matt Baker  <mattbaker@apple.com>
 
+        Web Inspector: REGRESSION(r238602): Elements: deleting multiple DOM nodes doesn't select the nearest node after deletion
+        https://bugs.webkit.org/show_bug.cgi?id=192116
+        <rdar://problem/46344339>
+
+        Reviewed by Devin Rousso.
+
+        * UserInterface/Controllers/SelectionController.js:
+        (WI.SelectionController.prototype.removeSelectedItems):
+        Finding a new index to select should go through the delegate instead of
+        naively advancing the index.
+
+        * UserInterface/Views/DOMTreeElement.js:
+        (WI.DOMTreeElement.prototype._populateNodeContextMenu):
+        (WI.DOMTreeElement.prototype.ondelete): Deleted.
+        The menu item for removing the DOM node is now managed by the parent
+        DOMTreeOutline, since its UI and behavior now depend on whether there
+        are multiple elements selected.
+
+        * UserInterface/Views/DOMTreeOutline.js:
+        (WI.DOMTreeOutline.prototype.populateContextMenu):
+        (WI.DOMTreeOutline.prototype.ondelete.level):
+        (WI.DOMTreeOutline.prototype.ondelete):
+        Implement `ondelete` to remove selected DOM nodes using the delete and
+        backspace keys. Also used by the DOMTreeOutline's context menu handler.
+
+2018-12-13  Matt Baker  <mattbaker@apple.com>
+
         Web Inspector: Table selection becomes corrupted when deleting selected cookies
         https://bugs.webkit.org/show_bug.cgi?id=192388
         <rdar://problem/46472364>
index d1e9f31..7066a3d 100644 (file)
@@ -177,19 +177,19 @@ WI.SelectionController = class SelectionController extends WI.Object
 
         // Try selecting the item following the selection.
         let lastSelectedIndex = this._selectedIndexes.lastIndex;
-        let indexToSelect = lastSelectedIndex + 1;
-        if (indexToSelect === this.numberOfItems) {
+        let indexToSelect = this._nextSelectableIndex(lastSelectedIndex);
+        if (isNaN(indexToSelect)) {
             // If no item exists after the last item in the selection, try selecting
             // a deselected item (hole) within the selection.
             let firstSelectedIndex = this._selectedIndexes.firstIndex;
             if (lastSelectedIndex - firstSelectedIndex > numberOfSelectedItems) {
-                indexToSelect = this._selectedIndexes.firstIndex + 1;
+                indexToSelect = this._nextSelectableIndex(firstSelectedIndex);
                 while (this._selectedIndexes.has(indexToSelect))
-                    indexToSelect++;
+                    indexToSelect = this._nextSelectableIndex(firstSelectedIndex);
             } else {
                 // If the selection contains no holes, try selecting the item
                 // preceding the selection.
-                indexToSelect = firstSelectedIndex > 0 ? firstSelectedIndex - 1 : NaN;
+                indexToSelect = firstSelectedIndex > 0 ? this._previousSelectableIndex(firstSelectedIndex) : NaN;
             }
         }
 
index 942b0c7..d5d0faf 100644 (file)
@@ -625,19 +625,6 @@ WI.DOMTreeElement = class DOMTreeElement extends WI.TreeElement
         listItemElement.classList.add(WI.DOMTreeElement.HighlightStyleClassName);
     }
 
-    ondelete()
-    {
-        if (!this.editable)
-            return false;
-
-        var startTagTreeElement = this.treeOutline.findTreeElement(this.representedObject);
-        if (startTagTreeElement)
-            startTagTreeElement.remove();
-        else
-            this.remove();
-        return true;
-    }
-
     onenter()
     {
         if (!this.editable)
@@ -808,7 +795,9 @@ WI.DOMTreeElement = class DOMTreeElement extends WI.TreeElement
 
         subMenus.edit.appendItem(WI.UIString("HTML"), this._editAsHTML.bind(this), !this.editable);
         subMenus.copy.appendItem(WI.UIString("HTML"), this._copyHTML.bind(this), node.isPseudoElement());
-        subMenus.delete.appendItem(WI.UIString("Node"), this.remove.bind(this), !this.editable);
+
+        if (!this.selected || this.treeOutline.selectedTreeElements.length === 1)
+            subMenus.delete.appendItem(WI.UIString("Node"), this.remove.bind(this), !this.editable);
 
         for (let subMenu of Object.values(subMenus))
             contextMenu.pushItem(subMenu);
index b378434..0dc324a 100644 (file)
@@ -250,6 +250,9 @@ WI.DOMTreeOutline = class DOMTreeOutline extends WI.TreeOutline
             delete: new WI.ContextSubMenuItem(contextMenu, WI.UIString("Delete")),
         };
 
+        if (treeElement.selected && this.selectedTreeElements.length > 1)
+            subMenus.delete.appendItem(WI.UIString("Nodes"), () => { this.ondelete(); }, !this._editable);
+
         if (tag && treeElement._populateTagContextMenu)
             treeElement._populateTagContextMenu(contextMenu, event, subMenus);
         else if (textNode && treeElement._populateTextContextMenu)
@@ -270,6 +273,46 @@ WI.DOMTreeOutline = class DOMTreeOutline extends WI.TreeOutline
     {
     }
 
+    ondelete()
+    {
+        if (!this._editable)
+            return false;
+
+        let selectedTreeElements = this.selectedTreeElements;
+        this._selectionController.removeSelectedItems();
+
+        let levelMap = new Map;
+
+        function getLevel(treeElement) {
+            let level = levelMap.get(treeElement);
+            if (isNaN(level)) {
+                level = 0;
+                let current = treeElement;
+                while (current = current.parent)
+                    level++;
+                levelMap.set(treeElement, level);
+            }
+            return level;
+        }
+
+        // Sort in descending order by node level. This ensures that child nodes
+        // are removed before their ancestors.
+        selectedTreeElements.sort((a, b) => getLevel(b) - getLevel(a));
+
+        // Track removed elements, since the opening and closing tags for the
+        // same WI.DOMNode can both be selected.
+        let removedTreeElements = new Set;
+
+        for (let treeElement of selectedTreeElements) {
+            if (removedTreeElements.has(treeElement))
+                continue;
+            removedTreeElements.add(treeElement)
+            treeElement.remove();
+        }
+
+        return true;
+    }
+
     // Private
 
     _revealAndSelectNode(node, omitFocus)