Web Inspector: REGRESSION (r248873): Debugger: pressing delete on a breakpoint will...
authordrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Aug 2019 20:50:24 +0000 (20:50 +0000)
committerdrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Aug 2019 20:50:24 +0000 (20:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=200939

Reviewed by Joseph Pecoraro.

* UserInterface/Views/DebuggerSidebarPanel.js:
(WI.DebuggerSidebarPanel.prototype._breakpointTreeOutlineDeleteTreeElement):
(WI.DebuggerSidebarPanel.prototype._handleBreakpointElementAddedOrRemoved):
(WI.DebuggerSidebarPanel.prototype._breakpointTreeOutlineDeleteTreeElement.checkIfSelectionAdjustmentNeeded): Deleted.
* UserInterface/Views/SourcesNavigationSidebarPanel.js:
(WI.SourcesNavigationSidebarPanel):
(WI.SourcesNavigationSidebarPanel.prototype._handleBreakpointElementAddedOrRemoved):
(WI.SourcesNavigationSidebarPanel.this._breakpointsTreeOutline.ondelete.checkIfSelectionAdjustmentNeeded): Deleted.
When the `WI.TreeOutline`'s own `ondelete` is called, that means we must be handling a
delete that was _not_ handled by a `WI.TreeElement`. This means that the `selectedTreeElement`
has to be a resource/script, the `window` object, or one of the non-deletable breakpoints.

In the case of a non-deletable breakpoint, since they're never removed from their parent
`WI.TreeOutline`, we just shift the selection to the next selectable `WI.TreeElement`.

Otherwise, wait for the `WI.TreeOutline.Event.ElementRemoved` event to be fired, and adjust
the selection then based on whether the new `selectedTreeElement` is one of the "top" items,
namely the "All Exceptions", "Uncaught Exceptions", and "Assertion Failures" breakpoints.

* UserInterface/Views/BreakpointTreeElement.js:
(WI.BreakpointTreeElement.prototype.ondelete):
* UserInterface/Views/DOMBreakpointTreeElement.js:
(WI.DOMBreakpointTreeElement.prototype.ondelete):
* UserInterface/Views/DOMNodeTreeElement.js:
(WI.DOMNodeTreeElement.prototype.ondelete):
* UserInterface/Views/EventBreakpointTreeElement.js:
(WI.EventBreakpointTreeElement.prototype.ondelete):
* UserInterface/Views/URLBreakpointTreeElement.js:
(WI.URLBreakpointTreeElement.prototype.ondelete):
Add `return true;` to let the parent `WI.TreeOutline` know that the delete event was handled.
This prevents the parent `WI.TreeOutline`'s own `ondelete` from being called, which would
cause a double-delete as there would be a different `selectedTreeElement`.

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

Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Views/BreakpointTreeElement.js
Source/WebInspectorUI/UserInterface/Views/DOMBreakpointTreeElement.js
Source/WebInspectorUI/UserInterface/Views/DOMNodeTreeElement.js
Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js
Source/WebInspectorUI/UserInterface/Views/EventBreakpointTreeElement.js
Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js
Source/WebInspectorUI/UserInterface/Views/URLBreakpointTreeElement.js

index 1c0c4f7..e47c889 100644 (file)
@@ -1,3 +1,43 @@
+2019-08-29  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: REGRESSION (r248873): Debugger: pressing delete on a breakpoint will also delete any resource/element parent immediately before it in the list
+        https://bugs.webkit.org/show_bug.cgi?id=200939
+
+        Reviewed by Joseph Pecoraro.
+
+        * UserInterface/Views/DebuggerSidebarPanel.js:
+        (WI.DebuggerSidebarPanel.prototype._breakpointTreeOutlineDeleteTreeElement):
+        (WI.DebuggerSidebarPanel.prototype._handleBreakpointElementAddedOrRemoved):
+        (WI.DebuggerSidebarPanel.prototype._breakpointTreeOutlineDeleteTreeElement.checkIfSelectionAdjustmentNeeded): Deleted.
+        * UserInterface/Views/SourcesNavigationSidebarPanel.js:
+        (WI.SourcesNavigationSidebarPanel):
+        (WI.SourcesNavigationSidebarPanel.prototype._handleBreakpointElementAddedOrRemoved):
+        (WI.SourcesNavigationSidebarPanel.this._breakpointsTreeOutline.ondelete.checkIfSelectionAdjustmentNeeded): Deleted.
+        When the `WI.TreeOutline`'s own `ondelete` is called, that means we must be handling a
+        delete that was _not_ handled by a `WI.TreeElement`. This means that the `selectedTreeElement`
+        has to be a resource/script, the `window` object, or one of the non-deletable breakpoints.
+
+        In the case of a non-deletable breakpoint, since they're never removed from their parent
+        `WI.TreeOutline`, we just shift the selection to the next selectable `WI.TreeElement`.
+
+        Otherwise, wait for the `WI.TreeOutline.Event.ElementRemoved` event to be fired, and adjust
+        the selection then based on whether the new `selectedTreeElement` is one of the "top" items,
+        namely the "All Exceptions", "Uncaught Exceptions", and "Assertion Failures" breakpoints.
+
+        * UserInterface/Views/BreakpointTreeElement.js:
+        (WI.BreakpointTreeElement.prototype.ondelete):
+        * UserInterface/Views/DOMBreakpointTreeElement.js:
+        (WI.DOMBreakpointTreeElement.prototype.ondelete):
+        * UserInterface/Views/DOMNodeTreeElement.js:
+        (WI.DOMNodeTreeElement.prototype.ondelete):
+        * UserInterface/Views/EventBreakpointTreeElement.js:
+        (WI.EventBreakpointTreeElement.prototype.ondelete):
+        * UserInterface/Views/URLBreakpointTreeElement.js:
+        (WI.URLBreakpointTreeElement.prototype.ondelete):
+        Add `return true;` to let the parent `WI.TreeOutline` know that the delete event was handled.
+        This prevents the parent `WI.TreeOutline`'s own `ondelete` from being called, which would
+        cause a double-delete as there would be a different `selectedTreeElement`.
+
 2019-08-29  Keith Rollin  <krollin@apple.com>
 
         Remove support for macOS < 10.13 (part 3)
 2019-08-29  Keith Rollin  <krollin@apple.com>
 
         Remove support for macOS < 10.13 (part 3)
index 2e380de..263f07c 100644 (file)
@@ -77,20 +77,21 @@ WI.BreakpointTreeElement = class BreakpointTreeElement extends WI.GeneralTreeEle
 
     ondelete()
     {
 
     ondelete()
     {
-        if (!WI.debuggerManager.isBreakpointRemovable(this._breakpoint)) {
-            if (this._breakpoint.disabled)
-                InspectorFrontendHost.beep();
-            else
-                this._breakpoint.disabled = true;
-            return;
-        }
-
         // We set this flag so that TreeOutlines that will remove this
         // BreakpointTreeElement will know whether it was deleted from
         // within the TreeOutline or from outside it (e.g. TextEditor).
         this.__deletedViaDeleteKeyboardShortcut = true;
 
         // We set this flag so that TreeOutlines that will remove this
         // BreakpointTreeElement will know whether it was deleted from
         // within the TreeOutline or from outside it (e.g. TextEditor).
         this.__deletedViaDeleteKeyboardShortcut = true;
 
-        WI.debuggerManager.removeBreakpoint(this._breakpoint);
+        if (WI.debuggerManager.isBreakpointRemovable(this._breakpoint)) {
+            WI.debuggerManager.removeBreakpoint(this._breakpoint);
+            return true;
+        }
+
+        if (this._breakpoint.disabled)
+            InspectorFrontendHost.beep();
+        else
+            this._breakpoint.disabled = true;
+        return false;
     }
 
     onenter()
     }
 
     onenter()
index e186f70..ad6bb1a 100644 (file)
@@ -106,6 +106,8 @@ WI.DOMBreakpointTreeElement = class DOMBreakpointTreeElement extends WI.GeneralT
         this.__deletedViaDeleteKeyboardShortcut = true;
 
         WI.domDebuggerManager.removeDOMBreakpoint(this.representedObject);
         this.__deletedViaDeleteKeyboardShortcut = true;
 
         WI.domDebuggerManager.removeDOMBreakpoint(this.representedObject);
+
+        return true;
     }
 
     onenter()
     }
 
     onenter()
