Bug 132754

Summary: Web Inspector: empty timeline should not use previous timeline's zoom interval
Product: WebKit Reporter: Brian Burg <burg>
Component: Web InspectorAssignee: Matt Baker <mattbaker>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, graouts, joepeck, mattbaker, simon.fraser, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
[Patch] Proposed Fix
none
[Patch] Proposed Fix none

Brian Burg
Reported 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.
Attachments
Patch (3.19 KB, patch)
2014-08-08 17:05 PDT, Brian Burg 		no flags
DetailsFormatted DiffDiff
[Patch] Proposed Fix (4.00 KB, patch)
2015-05-13 19:17 PDT, Matt Baker 		no flags
DetailsFormatted DiffDiff
[Patch] Proposed Fix (8.18 KB, patch)
2015-05-14 19:56 PDT, Matt Baker 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2014-05-09 15:51:38 PDT
<rdar://problem/16872739>
Timothy Hatcher
Comment 2 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.
Brian Burg
Comment 3 2014-08-08 17:03:04 PDT
This should be per-TimelineRecording, with a sane default.
Brian Burg
Comment 4 2014-08-08 17:05:14 PDT
Created attachment 236325 [details] Patch
Timothy Hatcher
Comment 5 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?
Brian Burg
Comment 6 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.
Timothy Hatcher
Comment 7 2014-08-09 23:29:24 PDT
Correct. Only the auto-recorded timeline.
Timothy Hatcher
Comment 8 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?
Matt Baker
Comment 9 2015-05-13 19:17:40 PDT
Created attachment 253082 [details] [Patch] Proposed Fix
Matt Baker
Comment 10 2015-05-13 19:52:58 PDT
Wrong patch, disregard.
Matt Baker
Comment 11 2015-05-14 19:56:11 PDT
Created attachment 253169 [details] [Patch] Proposed Fix
Joseph Pecoraro
Comment 12 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!
Simon Fraser (smfr)
Comment 13 2015-05-15 20:04:51 PDT
Yay!
WebKit Commit Bot
Comment 14 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>
WebKit Commit Bot
Comment 15 2015-05-15 20:49:01 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.