Attachment 357249 Details for Bug 192116 – Patch for landing

[patch] Patch for landing

bug-192116-20181213135021.patch (text/plain), 7.39 KB, created by Matt Baker on 2018-12-13 13:50:39 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Matt Baker

Created: 2018-12-13 13:50:39 PST

Size: 7.39 KB

patch

obsolete

>Subversion Revision: 239131
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index ccf53915c347b28fa898459becb0bf5d61c9e990..f4cc25b9071f50e7b2454842c146fdccecf836ef 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,30 @@
>+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-12  Matt Baker  <mattbaker@apple.com>
> 
>         Web Inspector: Table selection becomes corrupted when deleting selected cookies
>diff --git a/Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js b/Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js
>index d1e9f31a73d6823a5da8cada2e7e640f912af7ce..7066a3dfbf3769030a911bbecb5613e01045f421 100644
>--- a/Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js
>+++ b/Source/WebInspectorUI/UserInterface/Controllers/SelectionController.js
>@@ -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;
>             }
>         }
> 
>diff --git a/Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js b/Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js
>index 942b0c7eca9bc4b044492dd451272c798c9b9f33..d5d0fafe5e5fcda3ba94eaad3df385049e2c60fe 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js
>@@ -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);
>diff --git a/Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.js b/Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.js
>index b3784345d6b3bd81637e64ea0d42748939de4b0c..0dc324afb9a9c01d974ae131b5eca602daee9293 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.js
>@@ -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)

Actions: View | Formatted Diff | Diff

Attachments on bug 192116: 357019 | 357212 | 357248 | 357249