Web Inspector: Selecting frame in debugger sidebar doesn't reveal code

• Component: Web Inspector
• Status: RESOLVED FIXED

[Antoine Quint]

Created attachment 230060 [details]
Behaviour in Safari 7.0.3

In WebKit nightlies, when pausing in the debugger, selecting a frame, any frame, in the debugger sidebar doesn't reveal the expected code in the content view. See URL for a test that automatically breaks in the debugger when loaded and attached movies showing the behaviour in Safari 7.0.3 and Nightly r167733 on 10.9.3.

[Antoine Quint]

Created attachment 230061 [details]
Behaviour in WebKit nightly r167733

[Radar WebKit Bug Importer]

<rdar://problem/16709980>

[Timothy Hatcher]

Dupe of <rdar://problem/16491500>.

[Jonathan Wells]

Created attachment 231026 [details]
[PATCH] Proposed Fix: Addition of NavigationSidebar#isElementSelected getter

[Brian Burg]

Comment on attachment 231026 [details]
[PATCH] Proposed Fix: Addition of NavigationSidebar#isElementSelected getter

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

This seems good to me, since we shouldn't assume that the navigation sidebar has only one outline (or that its selection is within the content outline).

I think you need to rebase the patch.

> Source/WebInspectorUI/UserInterface/Base/Main.js:-1838
> -

Are you adding or removing trailing whitespace here?

[Joseph Pecoraro]

Comment on attachment 231026 [details]
[PATCH] Proposed Fix: Addition of NavigationSidebar#isElementSelected getter

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

r=me with some comments

> Source/WebInspectorUI/ChangeLog:8
> +        `WebInspector._revealAndSelectRepresentedObjectInNavigationSidebar` was checking

No need for all the backticks here.

> Source/WebInspectorUI/UserInterface/Base/Main.js:925
> +    if (!selectedSidebarPanel.elementIsSelected)

I think the name "elementIsSelected" is weird. Maybe "hasSelectedElement" or "hasSelection"?

> Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:132
> +    get elementIsSelected() {

Style: Brace on newline.

[Jonathan Wells]

Created attachment 231085 [details]
[PATCH] Proposed Fix: Addition of NavigationSidebar#isElementSelected getter REV 1

[Joseph Pecoraro]

Comment on attachment 231085 [details]
[PATCH] Proposed Fix: Addition of NavigationSidebar#isElementSelected getter REV 1

r=me

[WebKit Commit Bot]

Comment on attachment 231085 [details]
[PATCH] Proposed Fix: Addition of NavigationSidebar#isElementSelected getter REV 1

Clearing flags on attachment: 231085

Committed r168484: <http://trac.webkit.org/changeset/168484>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Timothy Hatcher]

Comment on attachment 231085 [details]
[PATCH] Proposed Fix: Addition of NavigationSidebar#isElementSelected getter REV 1

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

> Source/WebInspectorUI/ChangeLog:10
> +        base getter elementIsSelected to NavigationSidebarPanel and extended it in 

"getter" should be "function" with the change below.

> Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:135
> +    get hasSelectedElement()
> +    {
> +        return !!this._breakpointsContentTreeOutline.selectedTreeElement || !!this._callStackContentTreeOutline.selectedTreeElement;
> +    },

We don't use verb prefixes with getters. This should be a plain function.

> Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:150
> +    get hasSelectedElement()

Ditto.