add "Don't show this again" checkbox to e10s slow script dialog

NEW
Assigned to

Status

()

3 years ago
a year ago

People

(Reporter: blassey, Assigned: blassey)

Tracking

(Blocks: 1 bug)

unspecified
Points:
---

Firefox Tracking Flags

(e10s+)

Details

Attachments

(1 attachment)

Comment hidden (empty)
The general feeling here is that:

1) we shouldn't place a checkbox in front end ui that sets a global pref that disables an important safety feature.
2) the pref that controls this is available in about:config for power users

hence the e10s team felt this should be wontfix.

not blocking rollout as well.
tracking-e10s: --- → +
Flags: needinfo?(bbermes)
Assignee: nobody → blassey.bugs
Created attachment 8741476 [details] [diff] [review]
add_never_ask_checkbox.patch

See bug 1260769, the argument for this is parity with non-e10s
Attachment #8741476 - Flags: review?(mconley)
Comment on attachment 8741476 [details] [diff] [review]
add_never_ask_checkbox.patch

Gonna hold on this review until barbara comes back with what we want here.
Attachment #8741476 - Flags: review?(mconley)
Let me ask the other way around, could we remove this from the non-e10s to make it parity?

Or based on my comment: https://bugzilla.mozilla.org/show_bug.cgi?id=1260769#c11, in order to compare non-e10s and e10s properly, remove the users from the comparison that have this set to "don't show again".
Flags: needinfo?(bbermes)
(In reply to Barbara Bermes [:barbara] from comment #4)
> Let me ask the other way around, could we remove this from the non-e10s to
> make it parity?
Sure, just need to make a product/UX call here. We've had this functionality for a long time, removing it shouldn't be taken lightly. Also, what should we do with users who already have it set.
 
> Or based on my comment:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1260769#c11, in order to
> compare non-e10s and e10s properly, remove the users from the comparison
> that have this set to "don't show again".

Benjamin, can you filter out the uptime of users who have dom.max_script_run_time set to anything bug 10?
Flags: needinfo?(benjamin)

Comment 6

3 years ago
The checkbox does fundamentally different things on e10s and non-e10s.

On non-e10s, when the dialog is displayed the content JS is not running. So once the user sees the dialog, they *have* to make a decision.

With e10s, we show the dialog as a courtesy/warning, but the content is still running in the background. If it recovers, the dialog self-dismisses (I think: Jim or Brad, please confirm). So the consequences of removing this option are very different.

I support the plan of not having this checkbox for e10s.

I'm not sure what the "can you filter" question is about. I'm pretty sure we currently don't record that pref value, based on reading http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/TelemetryEnvironment.jsm#96

If we started recording that value, we could use it as a filter or correlate for analysis passes.
Flags: needinfo?(benjamin)
I'm surprised we tie this checkbox to claring the global slow script handling dialog. This means that if a user randomly checks "don't bug me about this" firefox will never stop a slow script again. This to me feels more like a bug vs. a feature so I filed bug 1265059.
You need to log in before you can comment on or make changes to this bug.