Web Inspector: REGRESSION: Debugger: pressing delete when the all/uncaught exceptions...
authordrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 19 Aug 2019 22:21:21 +0000 (22:21 +0000)
committerdrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 19 Aug 2019 22:21:21 +0000 (22:21 +0000)
https://bugs.webkit.org/show_bug.cgi?id=200876

Reviewed by Joseph Pecoraro.

Removing the `return true;` from the various `WI.TreeElement` breakpoint classes allows the
owner `WI.TreeOutline` to also handle the delete event. In the Debugger/Sources navigation
sidebar, the owner `WI.TreeOutline` checks to see if the currently selected `WI.TreeElement`
is one of the "top" items ("All Exceptions", "Uncaught Exceptions", "Assertion Failures")
and if so, select the next tree element (if able) instead of the previous one.

This is a preferred experience because the "top" items can only be disabled, not deleted, so
trying to delete them wouldn't actually change the selection. They should only ever be
selected if there's nothing else that can be selected.

* UserInterface/Views/DebuggerSidebarPanel.js:
(WI.DebuggerSidebarPanel.prototype._breakpointTreeOutlineDeleteTreeElement):
(WI.DebuggerSidebarPanel.prototype._breakpointTreeOutlineDeleteTreeElement.checkIfSelectionAdjustmentNeeded): Added.
* UserInterface/Views/SourcesNavigationSidebarPanel.js:
(WI.SourcesNavigationSidebarPanel):
(WI.SourcesNavigationSidebarPanel.checkIfSelectionAdjustmentNeeded): Added.

* 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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248873 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 58348e4..38d2239 100644 (file)
@@ -1,5 +1,40 @@
 2019-08-19  Devin Rousso  <drousso@apple.com>
 
+        Web Inspector: REGRESSION: Debugger: pressing delete when the all/uncaught exceptions breakpoint is selected should select the next tree element
+        https://bugs.webkit.org/show_bug.cgi?id=200876
+
+        Reviewed by Joseph Pecoraro.
+
+        Removing the `return true;` from the various `WI.TreeElement` breakpoint classes allows the
+        owner `WI.TreeOutline` to also handle the delete event. In the Debugger/Sources navigation
+        sidebar, the owner `WI.TreeOutline` checks to see if the currently selected `WI.TreeElement`
+        is one of the "top" items ("All Exceptions", "Uncaught Exceptions", "Assertion Failures")
+        and if so, select the next tree element (if able) instead of the previous one.
+
+        This is a preferred experience because the "top" items can only be disabled, not deleted, so
+        trying to delete them wouldn't actually change the selection. They should only ever be
+        selected if there's nothing else that can be selected.
+
+        * UserInterface/Views/DebuggerSidebarPanel.js:
+        (WI.DebuggerSidebarPanel.prototype._breakpointTreeOutlineDeleteTreeElement):
+        (WI.DebuggerSidebarPanel.prototype._breakpointTreeOutlineDeleteTreeElement.checkIfSelectionAdjustmentNeeded): Added.
+        * UserInterface/Views/SourcesNavigationSidebarPanel.js:
+        (WI.SourcesNavigationSidebarPanel):
+        (WI.SourcesNavigationSidebarPanel.checkIfSelectionAdjustmentNeeded): Added.
+
+        * 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):
+
+2019-08-19  Devin Rousso  <drousso@apple.com>
+
         Web Inspector: Debugger: don't show the All Requests breakpoint by default
         https://bugs.webkit.org/show_bug.cgi?id=200892
 
index 2ff7a6b..2e380de 100644 (file)
@@ -82,7 +82,7 @@ WI.BreakpointTreeElement = class BreakpointTreeElement extends WI.GeneralTreeEle
                 InspectorFrontendHost.beep();
             else
                 this._breakpoint.disabled = true;
-            return true;
+            return;
         }
 
         // We set this flag so that TreeOutlines that will remove this
@@ -91,7 +91,6 @@ WI.BreakpointTreeElement = class BreakpointTreeElement extends WI.GeneralTreeEle
         this.__deletedViaDeleteKeyboardShortcut = true;
 
         WI.debuggerManager.removeBreakpoint(this._breakpoint);
