Bug 135740

Summary: Web Inspector: Timeline Filter Bars are not appearing at all
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: burg, commit-queue, graouts, joepeck, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix
none
[PATCH] Proposed Fix none

Description Joseph Pecoraro 2014-08-07 20:50:34 PDT
* SUMMARY
Timelines used to have filter bars, and they aren't showing at all. The code is still there, looks like an accident.

* STEPS TO REPRODUCE
1. Inspect apple.com
2. Reload
3. Go to Network Timeline
  => should be able to filter just images
Comment 1 Radar WebKit Bug Importer 2014-08-07 20:50:47 PDT
<rdar://problem/17955461>
Comment 2 Joseph Pecoraro 2014-08-07 20:53:09 PDT
Created attachment 236260 [details]
[PATCH] Proposed Fix
Comment 3 Joseph Pecoraro 2014-08-07 20:53:48 PDT
Comment on attachment 236260 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/TimelineDataGrid.js:38
> -        var scopeBar = columns["scopeBar"];
> +        var scopeBar = column.scopeBar;

This is the actual fix. At some point "column" accidentally became "columns".
Comment 4 Brian Burg 2014-08-07 22:01:40 PDT
Comment on attachment 236260 [details]
[PATCH] Proposed Fix

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

>> Source/WebInspectorUI/UserInterface/Views/TimelineDataGrid.js:38
>> +        var scopeBar = column.scopeBar;
> 
> This is the actual fix. At some point "column" accidentally became "columns".

Mea culpa, probably happened when column data went from being a map back to being an object. In lieu of UI tests, it would be nice to have a checklist of major functionality to check for each content view.

> Source/WebInspectorUI/UserInterface/Views/TimelineDataGrid.js:42
> +        scopeBar.columnIdentifier = identifier;

ouch.
Comment 5 Brian Burg 2014-08-07 22:04:44 PDT
Comment on attachment 236260 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/ScopeBar.js:67
> +        for (var item of this._items) {

Or, go FP and do:

var defaultItem = this._defaultItem;
return this._items.some(function(item) { return item.selected && item !== defaultItem; })

> Source/WebInspectorUI/UserInterface/Views/TimelineDataGrid.js:364
> +                this.element.classList.toggle(WebInspector.TimelineDataGrid.HasNonDefaultFilterStyleClassName, scopeBar.hasNonDefaultItemSelected());

Mmm, toggle. I like toggle.
Comment 6 Joseph Pecoraro 2014-08-07 22:15:35 PDT
Created attachment 236266 [details]
[PATCH] Proposed Fix

I agree with the Array.prototype.some(callback, [thisObject]) suggestion. I wanted to do that.
Comment 7 WebKit Commit Bot 2014-08-07 23:56:46 PDT
Comment on attachment 236266 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 236266

Committed r172334: <http://trac.webkit.org/changeset/172334>
Comment 8 WebKit Commit Bot 2014-08-07 23:56:48 PDT
All reviewed patches have been landed.  Closing bug.