index ab5cf74..bf0d979 100644 (file)
@@ -47,6 +47,8 @@ WI.DOMNodeTreeElement = class DOMNodeTreeElement extends WI.GeneralTreeElement
 
         WI.domDebuggerManager.removeDOMBreakpointsForNode(this.representedObject);
         WI.domManager.removeEventListenerBreakpointsForNode(this.representedObject);
 
         WI.domDebuggerManager.removeDOMBreakpointsForNode(this.representedObject);
         WI.domManager.removeEventListenerBreakpointsForNode(this.representedObject);
+
+        return true;
     }
 
     populateContextMenu(contextMenu, event)
     }
 
     populateContextMenu(contextMenu, event)
index 8154685..a4c4859 100644 (file)
@@ -995,40 +995,22 @@ WI.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WI.NavigationSideba
     {
         console.assert(selectedTreeElement.selected);
 
     {
         console.assert(selectedTreeElement.selected);
 
-        let treeElementToSelect = null;
-        function checkIfSelectionAdjustmentNeeded(treeElement) {
-            if (!treeElement)
-                return;
-
-            let representedObjects = [
-                WI.debuggerManager.allExceptionsBreakpoint,
-                WI.debuggerManager.uncaughtExceptionsBreakpoint,
-                WI.debuggerManager.assertionFailuresBreakpoint,
-            ];
-            if (representedObjects.includes(treeElement.representedObject))
-                treeElementToSelect = selectedTreeElement.nextSibling;
-        }
-        checkIfSelectionAdjustmentNeeded(selectedTreeElement);
-
-        if (selectedTreeElement instanceof WI.ResourceTreeElement || selectedTreeElement instanceof WI.ScriptTreeElement) {
-            checkIfSelectionAdjustmentNeeded(selectedTreeElement.previousSibling);
-
+        if (!WI.debuggerManager.isBreakpointRemovable(selectedTreeElement.representedObject)) {
+            let treeElementToSelect = selectedTreeElement.nextSelectableSibling;
+            if (treeElementToSelect) {
+                const omitFocus = true;
+                const selectedByUser = true;
+                treeElementToSelect.select(omitFocus, selectedByUser);
+            }
+        } else if (selectedTreeElement instanceof WI.ResourceTreeElement || selectedTreeElement instanceof WI.ScriptTreeElement) {
             let breakpoints = this._breakpointsBeneathTreeElement(selectedTreeElement);
             this._removeAllBreakpoints(breakpoints);
         } else if (selectedTreeElement.representedObject === DebuggerSidebarPanel.__windowEventTargetRepresentedObject) {
             let breakpoints = this._breakpointsBeneathTreeElement(selectedTreeElement);
             this._removeAllBreakpoints(breakpoints);
         } else if (selectedTreeElement.representedObject === DebuggerSidebarPanel.__windowEventTargetRepresentedObject) {
-            checkIfSelectionAdjustmentNeeded(selectedTreeElement.previousSibling);
-
             let eventBreakpointsOnWindow = WI.domManager.eventListenerBreakpoints.filter((eventBreakpoint) => eventBreakpoint.eventListener.onWindow);
             for (let eventBreakpoint of eventBreakpointsOnWindow)
                 WI.domManager.removeBreakpointForEventListener(eventBreakpoint.eventListener);
         }
 
             let eventBreakpointsOnWindow = WI.domManager.eventListenerBreakpoints.filter((eventBreakpoint) => eventBreakpoint.eventListener.onWindow);
             for (let eventBreakpoint of eventBreakpointsOnWindow)
                 WI.domManager.removeBreakpointForEventListener(eventBreakpoint.eventListener);
         }
 
-        if (treeElementToSelect) {
-            const omitFocus = true;
-            const selectedByUser = true;
-            treeElementToSelect.select(omitFocus, selectedByUser);
-        }
-
         return true;
     }
 
         return true;
     }
 
