Bug 141232 - Web Inspector: Populate Debugger sidebar with all debuggable resources
Summary: Web Inspector: Populate Debugger sidebar with all debuggable resources
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: jonowells
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-02-03 22:16 PST by Nikita Vasilyev
Modified: 2015-03-12 12:20 PDT (History)
7 users (show)

See Also:


Attachments
[PATCH] Progress patch for feedback. (6.49 KB, patch)
2015-02-27 12:48 PST, jonowells
no flags Details | Formatted Diff | Diff
[SCREENSHOT] all scripts in the debugger sidebar (181.86 KB, image/png)
2015-02-27 12:50 PST, jonowells
no flags Details
[PATCH] Fix. (7.86 KB, patch)
2015-03-05 12:40 PST, jonowells
timothy: review+
jonowells: commit-queue-
Details | Formatted Diff | Diff
[PATCH] Fix with additions for removing breakpoints. (10.35 KB, patch)
2015-03-06 11:48 PST, jonowells
no flags Details | Formatted Diff | Diff
Patch, MISTAKENLY UPLOADED HERE (10.04 KB, text/plain)
2015-03-12 12:11 PDT, jonowells
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
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 jonowells 2015-02-03 23:06:34 PST
<rdar://problem/19523709>
Comment 3 jonowells 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 jonowells 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 jonowells 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 jonowells 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 jonowells 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 jonowells 2015-03-06 14:34:11 PST
Committed r181184: <http://trac.webkit.org/changeset/181184>
Comment 14 jonowells 2015-03-12 12:11:37 PDT
Reopening to attach new patch.
Comment 15 jonowells 2015-03-12 12:11:42 PDT
Created attachment 248530 [details]
Patch, MISTAKENLY UPLOADED HERE
Comment 16 jonowells 2015-03-12 12:20:07 PDT
Mistakenly uploaded patch from another bug.