-        return true;
     }
 
     onenter()
index 26753d0..e186f70 100644 (file)
@@ -106,7 +106,6 @@ WI.DOMBreakpointTreeElement = class DOMBreakpointTreeElement extends WI.GeneralT
         this.__deletedViaDeleteKeyboardShortcut = true;
 
         WI.domDebuggerManager.removeDOMBreakpoint(this.representedObject);
-        return true;
     }
 
     onenter()
index bf0d979..ab5cf74 100644 (file)
@@ -47,8 +47,6 @@ WI.DOMNodeTreeElement = class DOMNodeTreeElement extends WI.GeneralTreeElement
 
         WI.domDebuggerManager.removeDOMBreakpointsForNode(this.representedObject);
         WI.domManager.removeEventListenerBreakpointsForNode(this.representedObject);
-
-        return true;
     }
 
     populateContextMenu(contextMenu, event)
index 8e077ae..3a04138 100644 (file)
@@ -978,29 +978,43 @@ WI.DebuggerSidebarPanel = class DebuggerSidebarPanel extends WI.NavigationSideba
             breakpoints[i].disabled = disabled;
     }
 
-    _breakpointTreeOutlineDeleteTreeElement(treeElement)
+    _breakpointTreeOutlineDeleteTreeElement(selectedTreeElement)
     {
-        console.assert(treeElement.selected);
+        console.assert(selectedTreeElement.selected);
 
-        if (treeElement.representedObject === DebuggerSidebarPanel.__windowEventTargetRepresentedObject) {
-            let eventBreakpointsOnWindow = WI.domManager.eventListenerBreakpoints.filter((eventBreakpoint) => eventBreakpoint.eventListener.onWindow);
-            for (let eventBreakpoint of eventBreakpointsOnWindow)
-                WI.domManager.removeBreakpointForEventListener(eventBreakpoint.eventListener);
-            return true;
+        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);
 
-        console.assert(treeElement instanceof WI.ResourceTreeElement || treeElement instanceof WI.ScriptTreeElement);
-        if (!(treeElement instanceof WI.ResourceTreeElement) && !(treeElement instanceof WI.ScriptTreeElement))
-            return false;
+        if (selectedTreeElement instanceof WI.ResourceTreeElement || selectedTreeElement instanceof WI.ScriptTreeElement) {
+            checkIfSelectionAdjustmentNeeded(selectedTreeElement.previousSibling);
 
-        var wasTopResourceTreeElement = treeElement.previousSibling === this._assertionsBreakpointTreeElement || treeElement.previousSibling === this._allUncaughtExceptionsBreakpointTreeElement;
-        var nextSibling = treeElement.nextSibling;
+            let breakpoints = this._breakpointsBeneathTreeElement(selectedTreeElement);
+            this._removeAllBreakpoints(breakpoints);
+        } else if (selectedTreeElement.representedObject === DebuggerSidebarPanel.__windowEventTargetRepresentedObject) {
+            checkIfSelectionAdjustmentNeeded(selectedTreeElement.previousSibling);
 
-        var breakpoints = this._breakpointsBeneathTreeElement(treeElement);
-        this._removeAllBreakpoints(breakpoints);
+            let eventBreakpointsOnWindow = WI.domManager.eventListenerBreakpoints.filter((eventBreakpoint) => eventBreakpoint.eventListener.onWindow);
+            for (let eventBreakpoint of eventBreakpointsOnWindow)
+                WI.domManager.removeBreakpointForEventListener(eventBreakpoint.eventListener);
+        }
 
-        if (wasTopResourceTreeElement && nextSibling)
-            nextSibling.select(true, true);
+        if (treeElementToSelect) {
+            const omitFocus = true;
+            const selectedByUser = true;
+            treeElementToSelect.select(omitFocus, selectedByUser);
+        }
 
         return true;
     }
