The default bug view has changed. See this FAQ.

[Restart in Safe Mode] Add a checkbox in the prompt to choose between safe and normal restart.

VERIFIED FIXED in seamonkey2.1b3

Status

SeaMonkey
General
--
enhancement
VERIFIED FIXED
6 years ago
2 years ago

People

(Reporter: Philip Chee, Assigned: neil@parkwaycc.co.uk)

Tracking

(Blocks: 2 bugs, {ux-control, ux-error-recovery})

Trunk
seamonkey2.1b3
ux-control, ux-error-recovery
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

6 years ago
From Bug 574612 Comment 21:
> neil@parkwaycc.co.uk      2011-03-31 08:00:14 PDT
> 
> Comment on attachment 523268 [details] [diff] [review]
> Patch v1.1 Restart in Safe Mode only.
> 
> Jens had the neat idea of providing a checkbox in the prompt to choose between
> safe and normal restart, but we could do that in a followup. (I don't know how
> easy it would be to share the label of the checkbox with that of the menuitem.)
(Assignee)

Comment 1

6 years ago
Created attachment 523535 [details] [diff] [review]
Possible patch

I reused the label to avoid l10n issues.
Attachment #523535 - Flags: feedback?(philip.chee)
Attachment #523535 - Flags: feedback?(jh)

Comment 2

6 years ago
(In reply to comment #1)
> I reused the label to avoid l10n issues.

In that case, you probably should file a followup to disentangle that once we have forked 2.1 from 2.next.
Comment on attachment 523535 [details] [diff] [review]
Possible patch

This is a nice gimmick for sure! With this you can quickly restart the app in normal mode, and get back to normal mode if in safe mode. -> f=me

Maybe we should change the checkbox label to "Disable Add-ons for this restart" or something equally down to the point, but that's mostly nice to have and can therefore be moved to a follow-up.
Attachment #523535 - Flags: feedback?(jh) → feedback+
(Reporter)

Comment 4

6 years ago
Comment on attachment 523535 [details] [diff] [review]
Possible patch

>+  var checkboxText = menuitem ? menuitem.getAttribute("label") : "";
>+  var checkbox = { value: true };

Under what circumstances is menuitem null?

>   var rv = Services.prompt.confirmEx(window, promptTitle, promptMessage,
>                                      buttonFlags, restartText, null, null,
>-                                     null, {});
>+                                     checkboxText, checkbox);

If checkboxText can be "" the prompt will come up with a checkbox without any descriptive label right? How would a user respond to that?
Attachment #523535 - Flags: feedback?(philip.chee) → feedback-
(Reporter)

Comment 5

6 years ago
p.s. the patch works. so f+ on that.
(Assignee)

Comment 6

6 years ago
(In reply to comment #4)
>>+  var checkboxText = menuitem ? menuitem.getAttribute("label") : "";
>>+  var checkbox = { value: true };
>Under what circumstances is menuitem null?
In case an extension tries to call it.

>>   var rv = Services.prompt.confirmEx(window, promptTitle, promptMessage,
>>                                      buttonFlags, restartText, null, null,
>>-                                     null, {});
>>+                                     checkboxText, checkbox);
> If checkboxText can be "" the prompt will come up with a checkbox without any
> descriptive label right? How would a user respond to that?
Yeah, my bad, I guess I should have used menuitem && instead of ? : above.
(Reporter)

Comment 7

6 years ago
Hmm.
  var checkboxText = menuitem;
  var checkbox = menuitem ? { value: true } : {};
(Assignee)

Comment 8

6 years ago
Created attachment 523608 [details] [diff] [review]
Addressed review comments
[Checked in: Comment 15]

I'm going to be annoying and post several slightly different versions of this...
Assignee: nobody → neil
Attachment #523535 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #523608 - Flags: review?(philip.chee)
(Assignee)

Comment 9

