Bug 132202

Summary: Web Inspector: clicking Timelines tree view nodes should not change the current content view
Product: WebKit Reporter: Brian Burg <burg>
Component: Web InspectorAssignee: Matt Baker <mattbaker>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, graouts, joepeck, mattbaker, simon.fraser, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 139727    
Attachments:
Description Flags
[Patch] Proposed Fix
none
[Patch] Proposed Fix none

Description Brian Burg 2014-04-25 12:43:54 PDT
It's exceptionally hard to select/expand/contract the tree view disclosures. If the user wants to use the keyboard arrows, or mis-clicks on the disclosure triangle, then they are whisked away to a different content view. I've gotten really frustrated by this when trying to expand deep trees.

I think that behavior could be moved to command/ctrl + click. Some of the rows also have go-to arrows. We could add more of those if it's hard to view resources otherwise.
Comment 1 Radar WebKit Bug Importer 2014-04-25 12:44:13 PDT
<rdar://problem/16728603>
Comment 2 Timothy Hatcher 2014-04-25 12:59:55 PDT
FWIW, clicking a row in the content view works, and keyboard navigation (expand and contract too) from there works.

I'm not opposed to doing something different here. Maybe go-to arrows is right.
Comment 3 Brian Burg 2014-04-25 13:03:09 PDT
(In reply to comment #2)
> FWIW, clicking a row in the content view works, and keyboard navigation (expand and contract too) from there works.

Yeah, I figured it out eventually. But if you are staring at the text in the sidebar with the intent of exploring subtees (by looking at where the disclosures are), it's a rude surprise.

> I'm not opposed to doing something different here. Maybe go-to arrows is right.

Another thing I raised with Joe on IRC is that this scheme would let us highlight data points for a specific row on the timeline, when the relevant row is selected.
Comment 4 Matt Baker 2015-04-20 17:35:19 PDT
Created attachment 251206 [details]
[Patch] Proposed Fix
Comment 5 Brian Burg 2015-04-21 20:44:56 PDT
Comment on attachment 251206 [details]
[Patch] Proposed Fix

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

Nice work!

> Source/WebInspectorUI/UserInterface/Views/LayoutTimelineView.js:35
> +    var columns = {eventType: {}, location: {}, width: {}, height: {}, startTime: {}, totalTime: {}};

Was this actually functionally broken, or just not matching column names of other TimelineView subclasses?

> Source/WebInspectorUI/UserInterface/Views/NetworkTimelineView.js:158
> +        console.error("Unknown tree element selected.");

Please log treeElement as a second parameter. It eliminates a debugging step that you would always have to do to investigate this error message.

> Source/WebInspectorUI/UserInterface/Views/TimelineView.js:191
> +            console.error("Unknown tree element selected.");

As above.

> Source/WebInspectorUI/UserInterface/Views/TimelineView.js:249
> +        if (this.navigationSidebarTreeOutline.selectedTreeElement)

The existing logic is fine for this patch. I related an existing bug that proposes improving the logic.
Comment 6 Matt Baker 2015-04-22 12:45:15 PDT
Comment on attachment 251206 [details]
[Patch] Proposed Fix

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

>> Source/WebInspectorUI/UserInterface/Views/LayoutTimelineView.js:35
>> +    var columns = {eventType: {}, location: {}, width: {}, height: {}, startTime: {}, totalTime: {}};
> 
> Was this actually functionally broken, or just not matching column names of other TimelineView subclasses?

I broke this in https://trac.webkit.org/changeset/182660.
Comment 7 Matt Baker 2015-04-22 12:50:15 PDT
Created attachment 251357 [details]
[Patch] Proposed Fix
Comment 8 WebKit Commit Bot 2015-04-22 14:13:50 PDT
Comment on attachment 251357 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 251357

Committed r183134: <http://trac.webkit.org/changeset/183134>
Comment 9 WebKit Commit Bot 2015-04-22 14:13:54 PDT
All reviewed patches have been landed.  Closing bug.