Bug 45657

Summary: Web Inspector: [Resources panel] Need more visible “at a glance”.
Product: WebKit Reporter: Pavel Feldman <pfeldman>
Component: Web Inspector (Deprecated)Assignee: Pavel Feldman <pfeldman>
Status: RESOLVED FIXED    
Severity: Normal CC: bweinstein, eric, joepeck, keishi, loislo, masterov, pfeldman, pmuellr, rik, sullivan, timothy, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[IMAGE] Inspector today.
none
[IMAGE] Firebug today.
none
[IMAGE] Network panel mock.
none
[IMAGE] Network panel mock (item selected).
none
Network Panel expanded (extra columns)
none
Network Panel collapsed (fewer columns, larger timeline)
none
[PATCH] New panel behind the flag. Screenshots to follow.
none
[IMAGE] List mode, brief.
none
[IMAGE] List mode, detailed.
none
[PATCH] Sorting by timeline + filter + nicer detailed view.
none
[IMAGE] List mode, brief (2).
none
[IMAGE] List mode, detailed (2).
none
[PATCH] Proposed change. timothy: review+

Description Pavel Feldman 2010-09-13 06:03:36 PDT
Need more visible “at a glance”. In the other tools, the waterfall chart view shows the resource URL, whether it was a GET or POST, the response code, the domain, and the size at a glance. HttpWatch also shows color-coded bars for DNS Lookup, Connect, Send, Wait, Recieve, etc. Being able to see all this info without clicking saves me a lot of time. HttpWatch also allows the user to configure which of these columns are shown, which is a very nice option.
Comment 1 Pavel Feldman 2010-09-15 22:25:43 PDT
Created attachment 67769 [details]
[IMAGE] Inspector today.
Comment 2 Pavel Feldman 2010-09-15 22:31:35 PDT
Created attachment 67770 [details]
[IMAGE] Firebug today.

Few things to note here:

1. There is much more information available at a glance
2. Hovering over columns result in immediate rich tooltips:
    - Hovering over URL item (file name) converts it to a full URL decorated as a link (clickable) and shows tooltip with image preview
    - Hovering over Size shows a tooltip with Post and Response sizes with and without headers
    - Hovering over Timeline shows tooltip with time breakdown
    - Status and Domain show default image preview tooltip
3. Expanding a row shows a table with headers, cookies and such
Comment 3 Pavel Feldman 2010-09-15 22:33:36 PDT
I never noticed, but FB also allows toggling between pretty printed and raw headers views. I like it.
Comment 4 Pavel Feldman 2010-09-15 23:05:06 PDT
So I thought about it a bit and here is what I think:

1. We do mix the "network log" aspect with the "resource browsing" aspect in inspector.
2. Firebug's looks is much more favorable for the network-related use cases (IE9 adopted it too btw)
3. We can't migrate to FB's way since we'd like to navigate to resources from various places (such as CSS rules)

Here is a rather provocative and high level suggestion. I did not think for too long, just throwing it at you.
1. Rename "Resources" tab to "Network" tab and mimic Firebug's way there. Everything, including expandable rows and content preview. It is now a 'network log', not a 'browser'
2. Merge existing Resources tab into the Storage tab. Add a 'NETWORK' group there that would host all of the resources. These resources will have Headers, Contents tabs when you select them.
3. Replace Storage groups with the tree view to make it more lightweight

Rationale:
1. We separate "network log" and "resource browsing"
2. We cover both aspects, do not introduce new panels
3. We can add further IDE aspects to this new "Resources" tab.
* We get little things like when resource is loaded off the appcache, we show it under the appcache node, not under the network.
* Need to think about the Resource vs Scripts panel more. Maybe Scripts panel should instead be Resource panel's perspective (Eclipse terms) that arranges the screen for more convenient debugging. That is internal design though.

