Bug 194956 – Web Inspector: Timelines: add UI for preventing auto-stop

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

There should be some visible UI (e.g. not a keyboard shortcut) that lets the developer toggle auto-stop on/off.

[Devin Rousso]

Created attachment 362739 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/48319655>

[Radar WebKit Bug Importer]

<rdar://problem/48319654>

[Joseph Pecoraro]

Comment on attachment 362739 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js:957
> +        if (WI.settings.timelinesAutoStop.value)
> +            this._resetAutoRecordingMaxTimeTimeout();

We should probably only do this if we are capturing.

> Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js:59
> +        this._autoStopCheckboxNavigationItem = new WI.CheckboxNavigationItem("auto-stop-recording", WI.UIString("Stop recording after load"), WI.settings.timelinesAutoStop.value);

The string "Stop recording after load" sounds incorrect.

It sounds like "load" event, but it isn't...

And the auto stop behavior stops for different reasons, and the "load" event isn't exactly one of them:

    • WI.TimelineManager.MaximumAutoRecordDuration = 90000; // 90 seconds
    • WI.TimelineManager.MaximumAutoRecordDurationAfterLoadEvent = 10000; // 10 seconds
    • WI.TimelineManager.DeadTimeRequiredToStopAutoRecordingEarly = 2000; // 2 seconds

So perhaps this should be:

    Stop recordings after page load    
    Stop recordings after navigation
    Automatically stop recordings once page loads
    
Each have less of a "load" event connotation?

[Devin Rousso]

Comment on attachment 362739 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js:957
>> +            this._resetAutoRecordingMaxTimeTimeout();
> 
> We should probably only do this if we are capturing.

Ah, good catch!

>> Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js:59
>> +        this._autoStopCheckboxNavigationItem = new WI.CheckboxNavigationItem("auto-stop-recording", WI.UIString("Stop recording after load"), WI.settings.timelinesAutoStop.value);
> 
> The string "Stop recording after load" sounds incorrect.
> 
> It sounds like "load" event, but it isn't...
> 
> And the auto stop behavior stops for different reasons, and the "load" event isn't exactly one of them:
> 
>     • WI.TimelineManager.MaximumAutoRecordDuration = 90000; // 90 seconds
>     • WI.TimelineManager.MaximumAutoRecordDurationAfterLoadEvent = 10000; // 10 seconds
>     • WI.TimelineManager.DeadTimeRequiredToStopAutoRecordingEarly = 2000; // 2 seconds
> 
> So perhaps this should be:
> 
>     Stop recordings after page load    
>     Stop recordings after navigation
>     Automatically stop recordings once page loads
>     
> Each have less of a "load" event connotation?

I actually was trying to refer to the "load" event, as that's what I'd expect the vast majority of users encounter (having a page that takes more than 10s (or even 90s) to load is pretty rare).

I like "Stop recording once page loads" though.

[Devin Rousso]

Created attachment 362748 [details]
Patch

[Devin Rousso]

Created attachment 362749 [details]
[Image] After Patch is applied

[Joseph Pecoraro]

Comment on attachment 362748 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 362748 [details]
Patch

Clearing flags on attachment: 362748

Committed r241982: <https://trac.webkit.org/changeset/241982>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.