@@ -1595,6 +1577,21 @@ WI.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WI.NavigationSideba
         }
         if (setting)
             setting.value = !!treeElement.parent;
         }
         if (setting)
             setting.value = !!treeElement.parent;
+
+        if (event.type === WI.TreeOutline.Event.ElementRemoved) {
+            let selectedTreeElement = this._breakpointsContentTreeOutline.selectedTreeElement;
+            console.assert(selectedTreeElement);
+            if (selectedTreeElement.representedObject === WI.debuggerManager.assertionFailuresBreakpoint || !WI.debuggerManager.isBreakpointRemovable(selectedTreeElement.representedObject)) {
+                const skipUnrevealed = true;
+                const dontPopulate = true;
+                let treeElementToSelect = selectedTreeElement.traverseNextTreeElement(skipUnrevealed, dontPopulate);
+                if (treeElementToSelect) {
+                    const omitFocus = true;
+                    const selectedByUser = true;
+                    treeElementToSelect.select(omitFocus, selectedByUser);
+                }
+            }
+        }
     }
 
     _populateCreateBreakpointContextMenu(contextMenu)
     }
 
     _populateCreateBreakpointContextMenu(contextMenu)
index c8555b0..e0bfebf 100644 (file)
@@ -92,6 +92,8 @@ WI.EventBreakpointTreeElement = class EventBreakpointTreeElement extends WI.Gene
             WI.domManager.removeBreakpointForEventListener(this.representedObject.eventListener);
         else
             WI.domDebuggerManager.removeEventBreakpoint(this.representedObject);
             WI.domManager.removeBreakpointForEventListener(this.representedObject.eventListener);
         else
             WI.domDebuggerManager.removeEventBreakpoint(this.representedObject);