Timothy, what do you think?
Comment 5 Annie Sullivan 2010-09-16 00:01:32 PDT
I really like the idea of having a tab focused around network and making it awesome. I hadn't actually understood why it was called "resources" before--was used to the idea of inspecting a resource type in its own tab. For example, if I want to look at a script, I go to the "scripts" tab. (Firebug also has "HTML" and "CSS" tabs).
Comment 6 Timothy Hatcher 2010-09-16 14:09:16 PDT
Some high-level comments:

* Resources do not belong in Storage. Or it needs a new name.
* Duplicating the list of resources (requests) is a bad design and that is why we combine them. We use to have duplicate lists in the previous design of the inspector and it was confusing.

I'd be more in favor of combining Scripts and Resources (which you mention as a alt design), since there is already duplication of info there (lists of scripts/resources). Then Resources could be just the network log.

We would need a better design for Scripts that does not have a <select>, but more of a sidebar. But that takes up lots of space.
Comment 7 Timothy Hatcher 2010-09-16 14:11:32 PDT
I would also be fine with combining Scripts and Storage to be an uber list of Resources, which I think would be a good name for the panel. Then have a Network panel that is the old Resources graphs.
Comment 8 Pavel Feldman 2010-09-16 14:32:46 PDT
So there was a confusion in the original proposal. Somehow the Storage -> Resources rename slipped away. Re-iterating:

Phase I.

1. Move files from Resources to Storage panel
2. Nuke old Resources panel
3. Rename Storage panel to Resources panel. This is our new IDE-alike panel.
4. Introduce Network log that is not ugly
As a result, tabs are:
Resources, Elements, Network, Scripts, Timeline, Profiles, Audits, Console

Phase II.

Merge Scripts into Resources so that it becomes a special Resources' mode. Once on a breakpoint, sidebar appears and such. Use popovers and drawers to save screen real estate.
As a result, tabs are:
Resources, Elements, Network, Timeline, Profiles, Audits, Console
Comment 9 Timothy Hatcher 2010-09-16 14:34:40 PDT
Agreed!
Comment 10 Annie Sullivan 2010-09-16 14:48:14 PDT
Sounds great to me except for one thing--at the end of phase 2 we no longer have a tab called "scripts"--how will developers know where to click so that they can set a breakpoint in their javascript?
Comment 11 Pavel Feldman 2010-09-24 09:32:02 PDT
Created attachment 68691 [details]
[IMAGE] Network panel mock.
Comment 12 Pavel Feldman 2010-09-24 09:32:57 PDT
Created attachment 68692 [details]
[IMAGE] Network panel mock (item selected).
Comment 13 Timothy Hatcher 2010-09-24 09:37:04 PDT
The width of the waterfall is smaller than I would like.

How do you get back to the full list after you select something?
Comment 14 Annie Sullivan 2010-09-24 09:42:53 PDT
I really like the new mock! A couple tweaks would make it perfect:
* Ability to hide/show columns
* Quick way to tell the compressed/uncompressed sizes in bytes column for resources that were compressed over the wire (maybe a tooltip)?
* In the selected mock, if that could be a panel under the timeline panel (like httpwatch), it would be a lot easier to use both features at once. Of course this works better more for users who are trying to get a "network" view instead of a "resources" view.
* In the selected mock, would be awesome if the context menu for a header showed "copy all request headers", "copy row", "copy value".
* In the selected mock, Request URL should be clickable (opens in new tab).
Comment 15 Timothy Hatcher 2010-09-24 09:47:38 PDT
More thoughts:

