Bug 157713 – Web Inspector: unable to switch between navigation tree outlines using up/down arrow keys

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Blaze Burg]

STEPS TO REPRODUCE:

Open the debugger tab
Select "All Uncaught Exceptions"
Press arrow key down

EXPECTED:

The root of the next tree outline "Sources" should become selected, or its first child.

ACTUAL:

Nothing happens.

[Radar WebKit Bug Importer]

<rdar://problem/26287086>

[Matt Baker]

Let's make better use of the tree outline tab order, instead of adding non-standard keyboard navigation behavior. Tab/Shift+Tab currently changes the focus between the two trees, but doesn't update the selection.

We should remember the selection when a tree loses focus, and reselect it when it gains the focus back.

[Matt Baker]

Created attachment 279197 [details]
[Video] New keyboard navigation behavior

[Matt Baker]

Created attachment 279210 [details]
[Patch] Proposed Fix

[Timothy Hatcher]

Comment on attachment 279210 [details]
[Patch] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:159
> +        contentTreeOutline.element.addEventListener("focus", this._contentTreeOutlineDidFocus, this);
> +
> +        // FIXME Remove ContentTreeOutlineSymbol once <https://webkit.org/b/157825> is finished.
> +        contentTreeOutline.element[WebInspector.NavigationSidebarPanel.ContentTreeOutlineSymbol] = contentTreeOutline;

This might be better handled in TreeOutline.js, and add a new focus event.

> Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:678
> +        // Prevent two selections in the sidebar, if the selected tree outline is changing.
> +        let treeOutline = selectedElement.treeOutline;
> +        if (this._selectedContentTreeOutline && this._selectedContentTreeOutline !== treeOutline)
> +            this._selectedContentTreeOutline.selectedTreeElement.deselect();

I thought navigation sidebar panel handled this now?

[Matt Baker]

Comment on attachment 279210 [details]
[Patch] Proposed Fix

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

>> Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:159
>> +        contentTreeOutline.element[WebInspector.NavigationSidebarPanel.ContentTreeOutlineSymbol] = contentTreeOutline;
> 
> This might be better handled in TreeOutline.js, and add a new focus event.

Trees usually wouldn't lose their selection on blur, so I'm not sure this applies to tree outline's in general.

>> Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:678
>> +            this._selectedContentTreeOutline.selectedTreeElement.deselect();
> 
> I thought navigation sidebar panel handled this now?

That's still the case. It's been simplified to use _selectedContentTreeOutline instead of checking every content tree outline.

[WebKit Commit Bot]

Comment on attachment 279210 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 279210

Committed r201125: <http://trac.webkit.org/changeset/201125>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.