Bug 141232

Summary: Web Inspector: Populate Debugger sidebar with all debuggable resources
Product: WebKit Reporter: Nikita Vasilyev <nvasilyev>
Component: Web InspectorAssignee: Jonathan Wells <jonowells>
Status: RESOLVED FIXED    
Severity: Normal CC: graouts, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Progress patch for feedback.
none
[SCREENSHOT] all scripts in the debugger sidebar
none
[PATCH] Fix.
timothy: review+, jonowells: commit-queue-
[PATCH] Fix with additions for removing breakpoints.
none
Patch, MISTAKENLY UPLOADED HERE none

Description Nikita Vasilyev 2015-02-03 22:16:57 PST
Display all debuggable resources in the Debugger sidebar. Debuggable resources include JS files, HTML files with in inline JS, and transpiled files such as CoffeeScript and TypeScript.

Currently, we only display files what already have breakpoints.

There should also be a filter icon to only show resources with breakpoints.

The idea originated in https://bugs.webkit.org/show_bug.cgi?id=139526#c17.
Comment 1 Radar WebKit Bug Importer 2015-02-03 22:17:27 PST
<rdar://problem/19711872>
Comment 2 Jonathan Wells 2015-02-03 23:06:34 PST
<rdar://problem/19523709>
Comment 3 Jonathan Wells 2015-02-27 12:48:33 PST
Created attachment 247536 [details]
[PATCH] Progress patch for feedback.

Please review progress so far. Screenshot to follow. Working on isolating evaled scripts generated by the WebInspector front-end from those generated by the user's code.
Comment 4 Jonathan Wells 2015-02-27 12:50:15 PST
Created attachment 247537 [details]
[SCREENSHOT] all scripts in the debugger sidebar
Comment 5 Nikita Vasilyev 2015-02-27 19:35:41 PST
First of all, it works and it feels just right staying in the debugger to open JS files.

https://cldup.com/gJCylgVnzW-3000x3000.png

I noticed ~50 anonymous scripts on http://n12v.com. I don’t even know where are they coming from. Can we move them into a separate directory, same way it’s done in the resources sidebar?
Comment 6 Nikita Vasilyev 2015-02-27 19:44:54 PST
Comment on attachment 247536 [details]
[PATCH] Progress patch for feedback.

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

> Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:348
> +        // filter out those that were generated by the console.

https://bugs.webkit.org/show_bug.cgi?id=142123
Comment 7 Timothy Hatcher 2015-03-04 13:43:37 PST
Comment on attachment 247536 [details]
[PATCH] Progress patch for feedback.

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

Overall the patch looks good.

> Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:353
> +        // if (!script.url)
> +        //     return;

We should hide Anonymous Scripts for now. Then we can add support after filtering in in place.
Comment 8 Jonathan Wells 2015-03-05 12:40:22 PST
Created attachment 247980 [details]
[PATCH] Fix.
Comment 9 Timothy Hatcher 2015-03-05 16:37:14 PST
Comment on attachment 247980 [details]
[PATCH] Fix.

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

> Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:332
> +        this._getTreeElementForSourceCodeAndAddToContentTreeOutline(resource);

_addTreeElementForSourceCodeToContentTreeOutline would be a better name, since we don't always use the result.
Comment 10 Jonathan Wells 2015-03-05 20:56:29 PST
Comment on attachment 247980 [details]
[PATCH] Fix.

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

>> Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:332
>> +        this._getTreeElementForSourceCodeAndAddToContentTreeOutline(resource);
> 
> _addTreeElementForSourceCodeToContentTreeOutline would be a better name, since we don't always use the result.

That's much better.
Comment 11 Jonathan Wells 2015-03-06 11:48:21 PST
Created attachment 248083 [details]
[PATCH] Fix with additions for removing breakpoints.

I forgot to consider removing breakpoints. It was still the case that removing all breakpoints from a resource would cause the resource to be removed from the sidebar. This has been fixed with a change in TreeOutline.js and a small change to DebuggerSidebarPanel.js.
Comment 12 Timothy Hatcher 2015-03-06 12:20:21 PST
Comment on attachment 248083 [details]
[PATCH] Fix with additions for removing breakpoints.

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

> Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:329
> +        if ([WebInspector.Resource.Type.Document, WebInspector.Resource.Type.Script].indexOf(resource.type) < 0)

Nit: .contains(resource.type) would be better.
Comment 13 Jonathan Wells 2015-03-06 14:34:11 PST
Committed r181184: <http://trac.webkit.org/changeset/181184>
Comment 14 Jonathan Wells 2015-03-12 12:11:37 PDT
Reopening to attach new patch.
Comment 15 Jonathan Wells 2015-03-12 12:11:42 PDT
Created attachment 248530 [details]
Patch, MISTAKENLY UPLOADED HERE
Comment 16 Jonathan Wells 2015-03-12 12:20:07 PDT
Mistakenly uploaded patch from another bug.