* I want that file icons back, and the large file rows. They are useful for images when quickly looking for an image.
* I want a way to colapse the other coloums and just have the path name and timeline. A left right toggle arrow in the header next to the timline column divider line would do. We should remeber this setting, since I don't care about the other data (and If I do I will click it).
* Related to the above comment, please keep the timeline data on hover. We could have more data than we do today that shows up on hover (some of the other columns). Maybe only when in the collapsed column mode.
Comment 16 Pavel Feldman 2010-09-24 09:51:20 PDT
(In reply to comment #13)
> The width of the waterfall is smaller than I would like.

It is currently 40%, can increase to 50%. Note that migration to squarish bars would allow more dense views.

> How do you get back to the full list after you select something?

See cross on the title of the selected resource. Also Escape first brings to full list mode, then toggles console.

(In reply to comment #14)
> I really like the new mock! A couple tweaks would make it perfect:
> * Ability to hide/show columns
> * Quick way to tell the compressed/uncompressed sizes in bytes column for resources that were compressed over the wire (maybe a tooltip)?

Tooltip.

> * In the selected mock, if that could be a panel under the timeline panel (like httpwatch), it would be a lot easier to use both features at once. Of course this works better more for users who are trying to get a "network" view instead of a "resources" view.

This does not work well for docked view.

> * In the selected mock, would be awesome if the context menu for a header showed "copy all request headers", "copy row", "copy value".

One thing at a time :)

> * In the selected mock, Request URL should be clickable (opens in new tab).

Ok.

(In reply to comment #15)
> * I want that file icons back, and the large file rows. They are useful for images when quickly looking for an image.

Firebug does tooltips on rows.

> * I want a way to colapse the other coloums and just have the path name and timeline. A left right toggle arrow in the header next to the timline column divider line would do. We should remeber this setting, since I don't care about the other data (and If I do I will click it).

Expecting mock!

> * Related to the above comment, please keep the timeline data on hover. We could have more data than we do today that shows up on hover (some of the other columns). Maybe only when in the collapsed column mode.
Comment 17 Timothy Hatcher 2010-09-24 10:05:55 PDT
Created attachment 68699 [details]
Network Panel expanded (extra columns)
Comment 18 Timothy Hatcher 2010-09-24 10:06:30 PDT
Created attachment 68700 [details]
Network Panel collapsed (fewer columns, larger timeline)
Comment 19 Timothy Hatcher 2010-09-24 10:11:33 PDT
I'm fine adding image tooltips, but I find scrolling through the list and looking at all the images in the icons helpful (you see them all at once). It is also a small addition that does not take much room. The large rows could also give you more room to show the file name and the domain/full url on the second row in the URL column.
Comment 20 Timothy Hatcher 2010-09-24 10:18:37 PDT
As I said on IRC:

Large row mode is not just about the icons, though that does benifit from larger icons.

You also get the domain/path for the resource. And with the other coulmuns you can include more data on the second text row.

Size: Network Size / Uncompressed Size
Time: Totlal / Latency
Status: Bubble + Code / Text

The Method and Type are still one line, nd would just be top aligned.
Comment 21 Pavel Feldman 2010-09-24 10:28:37 PDT
Summary of IRC chat.

We start with refactoring Resources Panel into this one. We lose Summary graphs and Size View. Here is how this is addressed.

Summary graphs
- We show a double-row summary in the end, in bold. It shows total for all the columns. Timeline column has two bar graphs with time and size distributions.

Size View
- We allow sorting by size
- Compressed size is available on hover
Comment 22 Timothy Hatcher 2010-09-24 10:34:51 PDT
I think the status bar should be more like the Timeline panel, with checkboxes. This serves three goals, it gives you a legend for the Timeline column (which is missing in the Mocks), it lets you toggle them, and it makes it consistent with the Timeline panel UI.

Another option would be to keep the UI as the pills and include color lights inside the pills for the legend. But I think the checkboxes are better and more consistent.
Comment 23 Timothy Hatcher 2010-09-24 10:39:11 PDT
Also Timeline column should keep all the hover effects we have today, at least when columns are collapsed.
Comment 24 Annie Sullivan 2010-09-24 10:43:07 PDT
> > * In the selected mock, if that could be a panel under the timeline panel (like httpwatch), it would be a lot easier to use both features at once. Of course this works better more for users who are trying to get a "network" view instead of a "resources" view.
> 
> This does not work well for docked view.

Could we take Firebug's approach of showing the resource inline under the current timeline entry when you click on it?
Comment 25 Annie Sullivan 2010-09-24 10:49:24 PDT
I'm fine with the mocks to show/collapse all columns. But I think the best approach is what httpwatch does: they support a dozen or so different columns, put a few of the most useful ones on by default, and have a dialog to let each user customize exactly which columns are shown (sticky across sessions).

We should also make sure the columns can be resized.
Comment 26 Pavel Feldman 2010-09-28 10:31:26 PDT
Created attachment 69068 [details]
[PATCH] New panel behind the flag. Screenshots to follow.
Comment 27 Pavel Feldman 2010-09-28 10:33:36 PDT
Created attachment 69069 [details]
[IMAGE] List mode, brief.
Comment 28 Pavel Feldman 2010-09-28 10:34:08 PDT
Created attachment 69070 [details]
[IMAGE] List mode, detailed.
Comment 29 Pavel Feldman 2010-09-28 10:36:26 PDT
Things to do to make network panel live:
- Opening resources
- Filters
- Toggle optional columns
- Search
- Warning markers
- Context menu
- Make it outlive reset and not fail on resource load attempt.

Once the items above are done, I will move items from Storage panel into Resources panel.
Comment 30 Timothy Hatcher 2010-09-28 10:44:36 PDT
Comment on attachment 69068 [details]
[PATCH] New panel behind the flag. Screenshots to follow.

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

> WebCore/WebCore.vcproj/WebCore.vcproj:52787
> +					RelativePath="..\inspector\front-end\network.css"

NetworkPanel.css is a better name I think.

> WebCore/inspector/front-end/DataGrid.js:539
> +        function comparatorWrapper(trA, trB)

Just use a and b.

> WebCore/inspector/front-end/NetworkPanel.js:64
> +        return [ this._largerResourcesButton.element, this._clearButton.element ];

No spaces after [ and before ].

> WebCore/inspector/front-end/NetworkPanel.js:74
> +        return [ this.containerElement ];

No spaces after [ and before ].

> WebCore/inspector/front-end/NetworkPanel.js:96
> +        var columns = { url: {}, method: {}, status: {}, type: {}, size: {}, time: {}, timeline: {} };

No spaces after { and before }.

> WebCore/inspector/front-end/NetworkPanel.js:204
> +        if (this.calculator.startAtZero || !this.calculator.computePercentageFromEventTime) {

Does this code path even apply to the Network panel now?

> WebCore/inspector/front-end/NetworkPanel.js:439
> +        for (var i = 0; i < staleItemsLength; ++i) {

No braces.

> WebCore/inspector/front-end/NetworkPanel.js:515
> +//        if (!this.currentQuery && resource._resourceGridNode)

Don't usually land commented code.

> WebCore/inspector/front-end/NetworkPanel.js:531
> +//          if (!this.currentQuery && resource._resourceGridNode)

Don't usually land commented code.

> WebCore/inspector/front-end/NetworkPanel.js:725
> +        if (resource.timing.dnsStart !== -1) {

No braces.
Comment 31 Timothy Hatcher 2010-09-28 10:58:05 PDT
Some comments about the screenshots:

* When the rows are large, and there isn't two lines of text in the URL column, I think the text should be vertical aligned in the middle.
* The status bubble looks odd when there are two rows of text and the row is large. Maybe just colored text would be better.
* The column headers for Time and Size should explain what the second row of text is. Like "Time / Latency" and "Size / Expanded". Or some other indication.
* The dim area of the extra columns should extend all the way down, not just for the rows with content.
* The last column time label is in the wrong position.
Comment 32 Pavel Feldman 2010-09-29 02:15:25 PDT
Review comments addressed, patch landed, bug remains open.

Committing to http://svn.webkit.org/repository/webkit/trunk ...
        M       WebCore/ChangeLog
        M       WebCore/English.lproj/localizedStrings.js
        M       WebCore/WebCore.gypi
        M       WebCore/WebCore.vcproj/WebCore.vcproj
        M       WebCore/inspector/front-end/DataGrid.js
        A       WebCore/inspector/front-end/Images/networkIcon.png
        A       WebCore/inspector/front-end/NetworkPanel.js
        M       WebCore/inspector/front-end/Settings.js
        M       WebCore/inspector/front-end/WebKit.qrc
        M       WebCore/inspector/front-end/inspector.css
        M       WebCore/inspector/front-end/inspector.html
        M       WebCore/inspector/front-end/inspector.js
        A       WebCore/inspector/front-end/networkPanel.css
Committed r68636
Comment 33 Pavel Feldman 2010-09-29 11:17:33 PDT
Created attachment 69224 [details]
[PATCH] Sorting by timeline + filter + nicer detailed view.
Comment 34 Pavel Feldman 2010-09-29 11:19:04 PDT
Created attachment 69226 [details]
[IMAGE] List mode, brief (2).
Comment 35 Pavel Feldman 2010-09-29 11:19:27 PDT
Created attachment 69227 [details]
[IMAGE] List mode, detailed (2).
Comment 36 Timothy Hatcher 2010-09-29 11:34:21 PDT
Comment on attachment 69224 [details]
[PATCH] Sorting by timeline + filter + nicer detailed view.

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

> WebCore/inspector/front-end/DataGrid.js:84
> +        if (column.titleInnerHTML)

It would be nicer/safer if this was "titleDOMFragment" and used DOM fragments insteast of raw HTML.

> WebCore/inspector/front-end/DataGrid.js:705
> +            this._sortColumnCell.removeStyleClass("sort-ascending");

You could use removeMatchingStyleClasses("sort-\w+")

> WebCore/inspector/front-end/NetworkPanel.js:98
> +        columns.url.titleInnerHTML = this._makeHeaderInnerHTML(WebInspector.UIString("URL"), WebInspector.UIString("Path"));

I think "Name" would be better than URL since we don't show the whole URL.

> WebCore/inspector/front-end/NetworkPanel.js:140
> +        return title + "<div class='network-header-subtitle'>" + subtitle + "</div>";

This code needs to escape the titles to be safe: title.escapeHTML()
Comment 37 Ilya Tikhonovsky 2010-09-29 14:29:04 PDT
(In reply to comment #35)
> Created an attachment (id=69227) [details]
> [IMAGE] List mode, detailed (2).

I think the status codes column with colored bullets have better discoverability than text only version.
Comment 38 Ilya Tikhonovsky 2010-09-29 14:43:53 PDT
(In reply to comment #35)
> Created an attachment (id=69227) [details]
> [IMAGE] List mode, detailed (2).

I'd prefer to align titles in a row in detailed view.
I mean title for single value column should be at the same height as the first title for two values column. As example Method in line with Status.
Comment 39 Pavel Feldman 2010-09-30 05:43:54 PDT
(In reply to comment #36)
> (From update of attachment 69224 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=69224&action=review
> 
> > WebCore/inspector/front-end/DataGrid.js:84
> > +        if (column.titleInnerHTML)
> 
> It would be nicer/safer if this was "titleDOMFragment" and used DOM fragments insteast of raw HTML.
> 

Done.

> > WebCore/inspector/front-end/DataGrid.js:705
> > +            this._sortColumnCell.removeStyleClass("sort-ascending");
> 
> You could use removeMatchingStyleClasses("sort-\w+")
> 

Nope, it matches against entire string... 

> > WebCore/inspector/front-end/NetworkPanel.js:98
> > +        columns.url.titleInnerHTML = this._makeHeaderInnerHTML(WebInspector.UIString("URL"), WebInspector.UIString("Path"));
> 
> I think "Name" would be better than URL since we don't show the whole URL.
> 

Done.

> > WebCore/inspector/front-end/NetworkPanel.js:140
> > +        return title + "<div class='network-header-subtitle'>" + subtitle + "</div>";
> 
> This code needs to escape the titles to be safe: title.escapeHTML()

Done.

Committing to http://svn.webkit.org/repository/webkit/trunk ...
        M       WebCore/ChangeLog
        M       WebCore/English.lproj/localizedStrings.js
        M       WebCore/inspector/front-end/DataGrid.js
        M       WebCore/inspector/front-end/NetworkPanel.js
        M       WebCore/inspector/front-end/ProfilesPanel.js
        M       WebCore/inspector/front-end/StoragePanel.js
        M       WebCore/inspector/front-end/TimelineGrid.js
        M       WebCore/inspector/front-end/TimelineOverviewPane.js
        M       WebCore/inspector/front-end/inspector.css
        M       WebCore/inspector/front-end/networkPanel.css
Committed r68778
Comment 40 Timothy Hatcher 2010-09-30 08:39:55 PDT
(In reply to comment #39)
> (In reply to comment #36)
> > (From update of attachment 69224 [details] [details])
> > > WebCore/inspector/front-end/DataGrid.js:705
> > > +            this._sortColumnCell.removeStyleClass("sort-ascending");
> > 
> > You could use removeMatchingStyleClasses("sort-\w+")
> > 
> 
> Nope, it matches against entire string… 

Nope, removeMatchingStyleClasses does:

    var regex = new RegExp("(^|\\s+)" + classNameRegex + "($|\\s+)");

So it only matches a single class.
Comment 41 Pavel Feldman 2010-11-01 07:56:41 PDT
Created attachment 72506 [details]
[PATCH] Proposed change.
Comment 42 Pavel Feldman 2010-11-01 10:58:23 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	D	WebCore/inspector/InspectorResource.cpp
	D	WebCore/inspector/InspectorResource.h
	D	WebCore/inspector/front-end/Images/resourcesSilhouette.png
	D	WebCore/inspector/front-end/ResourcesPanel.js
	M	LayoutTests/ChangeLog
	M	LayoutTests/http/tests/inspector/inspector-test2.js
	M	LayoutTests/http/tests/inspector/resource-har-conversion.html
	M	LayoutTests/http/tests/inspector/resource-parameters.html
	M	LayoutTests/inspector/audits-panel-functional.html
	M	LayoutTests/inspector/extensions-test.js
	M	LayoutTests/inspector/report-protocol-errors-expected.txt
	M	LayoutTests/inspector/report-protocol-errors.html
	M	LayoutTests/inspector/styles-source-offsets.html
	M	WebCore/CMakeLists.txt
	M	WebCore/ChangeLog
	M	WebCore/English.lproj/localizedStrings.js
	M	WebCore/GNUmakefile.am
	M	WebCore/WebCore.gypi
	M	WebCore/WebCore.pro
	M	WebCore/WebCore.vcproj/WebCore.vcproj
	M	WebCore/WebCore.xcodeproj/project.pbxproj
	M	WebCore/bindings/js/JSInjectedScriptHostCustom.cpp
	M	WebCore/inspector/InjectedScriptHost.cpp
	M	WebCore/inspector/Inspector.idl
	M	WebCore/inspector/InspectorCSSStore.cpp
	M	WebCore/inspector/InspectorController.cpp
	M	WebCore/inspector/InspectorController.h
	M	WebCore/inspector/InspectorFrontendHost.cpp
	M	WebCore/inspector/InspectorResourceAgent.cpp
	M	WebCore/inspector/InspectorResourceAgent.h
	M	WebCore/inspector/front-end/ConsoleView.js
	M	WebCore/inspector/front-end/ExtensionServer.js
	M	WebCore/inspector/front-end/Resource.js
	M	WebCore/inspector/front-end/ResourceManager.js
	M	WebCore/inspector/front-end/Settings.js
	M	WebCore/inspector/front-end/StoragePanel.js
	M	WebCore/inspector/front-end/WebKit.qrc
	M	WebCore/inspector/front-end/inspector.html
	M	WebCore/inspector/front-end/inspector.js
	M	WebKit/chromium/ChangeLog
	M	WebKit/chromium/src/WebDevToolsAgentImpl.cpp
	M	WebKit/chromium/src/WebDevToolsAgentImpl.h
Committed r71035
Comment 43 Eric Seidel (no email) 2010-12-14 01:47:51 PST
Looks like this was landed but not closed.

I wonder what we can do to make webkit-patch land better for folks. :)  It will auto-update/close bugs when landing...