Bug 178071

Summary: Web Inspector: Network Tab - Filter resources based on URL / Text Content
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: agomez, bburg, buildbot, inspector-bugzilla-changes, joepeck, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[IMAGE] Filter placeholder
none
[IMAGE] Filter active
none
[IMAGE] Filter no results
none
[IMAGE] Filter text content example
none
[PATCH] Proposed Fix
bburg: review+, bburg: commit-queue-
[IMAGE] Filter no results - updated
none
[PATCH] Proposed Fix none

Joseph Pecoraro
Reported 2017-10-08 19:46:58 PDT
Network Tab - Filter resources based on URL / Text Content Should be able to filter resources in the table based on their name or contents. Examples scenarios: • filter only those in a particular domain • filter to the resources that have "id: 12345" in their content • filter to all "min.js" resources • filter to the resources with "?t" in their URL
Attachments
[IMAGE] Filter placeholder (195.74 KB, image/png)
2017-10-08 19:54 PDT, Joseph Pecoraro
no flags
[IMAGE] Filter active (128.88 KB, image/png)
2017-10-08 19:54 PDT, Joseph Pecoraro
no flags
[IMAGE] Filter no results (89.69 KB, image/png)
2017-10-08 19:54 PDT, Joseph Pecoraro
no flags
[IMAGE] Filter text content example (226.24 KB, image/png)
2017-10-08 19:55 PDT, Joseph Pecoraro
no flags
[PATCH] Proposed Fix (30.98 KB, patch)
2017-10-08 19:55 PDT, Joseph Pecoraro
bburg: review+
bburg: commit-queue-
[IMAGE] Filter no results - updated (89.55 KB, image/png)
2017-10-09 13:22 PDT, Joseph Pecoraro
no flags
[PATCH] Proposed Fix (31.57 KB, patch)
2017-10-09 13:22 PDT, Joseph Pecoraro
no flags
Joseph Pecoraro
Comment 1 2017-10-08 19:47:29 PDT
Joseph Pecoraro
Comment 2 2017-10-08 19:54:31 PDT
Created attachment 323152 [details] [IMAGE] Filter placeholder
Joseph Pecoraro
Comment 3 2017-10-08 19:54:43 PDT
Created attachment 323153 [details] [IMAGE] Filter active
Joseph Pecoraro
Comment 4 2017-10-08 19:54:56 PDT
Created attachment 323154 [details] [IMAGE] Filter no results
Joseph Pecoraro
Comment 5 2017-10-08 19:55:13 PDT
Created attachment 323155 [details] [IMAGE] Filter text content example
Joseph Pecoraro
Comment 6 2017-10-08 19:55:36 PDT
Created attachment 323156 [details] [PATCH] Proposed Fix
Blaze Burg
Comment 7 2017-10-09 10:17:32 PDT
Comment on attachment 323156 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=323156&action=review r=me but please try EWS again. I have a few questions. > Source/WebInspectorUI/ChangeLog:46 > + warning in the Debugger tab when breakpoints are disabled. Nice touch. > Source/WebInspectorUI/UserInterface/Controllers/FrameResourceManager.js:68 > + resourceForIdentifier(requestIdentifier) Nit: resourceForRequestIdentifier? > Source/WebInspectorUI/UserInterface/Views/FilterBar.js:66 > + return this._inputField.placeholder; Nice cleanup. > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.css:99 > + right: 0; Please extract 0 to var(content-view-warning-banner-offset-start). Do we use warning banner inside other content views? This could be a generic rule + ContentView feature. > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:80 > + this._textFilterNavigationItem.filterBar.placeholder = WI.UIString("Filter Full URL and Text"); This string seems a little weird to me, but I can't come up with an obviously better replacement. > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:655 > + PageAgent.searchInResource(frame.id, resource.url, searchQuery, isCaseSensitive, isRegex, resource.requestIdentifier, (error, searchResults) => { Oh dear, so many parameters x_x > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:657 > + return; What does this early return do? > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:660 > + return; Please console.error() or WI.reportInternalError() so we can catch bugs here. > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:787 > + _positionEmptyFilterMessage() I like how this is its own function. > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1036 > + this._textFilterNavigationItem.filterBar.inputField.value = null; // Get the placeholder to show again. :| > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1090 > + this._hideResourceDetailView(); Did you try this out? I'm not sure which behavior is less annoying, just reading this. > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1113 > + // For those we should provide more custom filters. Is this something the content view does or the Table/delegate/datasource? > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1133 > + if (!error) { Ditto about logging errors. This could be a console.error and early return. > Source/WebInspectorUI/UserInterface/Views/ScopeBar.js:92 > + resetToDefault() I really wish UI tests were possible yet!
Joseph Pecoraro
Comment 8 2017-10-09 13:11:18 PDT
Comment on attachment 323156 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=323156&action=review >> Source/WebInspectorUI/ChangeLog:46 >> + warning in the Debugger tab when breakpoints are disabled. > > Nice touch. New patch will have a new appearance for this. >> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:657 >> + return; > > What does this early return do? This is if a new search was performed before this async result returned. >> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:660 >> + return; > > Please console.error() or WI.reportInternalError() so we can catch bugs here. This can happen. I know of 1 case where we error (main resource on reloads...) and not having search results just means no results were found. >> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1036 >> + this._textFilterNavigationItem.filterBar.inputField.value = null; // Get the placeholder to show again. > > :| I'm actually going to move this into FilterBar so that FilterBar can reset a little bit of its state. >> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1090 >> + this._hideResourceDetailView(); > > Did you try this out? I'm not sure which behavior is less annoying, just reading this. When users are showing a detail view, changing filters in general doesn't make much sense so I don't expect users to hit this much in practice. But in any case I don't want us to be showing Detail views for a resource that is not selected (filtered out). This is a simple solution. >> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:1113 >> + // For those we should provide more custom filters. > > Is this something the content view does or the Table/delegate/datasource? All filtering happens in NetworkTableContentView. Changing a filter modifies the dataSource data and asks the table to reload.
Joseph Pecoraro
Comment 9 2017-10-09 13:22:43 PDT
Created attachment 323208 [details] [IMAGE] Filter no results - updated
Joseph Pecoraro
Comment 10 2017-10-09 13:22:56 PDT
Created attachment 323209 [details] [PATCH] Proposed Fix
Joseph Pecoraro
Comment 11 2017-10-09 13:24:50 PDT
Comment on attachment 323209 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=323209&action=review > Source/WebInspectorUI/UserInterface/Views/FilterBar.js:124 > + clear() > + { > + this._inputField.value = ""; > + this._inputField.value = null; // Get the placeholder to show again. > + this._lastFilterValue = this.filters; > + } This was a small change. Reseting lastFilterValue means if you search "asdf", Clear Filters, and search "asdf" again that it will work. Previously it wasn't. > Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.css:79 > +.content-view.network .empty-content-placeholder { And this is the style change for No Filter Results no longer being a banner.
Joseph Pecoraro
Comment 12 2017-10-09 13:38:34 PDT
Note You need to log in before you can comment on or make changes to this bug.