+
+        return true;
     }
 
     onenter()
     }
 
     onenter()
index fb60d73..4ba5f52 100644 (file)
@@ -141,40 +141,22 @@ WI.SourcesNavigationSidebarPanel = class SourcesNavigationSidebarPanel extends W
         this._breakpointsTreeOutline.ondelete = (selectedTreeElement) => {
             console.assert(selectedTreeElement.selected);
 
         this._breakpointsTreeOutline.ondelete = (selectedTreeElement) => {
             console.assert(selectedTreeElement.selected);
 
-            let treeElementToSelect = null;
-            function checkIfSelectionAdjustmentNeeded(treeElement) {
-                if (!treeElement)
-                    return;
-
-                let representedObjects = [
-                    WI.debuggerManager.allExceptionsBreakpoint,
-                    WI.debuggerManager.uncaughtExceptionsBreakpoint,
-                    WI.debuggerManager.assertionFailuresBreakpoint,
-                ];
-                if (representedObjects.includes(treeElement.representedObject))
-                    treeElementToSelect = selectedTreeElement.nextSibling;
-            }
-            checkIfSelectionAdjustmentNeeded(selectedTreeElement);
-
-            if (selectedTreeElement instanceof WI.ResourceTreeElement || selectedTreeElement instanceof WI.ScriptTreeElement) {
-                checkIfSelectionAdjustmentNeeded(selectedTreeElement.previousSibling);
-
+            if (!WI.debuggerManager.isBreakpointRemovable(selectedTreeElement.representedObject)) {
+                let treeElementToSelect = selectedTreeElement.nextSelectableSibling;
+                if (treeElementToSelect) {
+                    const omitFocus = true;
+                    const selectedByUser = true;
+                    treeElementToSelect.select(omitFocus, selectedByUser);
+                }
+            } else if (selectedTreeElement instanceof WI.ResourceTreeElement || selectedTreeElement instanceof WI.ScriptTreeElement) {
                 let breakpoints = this._breakpointsBeneathTreeElement(selectedTreeElement);
                 this._removeAllBreakpoints(breakpoints);
             } else if (selectedTreeElement.representedObject === SourcesNavigationSidebarPanel.__windowEventTargetRepresentedObject) {
                 let breakpoints = this._breakpointsBeneathTreeElement(selectedTreeElement);
                 this._removeAllBreakpoints(breakpoints);
             } else if (selectedTreeElement.representedObject === SourcesNavigationSidebarPanel.__windowEventTargetRepresentedObject) {
-                checkIfSelectionAdjustmentNeeded(selectedTreeElement.previousSibling);
-
                 let eventBreakpointsOnWindow = WI.domManager.eventListenerBreakpoints.filter((eventBreakpoint) => eventBreakpoint.eventListener.onWindow);
                 for (let eventBreakpoint of eventBreakpointsOnWindow)
                     WI.domManager.removeBreakpointForEventListener(eventBreakpoint.eventListener);
             }
 
                 let eventBreakpointsOnWindow = WI.domManager.eventListenerBreakpoints.filter((eventBreakpoint) => eventBreakpoint.eventListener.onWindow);
                 for (let eventBreakpoint of eventBreakpointsOnWindow)
                     WI.domManager.removeBreakpointForEventListener(eventBreakpoint.eventListener);
             }
 
-            if (treeElementToSelect) {
-                const omitFocus = true;
-                const selectedByUser = true;
-                treeElementToSelect.select(omitFocus, selectedByUser);
-            }
-
             return true;
         };
         this._breakpointsTreeOutline.populateContextMenu = (contextMenu, event, treeElement) => {
             return true;
         };
         this._breakpointsTreeOutline.populateContextMenu = (contextMenu, event, treeElement) => {
@@ -1631,6 +1613,21 @@ WI.SourcesNavigationSidebarPanel = class SourcesNavigationSidebarPanel extends W
 
         if (setting)
             setting.value = !!treeElement.parent;
 
         if (setting)
             setting.value = !!treeElement.parent;
+
+        if (event.type === WI.TreeOutline.Event.ElementRemoved) {
+            let selectedTreeElement = this._breakpointsTreeOutline.selectedTreeElement;
+            console.assert(selectedTreeElement);
+            if (selectedTreeElement.representedObject === WI.debuggerManager.assertionFailuresBreakpoint || !WI.debuggerManager.isBreakpointRemovable(selectedTreeElement.representedObject)) {
+                const skipUnrevealed = true;
+                const dontPopulate = true;
+                let treeElementToSelect = selectedTreeElement.traverseNextTreeElement(skipUnrevealed, dontPopulate);
+                if (treeElementToSelect) {
+                    const omitFocus = true;
+                    const selectedByUser = true;
+                    treeElementToSelect.select(omitFocus, selectedByUser);
+                }
+            }
+        }
     }
 
     _populateCreateBreakpointContextMenu(contextMenu)
     }
 
     _populateCreateBreakpointContextMenu(contextMenu)
index 1904b01..028a506 100644 (file)
@@ -94,6 +94,8 @@ WI.URLBreakpointTreeElement = class URLBreakpointTreeElement extends WI.GeneralT
         this.__deletedViaDeleteKeyboardShortcut = true;
 
         WI.domDebuggerManager.removeURLBreakpoint(this.representedObject);
         this.__deletedViaDeleteKeyboardShortcut = true;
 
         WI.domDebuggerManager.removeURLBreakpoint(this.representedObject);
+
+        return true;
     }
 
     onenter()
     }
 
     onenter()