Changeset 249291 in webkit


Ignore:
Timestamp:
Aug 29, 2019 1:50:24 PM (5 years ago)
Author:
Devin Rousso
Message:

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.

Location:
trunk/Source/WebInspectorUI
Files:
8 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebInspectorUI/ChangeLog

    r249286 r249291  
     12019-08-29  Devin Rousso  <drousso@apple.com>
     2
     3        Web Inspector: REGRESSION (r248873): Debugger: pressing delete on a breakpoint will also delete any resource/element parent immediately before it in the list
     4        https://bugs.webkit.org/show_bug.cgi?id=200939
     5
     6        Reviewed by Joseph Pecoraro.
     7
     8        * UserInterface/Views/DebuggerSidebarPanel.js:
     9        (WI.DebuggerSidebarPanel.prototype._breakpointTreeOutlineDeleteTreeElement):
     10        (WI.DebuggerSidebarPanel.prototype._handleBreakpointElementAddedOrRemoved):
     11        (WI.DebuggerSidebarPanel.prototype._breakpointTreeOutlineDeleteTreeElement.checkIfSelectionAdjustmentNeeded): Deleted.
     12        * UserInterface/Views/SourcesNavigationSidebarPanel.js:
     13        (WI.SourcesNavigationSidebarPanel):
     14        (WI.SourcesNavigationSidebarPanel.prototype._handleBreakpointElementAddedOrRemoved):
     15        (WI.SourcesNavigationSidebarPanel.this._breakpointsTreeOutline.ondelete.checkIfSelectionAdjustmentNeeded): Deleted.
     16        When the `WI.TreeOutline`'s own `ondelete` is called, that means we must be handling a
     17        delete that was _not_ handled by a `WI.TreeElement`. This means that the `selectedTreeElement`
     18        has to be a resource/script, the `window` object, or one of the non-deletable breakpoints.
     19
     20        In the case of a non-deletable breakpoint, since they're never removed from their parent
     21        `WI.TreeOutline`, we just shift the selection to the next selectable `WI.TreeElement`.
     22
     23        Otherwise, wait for the `WI.TreeOutline.Event.ElementRemoved` event to be fired, and adjust
     24        the selection then based on whether the new `selectedTreeElement` is one of the "top" items,
     25        namely the "All Exceptions", "Uncaught Exceptions", and "Assertion Failures" breakpoints.
     26
     27        * UserInterface/Views/BreakpointTreeElement.js:
     28        (WI.BreakpointTreeElement.prototype.ondelete):
     29        * UserInterface/Views/DOMBreakpointTreeElement.js:
     30        (WI.DOMBreakpointTreeElement.prototype.ondelete):
     31        * UserInterface/Views/DOMNodeTreeElement.js:
     32        (WI.DOMNodeTreeElement.prototype.ondelete):
     33        * UserInterface/Views/EventBreakpointTreeElement.js:
     34        (WI.EventBreakpointTreeElement.prototype.ondelete):
     35        * UserInterface/Views/URLBreakpointTreeElement.js:
     36        (WI.URLBreakpointTreeElement.prototype.ondelete):
     37        Add `return true;` to let the parent `WI.TreeOutline` know that the delete event was handled.
     38        This prevents the parent `WI.TreeOutline`'s own `ondelete` from being called, which would
     39        cause a double-delete as there would be a different `selectedTreeElement`.
     40
    1412019-08-29  Keith Rollin  <krollin@apple.com>
    242
  • trunk/Source/WebInspectorUI/UserInterface/Views/BreakpointTreeElement.js

    r248873 r249291  
    7878    ondelete()
    7979    {
    80         if (!WI.debuggerManager.isBreakpointRemovable(this._breakpoint)) {
    81             if (this._breakpoint.disabled)
    82                 InspectorFrontendHost.beep();
    83             else
    84                 this._breakpoint.disabled = true;
    85             return;
    86         }
    87 
    8880        // We set this flag so that TreeOutlines that will remove this
    8981        // BreakpointTreeElement will know whether it was deleted from
     
    9183        this.__deletedViaDeleteKeyboardShortcut = true;
    9284
    93         WI.debuggerManager.removeBreakpoint(this._breakpoint);
     85        if (WI.debuggerManager.isBreakpointRemovable(this._breakpoint)) {
     86            WI.debuggerManager.removeBreakpoint(this._breakpoint);
     87            return true;
     88        }
     89
     90        if (this._breakpoint.disabled)
     91            InspectorFrontendHost.beep();
     92        else
     93            this._breakpoint.disabled = true;
     94        return false;
    9495    }
    9596
  • trunk/Source/WebInspectorUI/UserInterface/Views/DOMBreakpointTreeElement.js

    r248873 r249291  
    107107
    108108        WI.domDebuggerManager.removeDOMBreakpoint(this.representedObject);
     109
     110        return true;
    109111    }
    110112
  • trunk/Source/WebInspectorUI/UserInterface/Views/DOMNodeTreeElement.js

    r248873 r249291  
    4848        WI.domDebuggerManager.removeDOMBreakpointsForNode(this.representedObject);
    4949        WI.domManager.removeEventListenerBreakpointsForNode(this.representedObject);
     50
     51        return true;
    5052    }
    5153
  • trunk/Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js

    r248894 r249291  
    996996        console.assert(selectedTreeElement.selected);
    997997
    998         let treeElementToSelect = null;
    999         function checkIfSelectionAdjustmentNeeded(treeElement) {
    1000             if (!treeElement)
    1001                 return;
    1002 
    1003             let representedObjects = [
    1004                 WI.debuggerManager.allExceptionsBreakpoint,
    1005                 WI.debuggerManager.uncaughtExceptionsBreakpoint,
    1006                 WI.debuggerManager.assertionFailuresBreakpoint,
    1007             ];
    1008             if (representedObjects.includes(treeElement.representedObject))
    1009                 treeElementToSelect = selectedTreeElement.nextSibling;
    1010         }
    1011         checkIfSelectionAdjustmentNeeded(selectedTreeElement);
    1012 
    1013         if (selectedTreeElement instanceof WI.ResourceTreeElement || selectedTreeElement instanceof WI.ScriptTreeElement) {
    1014             checkIfSelectionAdjustmentNeeded(selectedTreeElement.previousSibling);
    1015 
     998        if (!WI.debuggerManager.isBreakpointRemovable(selectedTreeElement.representedObject)) {
     999            let treeElementToSelect = selectedTreeElement.nextSelectableSibling;
     1000            if (treeElementToSelect) {
     1001                const omitFocus = true;
     1002                const selectedByUser = true;
     1003                treeElementToSelect.select(omitFocus, selectedByUser);
     1004            }
     1005        } else if (selectedTreeElement instanceof WI.ResourceTreeElement || selectedTreeElement instanceof WI.ScriptTreeElement) {
    10161006            let breakpoints = this._breakpointsBeneathTreeElement(selectedTreeElement);
    10171007            this._removeAllBreakpoints(breakpoints);
    10181008        } else if (selectedTreeElement.representedObject === DebuggerSidebarPanel.__windowEventTargetRepresentedObject) {
    1019             checkIfSelectionAdjustmentNeeded(selectedTreeElement.previousSibling);
    1020 
    10211009            let eventBreakpointsOnWindow = WI.domManager.eventListenerBreakpoints.filter((eventBreakpoint) => eventBreakpoint.eventListener.onWindow);
    10221010            for (let eventBreakpoint of eventBreakpointsOnWindow)
    10231011                WI.domManager.removeBreakpointForEventListener(eventBreakpoint.eventListener);
    1024         }
    1025 
    1026         if (treeElementToSelect) {
    1027             const omitFocus = true;
    1028             const selectedByUser = true;
    1029             treeElementToSelect.select(omitFocus, selectedByUser);
    10301012        }
    10311013
     
    15961578        if (setting)
    15971579            setting.value = !!treeElement.parent;
     1580
     1581        if (event.type === WI.TreeOutline.Event.ElementRemoved) {
     1582            let selectedTreeElement = this._breakpointsContentTreeOutline.selectedTreeElement;
     1583            console.assert(selectedTreeElement);
     1584            if (selectedTreeElement.representedObject === WI.debuggerManager.assertionFailuresBreakpoint || !WI.debuggerManager.isBreakpointRemovable(selectedTreeElement.representedObject)) {
     1585                const skipUnrevealed = true;
     1586                const dontPopulate = true;
     1587                let treeElementToSelect = selectedTreeElement.traverseNextTreeElement(skipUnrevealed, dontPopulate);
     1588                if (treeElementToSelect) {
     1589                    const omitFocus = true;
     1590                    const selectedByUser = true;
     1591                    treeElementToSelect.select(omitFocus, selectedByUser);
     1592                }
     1593            }
     1594        }
    15981595    }
    15991596
  • trunk/Source/WebInspectorUI/UserInterface/Views/EventBreakpointTreeElement.js

    r248873 r249291  
    9393        else
    9494            WI.domDebuggerManager.removeEventBreakpoint(this.representedObject);
     95
     96        return true;
    9597    }
    9698
  • trunk/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js

    r248916 r249291  
    142142            console.assert(selectedTreeElement.selected);
    143143
    144             let treeElementToSelect = null;
    145             function checkIfSelectionAdjustmentNeeded(treeElement) {
    146                 if (!treeElement)
    147                     return;
    148 
    149                 let representedObjects = [
    150                     WI.debuggerManager.allExceptionsBreakpoint,
    151                     WI.debuggerManager.uncaughtExceptionsBreakpoint,
    152                     WI.debuggerManager.assertionFailuresBreakpoint,
    153                 ];
    154                 if (representedObjects.includes(treeElement.representedObject))
    155                     treeElementToSelect = selectedTreeElement.nextSibling;
    156             }
    157             checkIfSelectionAdjustmentNeeded(selectedTreeElement);
    158 
    159             if (selectedTreeElement instanceof WI.ResourceTreeElement || selectedTreeElement instanceof WI.ScriptTreeElement) {
    160                 checkIfSelectionAdjustmentNeeded(selectedTreeElement.previousSibling);
    161 
     144            if (!WI.debuggerManager.isBreakpointRemovable(selectedTreeElement.representedObject)) {
     145                let treeElementToSelect = selectedTreeElement.nextSelectableSibling;
     146                if (treeElementToSelect) {
     147                    const omitFocus = true;
     148                    const selectedByUser = true;
     149                    treeElementToSelect.select(omitFocus, selectedByUser);
     150                }
     151            } else if (selectedTreeElement instanceof WI.ResourceTreeElement || selectedTreeElement instanceof WI.ScriptTreeElement) {
    162152                let breakpoints = this._breakpointsBeneathTreeElement(selectedTreeElement);
    163153                this._removeAllBreakpoints(breakpoints);
    164154            } else if (selectedTreeElement.representedObject === SourcesNavigationSidebarPanel.__windowEventTargetRepresentedObject) {
    165                 checkIfSelectionAdjustmentNeeded(selectedTreeElement.previousSibling);
    166 
    167155                let eventBreakpointsOnWindow = WI.domManager.eventListenerBreakpoints.filter((eventBreakpoint) => eventBreakpoint.eventListener.onWindow);
    168156                for (let eventBreakpoint of eventBreakpointsOnWindow)
    169157                    WI.domManager.removeBreakpointForEventListener(eventBreakpoint.eventListener);
    170             }
    171 
    172             if (treeElementToSelect) {
    173                 const omitFocus = true;
    174                 const selectedByUser = true;
    175                 treeElementToSelect.select(omitFocus, selectedByUser);
    176158            }
    177159
     
    16321614        if (setting)
    16331615            setting.value = !!treeElement.parent;
     1616
     1617        if (event.type === WI.TreeOutline.Event.ElementRemoved) {
     1618            let selectedTreeElement = this._breakpointsTreeOutline.selectedTreeElement;
     1619            console.assert(selectedTreeElement);
     1620            if (selectedTreeElement.representedObject === WI.debuggerManager.assertionFailuresBreakpoint || !WI.debuggerManager.isBreakpointRemovable(selectedTreeElement.representedObject)) {
     1621                const skipUnrevealed = true;
     1622                const dontPopulate = true;
     1623                let treeElementToSelect = selectedTreeElement.traverseNextTreeElement(skipUnrevealed, dontPopulate);
     1624                if (treeElementToSelect) {
     1625                    const omitFocus = true;
     1626                    const selectedByUser = true;
     1627                    treeElementToSelect.select(omitFocus, selectedByUser);
     1628                }
     1629            }
     1630        }
    16341631    }
    16351632
  • trunk/Source/WebInspectorUI/UserInterface/Views/URLBreakpointTreeElement.js

    r248873 r249291  
    9595
    9696        WI.domDebuggerManager.removeURLBreakpoint(this.representedObject);
     97
     98        return true;
    9799    }
    98100
Note: See TracChangeset for help on using the changeset viewer.