Bug 2019055, 2019267 - Customize CrashReporter UI for policy autosubmits and Enterprise restart behavior#494
Conversation
|
You will want a review from @afranchuk I guess ? |
| @@ -7,6 +7,7 @@ crashreporter-branded-title = { -brand-short-name } Crash Reporter | |||
| crashreporter-apology = We’re Sorry | |||
| crashreporter-crashed-and-restore = { -brand-short-name } had a problem and crashed. We’ll try to restore your tabs and windows when it restarts. | |||
| crashreporter-plea = To help us diagnose and fix the problem, you can send us a crash report. | |||
| crashreporter-policy-explanation = Per configured policy, a report is automatically sent to your admin. | |||
There was a problem hiding this comment.
Those strings are enterprise specific? We need to introduce a new crashreporter-enterprise.ftl for example and isolate all enterprise-specific string there.
| let input_enabled = if config.policy_auto_submit { | ||
| data::Synchronized::new(false) | ||
| } else { | ||
| submit_state.mapped(|s| s == &SubmitState::Initial) | ||
| }; |
There was a problem hiding this comment.
If config.policy_auto_submit is true, maybe it would make more sense to set the submit state to InProgress (which would have the side-effect of disabling the inputs). InProgress (and the follow-on states) is sort of defined as not allowing any more input (and we could more rigorously document that in comments to be sure that remains true).
| Button["quit"] on_click(cc! { (logic) move || logic.push(|s| s.quit()) }) | ||
| enabled(&input_enabled) hsize(160) | ||
| enabled(&quit_enabled) hsize(160) | ||
| { | ||
| Label text(config.string("crashreporter-button-quit")) | ||
| Label text(config.string(if cfg!(feature = "enterprise") {"crashreporter-button-ok"} else {"crashreporter-button-quit"})) | ||
| } |
There was a problem hiding this comment.
Since the behavior of the "Ok" button is different from the "Quit" button (quit potentially triggers submission, whereas ok will just dismiss the window), I'd prefer it be defined as a separate button entirely.
We could add support for attributes in the ui! macro, which would allow us to more easily add/remove the buttons with #![cfg(...)]. Or just add support for "switching back" to imperative code. I'll take a look at that.
There was a problem hiding this comment.
Yep, there's a pretty simple change we can make to the ui! macro to allow you to insert regular imperative code (the macro is already pretty simple, so it made it easy). I can make a bug to get that into f-m.
There was a problem hiding this comment.
Sounds good. Yeah I tried the cfg attribute first and ran into the macro issues and guessed it would be hard to deal with that; glad it sounds like it was easy!
There was a problem hiding this comment.
I created https://bugzilla.mozilla.org/show_bug.cgi?id=2021010. I'll have a patch in soon.
3 participants
Note: This does not add the behavior of auto-submitting yet, so even with the env variable for policy auto-submit, the crash report will be submitted on pressing the button in the crash reporter app rather than any earlier.
Enterprise crash reporter when policy is not on:
Enterprise crash reporter when policy is on:
Note: The text at the bottom "Your crash report will automatically be submitted." should not end up being visible once the policy autosubmit behavior is implemented; that label is tied to the submit status, and the submit will be kicked off as the UI becomes visible, so users will instead see a progress bar/completion message.