6 years ago
Created attachment 523609 [details] [diff] [review]
Alternative patch
Attachment #523609 - Flags: review?(philip.chee)
(Assignee)

Comment 10

6 years ago
Created attachment 523610 [details] [diff] [review]
Another alternative patch
Attachment #523610 - Flags: review?(philip.chee)
(Reporter)

Comment 11

6 years ago
Comment on attachment 523608 [details] [diff] [review]
Addressed review comments
[Checked in: Comment 15]

I like this one. It allows an extension to pass null to remove the checkbox as an option.
Attachment #523608 - Flags: review?(philip.chee) → review+
(Reporter)

Comment 12

6 years ago
Comment on attachment 523609 [details] [diff] [review]
Alternative patch

> safeModeRestartPromptTitle=Restart with Add-ons Disabled
> safeModeRestartPromptMessage=Are you sure you want to disable all add-ons and restart?
> safeModeRestartButton=Restart
>+safeModeRestartCheckbox=Restart with Add-ons Disabled
Hardly worth a additional string since it'll be string frozen on 2.1 and would likely be removed or changed for 2.next.
Attachment #523609 - Flags: review?(philip.chee)
(Reporter)

Comment 13

6 years ago
Comment on attachment 523610 [details] [diff] [review]
Another alternative patch

Nothing wrong with this. I just like the flexibility in attachment 523608 [details] [diff] [review]
Attachment #523610 - Flags: review?(philip.chee)
(Reporter)

Comment 14

6 years ago
Comment on attachment 523610 [details] [diff] [review]
Another alternative patch

r=me in case you prefer this one.
Attachment #523610 - Flags: review+
(Assignee)

Comment 15

6 years ago
Pushed changeset bf54745a7aec to comm-central.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
I wonder whether we should add a localization note for the fact that we reuse the menuitem label for the checkbox now. Otherwise localizers might make wrong assumptions (I mean, in other languages, a menuitem label string might be subject to different rules/constraints than a checkbox label string).
(Reporter)

Comment 17

6 years ago
Comment on attachment 523609 [details] [diff] [review]
Alternative patch

> I wonder whether we should add a localization note for the fact that we reuse
> the menuitem label for the checkbox now. Otherwise localizers might make wrong
> assumptions (I mean, in other languages, a menuitem label string might be
> subject to different rules/constraints than a checkbox label string).

I hadn't considered that. In that case rs=me for adding something like:
> +safeModeRestartCheckbox=Restart with Add-ons Disabled
If you can get this in before Callek spins 2.1b3
Attachment #523609 - Flags: review+
(Reporter)

Updated

6 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Created attachment 523866 [details] [diff] [review]
possible l10n improvement [Checkin: comment 20]
Attachment #523866 - Flags: review?(philip.chee)
(Reporter)

Updated

6 years ago
Attachment #523866 - Flags: review?(philip.chee) → review+
(Reporter)

Comment 19

6 years ago
rs=me
Comment on attachment 523866 [details] [diff] [review]
possible l10n improvement [Checkin: comment 20]

http://hg.mozilla.org/comm-central/rev/21b306aef054
Attachment #523866 - Attachment description: possible l10n improvement → possible l10n improvement [Checkin: comment 20]
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b3
Blocks: 646897
Attachment #523608 - Attachment description: Addressed review comments → Addressed review comments [Checked in: Comment 15]
Attachment #523609 - Attachment is obsolete: true
Attachment #523610 - Attachment is obsolete: true
I happen to notice that this bug hasn't yet been VERIFIED
...let's do it.

Mozilla/5.0 (X11; Linux x86_64; rv:2.2a1pre) Gecko/20110412 Firefox/4.2a1pre SeaMonkey/2.2a1pre ID:20110412003004

The checkbox is present, and considering that no-one said it wasn't: → VERIFIED
Status: RESOLVED → VERIFIED
(Reporter)

Updated

2 years ago
Blocks: 1194744
You need to log in before you can comment on or make changes to this bug.