[webkit-reviews] review denied: [Bug 198228] Web Inspector: Debugger: sidebar should always reveal active call frame when hitting a breakpoint : [Attachment 370982] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has denied Matt Baker <mattbaker at apple.com>'s
request for review:
Bug 198228: Web Inspector: Debugger: sidebar should always reveal active call
frame when hitting a breakpoint
https://bugs.webkit.org/show_bug.cgi?id=198228

Attachment 370982: Patch

https://bugs.webkit.org/attachment.cgi?id=370982&action=review




--- Comment #7 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 370982
  --> https://bugs.webkit.org/attachment.cgi?id=370982
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=370982&action=review

r-, as this throws an error when I try to switch to the Debugger tab:

TypeError:​ undefined is not an object (evaluating 'detailsSection.element')​
(at DebuggerSidebarPanel.js:​343:​89)​
    ?​ @ DebuggerSidebarPanel.js:​343:​89
    find @ [native code]​
    treeElementRevealed @ DebuggerSidebarPanel.js:​343:​54
    didRevealTreeElement @ TreeOutline.js:​900:​45
    reveal @ TreeElement.js:​493:​50
    revealAndSelect @ TreeElement.js:​535:​20
    _revealAndSelectRepresentedObject @
ContentBrowserTabContentView.js:​319:​40
    _contentBrowserCurrentContentViewDidChange @
ContentBrowserTabContentView.js:​301:​47
    dispatch @ Object.js:​165:​30
    dispatchEventToListeners @ Object.js:​172:​17
    _currentContentViewDidChange @ ContentBrowser.js:​520:​38
    dispatch @ Object.js:​165:​30
    dispatchEventToListeners @ Object.js:​172:​17
    showBackForwardEntryForIndex @ ContentViewContainer.js:​170:​38
    showContentView @ ContentViewContainer.js:​142:​42
    showDefaultContentViewForTreeElement @ NavigationSidebarPanel.js:​202:​82
    _checkElementsForPendingViewStateCookie @
NavigationSidebarPanel.js:​724:​79
    _checkOutlinesForPendingViewStateCookie @
NavigationSidebarPanel.js:​680:​53
    restoreStateFromCookie @ NavigationSidebarPanel.js:​245:​53
    restoreStateFromCookie @ DebuggerSidebarPanel.js:​422:​41
    restoreStateFromCookie @ TabContentView.js:​146:​63
    shown @ TabContentView.js:​125:​40
    shown @ ContentBrowserTabContentView.js:​106:​20
    prepareToShow @ BackForwardEntry.js:​84:​35
    _showEntry @ ContentViewContainer.js:​450:​28
    showBackForwardEntryForIndex @ ContentViewContainer.js:​166:​28
    showContentView @ ContentViewContainer.js:​142:​42
    _tabBarItemSelected @ TabBrowser.js:​238:​55
    dispatch @ Object.js:​165:​30
    dispatchEventToListeners @ Object.js:​172:​17
    selectedTabBarItem @ LegacyTabBar.js:​386:​38
    _handleMouseDown @ LegacyTabBar.js:​635:​13
    _handleMouseDown @ [native code]​

> Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:334
> +	   let detailsSections = [this._pauseReasonSection,
this._callStackSection, this._breakpointsSection, this._scriptsSection];

You should move the instantiation of this inside `treeElementRevealed`, since
they could be null when `createContentTreeOutline` is called (it's called in
the constructor of the superclass `WI.NavigationSidebarPanel`).

> Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:336
> +	   function treeElementRevealed(treeElement)

Style: this could be inlined.