index 726803d..c8555b0 100644 (file)
@@ -92,7 +92,6 @@ WI.EventBreakpointTreeElement = class EventBreakpointTreeElement extends WI.Gene
             WI.domManager.removeBreakpointForEventListener(this.representedObject.eventListener);
         else
             WI.domDebuggerManager.removeEventBreakpoint(this.representedObject);
-        return true;
     }
 
     onenter()
index b4cd1bd..0ad26a4 100644 (file)
@@ -138,30 +138,41 @@ WI.SourcesNavigationSidebarPanel = class SourcesNavigationSidebarPanel extends W
         this._breakpointsTreeOutline.addEventListener(WI.TreeOutline.Event.ElementAdded, this._handleBreakpointElementAddedOrRemoved, this);
         this._breakpointsTreeOutline.addEventListener(WI.TreeOutline.Event.ElementRemoved, this._handleBreakpointElementAddedOrRemoved, this);
         this._breakpointsTreeOutline.addEventListener(WI.TreeOutline.Event.SelectionDidChange, this._handleTreeSelectionDidChange, this);
-        this._breakpointsTreeOutline.ondelete = (treeElement) => {
-            console.assert(treeElement.selected);
+        this._breakpointsTreeOutline.ondelete = (selectedTreeElement) => {
+            console.assert(selectedTreeElement.selected);
 
-            if (treeElement.representedObject === SourcesNavigationSidebarPanel.__windowEventTargetRepresentedObject) {
-                let eventBreakpointsOnWindow = WI.domManager.eventListenerBreakpoints.filter((eventBreakpoint) => eventBreakpoint.eventListener.onWindow);
-                for (let eventBreakpoint of eventBreakpointsOnWindow)
-                    WI.domManager.removeBreakpointForEventListener(eventBreakpoint.eventListener);
-                return true;
+            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);
 
-            console.assert(treeElement instanceof WI.ResourceTreeElement || treeElement instanceof WI.ScriptTreeElement);
-            if (!(treeElement instanceof WI.ResourceTreeElement) && !(treeElement instanceof WI.ScriptTreeElement))
-                return false;
+            if (selectedTreeElement instanceof WI.ResourceTreeElement || selectedTreeElement instanceof WI.ScriptTreeElement) {
+                checkIfSelectionAdjustmentNeeded(selectedTreeElement.previousSibling);
 
-            let wasTopResourceTreeElement = treeElement.previousSibling === this._assertionsBreakpointTreeElement || treeElement.previousSibling === this._allUncaughtExceptionsBreakpointTreeElement;
-            let nextSibling = treeElement.nextSibling;
+                let breakpoints = this._breakpointsBeneathTreeElement(selectedTreeElement);
+                this._removeAllBreakpoints(breakpoints);
+            } else if (selectedTreeElement.representedObject === SourcesNavigationSidebarPanel.__windowEventTargetRepresentedObject) {
+                checkIfSelectionAdjustmentNeeded(selectedTreeElement.previousSibling);
 
-            let breakpoints = this._breakpointsBeneathTreeElement(treeElement);
-            this._removeAllBreakpoints(breakpoints);
+                let eventBreakpointsOnWindow = WI.domManager.eventListenerBreakpoints.filter((eventBreakpoint) => eventBreakpoint.eventListener.onWindow);
+                for (let eventBreakpoint of eventBreakpointsOnWindow)
+                    WI.domManager.removeBreakpointForEventListener(eventBreakpoint.eventListener);
+            }
 
-            if (wasTopResourceTreeElement && nextSibling) {
+            if (treeElementToSelect) {
                 const omitFocus = true;
                 const selectedByUser = true;
-                nextSibling.select(omitFocus, selectedByUser);
+                treeElementToSelect.select(omitFocus, selectedByUser);
             }
 
             return true;
index 0e5455c..1904b01 100644 (file)
@@ -94,7 +94,6 @@ WI.URLBreakpointTreeElement = class URLBreakpointTreeElement extends WI.GeneralT
         this.__deletedViaDeleteKeyboardShortcut = true;
 
         WI.domDebuggerManager.removeURLBreakpoint(this.representedObject);
-        return true;
     }
 
     onenter()