Bug 143862

Summary:

Web Inspector: Implement stacktrace popovers

Product:

WebKit

Reporter:

Nikita Vasilyev <nvasilyev>

Component:

Web Inspector

Assignee:

Nikita Vasilyev <nvasilyev>

Status:

ASSIGNED ---

Severity:

Normal

CC:

graouts, inspector-bugzilla-changes, jonowells, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

See Also:

https://bugs.webkit.org/show_bug.cgi?id=146886

Bug Depends on:

144982

Bug Blocks:

Attachments:

Description	Flags
[Animated GIF] Stacktrace popover	none
Timeline stacktrace popover	none
[Animated GIF] WIP #1	none
Patch	nvasilyev: review-, nvasilyev: commit-queue-

Description Nikita Vasilyev 2015-04-16 21:32:11 PDT

Created attachment 251001 [details]
[Animated GIF] Stacktrace popover

http://timothy.hatcher.name/console/

Comment 1 Radar WebKit Bug Importer 2015-04-16 21:32:57 PDT

<rdar://problem/20582649>

Comment 2 Nikita Vasilyev 2015-04-19 21:24:44 PDT

Do we already have a popover that I can use? The one that is used for a color picker is different as it doesn’t overlay the target and has a pointy arrow instead.

Comment 3 Timothy Hatcher 2015-04-20 03:05:20 PDT

No, we don't have one like this. This one mimics a menu.

Comment 4 Nikita Vasilyev 2015-04-20 16:58:47 PDT

*** Bug 143853 has been marked as a duplicate of this bug. ***

Comment 5 Timothy Hatcher 2015-04-21 17:34:24 PDT

Per our conversation today, we are going to use a <select> like we do for NavigationItems.

Comment 6 Nikita Vasilyev 2015-04-22 18:01:39 PDT

(In reply to comment #5)
> Per our conversation today, we are going to use a <select> like we do for
> NavigationItems.

If select is used, it wouldn’t be possible to just hover over the text and click to go to the source location. E.g. on http://timothy.hatcher.name/console/ I want to be able to hover over initializeBanners, click it, jump to container.html:16.

I'm working on the popover that would look exactly like on your prototype.

Comment 7 Timothy Hatcher 2015-04-22 21:14:26 PDT

Sorry, I didn't intend hover to be the final design. It should be click. If you can make a popover, I would still make it be on mouse down.

Comment 8 Nikita Vasilyev 2015-04-22 23:37:28 PDT

Created attachment 251412 [details]
Timeline stacktrace popover

I’ve been looking to use WebInspector.CallFrameTreeElement for stacktrace popovers in the Console. I’ve noticed it’s already used in the Timeline. Do we want to eventually unify the UI for these two?

Comment 9 Timothy Hatcher 2015-04-23 04:27:10 PDT

Yes, they could use the same UI.

Comment 10 Nikita Vasilyev 2015-04-24 06:45:25 PDT

Created attachment 251548 [details]
[Animated GIF] WIP #1

First take. I reused popover from the Timeline as the quickest solution.

Comment 11 Nikita Vasilyev 2015-04-24 07:01:17 PDT

Created attachment 251549 [details]
Patch

I introduced StackTracePopOver.js and StackTracePopOver.css. It’s based on the current Timeline stacktrace popover. In this patch, it is only used in the console, but the plan is to use it for the Timeline as well.

In this patch, mouse over triggers the popover. I’ll change that, it was the easiest to implement.

Few other problems I’m aware of:
– an extra space at the bottom of the popover
– an empty space on the right side of the list item
– lots of CSS issues

Comment 12 Timothy Hatcher 2015-04-24 13:41:14 PDT

One thing missing from this patch that I would like to see: Function icon and name with location instead of just the location as we have today. I would do that part first. It is what you see right away.

If we go with a popover, hover is likely still correct. Rarely do you have a popover on mouse down. Though I am not too fond of the popover in this case. I would much rather it be a <select> or a menu like UI.

Nit: Popover not PopOver.