Bug 132754

Summary:

Web Inspector: empty timeline should not use previous timeline's zoom interval

Product:

WebKit

Reporter:

Brian Burg <burg>

Component:

Web Inspector

Assignee:

Matt Baker <mattbaker>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, graouts, joepeck, mattbaker, simon.fraser, timothy, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
[Patch] Proposed Fix	none
[Patch] Proposed Fix	none

Description Brian Burg 2014-05-09 15:51:11 PDT

As is, I feel this is really really confusing.

We could save timeline zoom intervals the same way we save other view state for back-forward navigations, but for unrelated page executions I don't see the use case.

Comment 1 Radar WebKit Bug Importer 2014-05-09 15:51:38 PDT

<rdar://problem/16872739>

Comment 2 Timothy Hatcher 2014-05-09 16:12:06 PDT

"unrelated page executions" is the key phase here. I find keeping the zoom level is useful for reload cases. Restoring a sane default for new / unrelated loads sounds fine.

Comment 3 Brian Burg 2014-08-08 17:03:04 PDT

This should be per-TimelineRecording, with a sane default.

Comment 4 Brian Burg 2014-08-08 17:05:14 PDT

Created attachment 236325 [details]
Patch

Comment 5 Timothy Hatcher 2014-08-09 20:59:55 PDT

Comment on attachment 236325 [details]
Patch

I think we should attempt to keep the state when reloading at minimum. If you are trying to see somthing happen on load it is annoying to keep fiddling with the zoom and selection. Maybe we need a hybrid approach here? First timeline uses saved settings, subsequent timelines get a sane default at the beginning of time? Maybe secondsPerPixel can always persist?

Comment 6 Brian Burg 2014-08-09 22:00:58 PDT

If the recording is explicitly started via the button, then any subsequent reload navigations do not appear at the start of the recording. So I think you are suggesting that we reuse the same zoom setting for auto-capturing sessions on the same page as the previous auto-capturing session.

Comment 7 Timothy Hatcher 2014-08-09 23:29:24 PDT

Correct. Only the auto-recorded timeline.

Comment 8 Timothy Hatcher 2015-02-10 09:46:00 PST

Comment on attachment 236325 [details]
Patch

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

Lets fix this up and make reload reuse the previous settings. Otherwise I am on board with doing this.

> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js:71
> +    this.selectionDuration = 5;

5 seconds seems short now. 10 or 15?

Comment 9 Matt Baker 2015-05-13 19:17:40 PDT

Created attachment 253082 [details]
[Patch] Proposed Fix

Comment 10 Matt Baker 2015-05-13 19:52:58 PDT

Wrong patch, disregard.

Comment 11 Matt Baker 2015-05-14 19:56:11 PDT

Created attachment 253169 [details]
[Patch] Proposed Fix

Comment 12 Joseph Pecoraro 2015-05-15 19:46:24 PDT

Comment on attachment 253169 [details]
[Patch] Proposed Fix

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

r=me

> Source/WebInspectorUI/ChangeLog:23
> +        Increase default selection time to 15 seconds.

I'm surprised by this, but I'll trust your judgement!

Comment 13 Simon Fraser (smfr) 2015-05-15 20:04:51 PDT

Yay!

Comment 14 WebKit Commit Bot 2015-05-15 20:48:55 PDT

Comment on attachment 253169 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 253169

Committed r184429: <http://trac.webkit.org/changeset/184429>

Comment 15 WebKit Commit Bot 2015-05-15 20:49:01 PDT

All reviewed patches have been landed.  Closing bug.