Bug 129913 - Web Inspector: show a debugging-oriented dashboard when scripts pause
Summary: Web Inspector: show a debugging-oriented dashboard when scripts pause
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: Brian Burg
URL:
Keywords:
Depends on: 129898
Blocks:
  Show dependency treegraph
 
Reported: 2014-03-07 14:02 PST by Brian Burg
Modified: 2014-03-10 11:09 PDT (History)
4 users (show)

See Also:


Attachments
Debugger dashboard - paused at full size (119.29 KB, image/png)
2014-03-09 19:12 PDT, Brian Burg
no flags Details
paused at small size, full width (58.23 KB, image/png)
2014-03-09 19:13 PDT, Brian Burg
no flags Details
oops, dup (119.29 KB, image/png)
2014-03-09 19:13 PDT, Brian Burg
no flags Details
paused at small size, narrow width (249.14 KB, image/png)
2014-03-09 19:14 PDT, Brian Burg
no flags Details
the patch (28.20 KB, patch)
2014-03-09 19:44 PDT, Brian Burg
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Burg 2014-03-07 14:02:08 PST
this is going to be sweet!
Comment 1 Brian Burg 2014-03-09 19:12:42 PDT
Created attachment 226266 [details]
Debugger dashboard - paused at full size
Comment 2 Brian Burg 2014-03-09 19:13:10 PDT
Created attachment 226267 [details]
paused at small size, full width
Comment 3 Brian Burg 2014-03-09 19:13:31 PDT
Created attachment 226268 [details]
oops, dup
Comment 4 Brian Burg 2014-03-09 19:14:28 PDT
Created attachment 226269 [details]
paused at small size, narrow width
Comment 5 Brian Burg 2014-03-09 19:44:05 PDT
Created attachment 226274 [details]
the patch
Comment 6 Brian Burg 2014-03-09 19:45:31 PDT
this patch depends on 129898, so the bots will have trouble.
Comment 7 Timothy Hatcher 2014-03-09 20:11:39 PDT
Comment on attachment 226274 [details]
the patch

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

> Source/WebInspectorUI/UserInterface/Models/DebuggerDashboard.js:35
> +    __proto__: WebInspector.Object.prototype,
> +
> +};

Nit: stray newline.

> Source/WebInspectorUI/UserInterface/Views/ButtonNavigationItem.css:63
> +    background-color: #999;

Nit: rgb(153, 153, 153)

> Source/WebInspectorUI/UserInterface/Views/DebuggerDashboardView.css:73
> +    from { opacity: 0.7; -webkit-transform: scale(1); }
> +    to { opacity: 1; -webkit-transform: scale(1.1); }

Not sure I would do scale. But I would have to see it.

> Source/WebInspectorUI/UserInterface/Views/DebuggerDashboardView.js:58
> +WebInspector.DebuggerDashboardView.FunctionIconStyleClassName = WebInspector.CallFrameTreeElement.FunctionIconStyleClassName;
> +WebInspector.DebuggerDashboardView.EventListenerIconStyleClassName = WebInspector.CallFrameTreeElement.EventListenerIconStyleClassName;

There are other places in the code where we cross reference directly. I like this approach better though.

> Source/WebInspectorUI/UserInterface/Views/DebuggerDashboardView.js:85
> +        // This is more than likely an event listener function with an "on" prefix and it is
> +        // as long or longer than the shortest event listener name -- "oncut".
> +        if (callFrame.functionName && callFrame.functionName.startsWith("on") && callFrame.functionName.length >= 5)
> +            iconClassName = WebInspector.DebuggerDashboardView.EventListenerIconStyleClassName;

We should probably have a helper for this logic. I think this is the third copy of it.
Comment 8 Brian Burg 2014-03-10 11:06:01 PDT
Comment on attachment 226274 [details]
the patch

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

>> Source/WebInspectorUI/UserInterface/Views/DebuggerDashboardView.css:73
>> +    to { opacity: 1; -webkit-transform: scale(1.1); }
> 
> Not sure I would do scale. But I would have to see it.

It's subtle enough that it looks more like a pulse instead of a corny ad. But it could definitely be tweaked further with your help. The glyph could probably be darker and have some sort of bezel to make it stand out better.

>> Source/WebInspectorUI/UserInterface/Views/DebuggerDashboardView.js:85
>> +            iconClassName = WebInspector.DebuggerDashboardView.EventListenerIconStyleClassName;
> 
> We should probably have a helper for this logic. I think this is the third copy of it.

This version omits the native call frame check since we can't pause in/on those as the top call frame (AFAIK?)
Comment 9 Brian Burg 2014-03-10 11:09:16 PDT
Committed r165383: <http://trac.webkit.org/changeset/165383>