384658 - Add checkbox for setting browser.warnOnQuit

Status: RESOLVED WONTFIX

(Firefox :: Settings UI, enhancement)

Description

[Michael Wu [:mwu]]

A checkbox somewhere in the preferences dialog is needed so the user can turn the quit confirmation dialog back on. (browser.warnOnQuit)

Comment 1

[bomfog]

Mozillazine thread: http://forums.mozillazine.org/viewtopic.php?p=3205327

Comment 2

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Prefs Patch v0.1

Possible way of doing this. I talked with Beltzner about this and he's on board with something along these lines.  I can attach screen shot if that will work better.

(It does enable/disable the "when firefox starts" menulist. I think this is what Beltzner and I decided upon)

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 330644 [details] [diff] [review]
Prefs Patch v0.1

Not sure if the wording is right, but yeah, I think this is the most straightforward way of doing this.

Comment 4

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Prefs Patch v0.2

Update to ensure that necessary UI changes are done when the pref pane is loaded.

Comment 5

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

I do have one concern with this change. The quit dialog doesn't give all 3 options about which startup path to take. So with this change users will see the dialog, and then they must choose between "quit" and "save and quit". The quit option doesn't distinguish between "home page" and "blank page", so this might lead to some confusion.

Also, what are the next steps here? Since there are new strings, this will need to be translated. I also wasn't sure about the ENTITY name or accesskey, so maybe those will need to be changed.

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #5)
> I do have one concern with this change. The quit dialog doesn't give all 3
> options about which startup path to take. So with this change users will see

That's fine; at quit time the choice should be between saving their state (one time) or not. If they choose to "not be asked again" then it will set the default startup state (session restore / !session restore) which will re-read the pref about how they wanted to startup.

I don't think we want to have users make choices about their next startup type at shutdown; just whether or not they were done what they were doing (quit) or if they're stopping in the middle of things (save & quit).

> Also, what are the next steps here? Since there are new strings, this will need
> to be translated. I also wasn't sure about the ENTITY name or accesskey, so
> maybe those will need to be changed.

If you've changed strings, you need to "rev the entities", which means change the names of the entities (so "foo.bar.quitString" might become "foo.bar.quitQuestion") which will alert localizers to the need for new translation. If you're adding strings, you don't need to do anything more.

Oh, and it needs review. Request from mconnor or a browser peer (vlad, gavin or mano).

Comment 7

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Prefs Patch v0.3

* changed entity name (for string change)
* removed unnecessary onchange

Comment 8

[Mike Connor [:mconnor]]

Comment on attachment 333702 [details] [diff] [review]
Prefs Patch v0.3


>+   readshowBrowserWarnOnQuit: function ()
>+   {
>+     this.showBrowserWarnOnQuitPrefChanged();
>+
>+     // don't override the preference's value in UI
>+     return undefined;
>+   },
>+
>+   /**
>+    * Enables or disables the "when Firefox starts" preference element
>+    * consequently updating the associated UI.
>+    */
>+   showBrowserWarnOnQuitPrefChanged: function ()
>+   {
>+     var warnOnQuitPref = document.getElementById("browser.warnOnQuit");
>+     var startupPagePref = document.getElementById("browser.startup.page");
>+     startupPagePref.disabled = warnOnQuitPref.value;
>+   },

Just inline this in the function above, it doesn't make sense to do this as a second function since there's only one consumer.

r=mconnor with that fixed.

Comment 9

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

I know this has been r+ed, but I realized that there's a problem with this. If the user unchecks the box (enabling the menulist), then selects "show my windows and tabs from last time", then checks the box, then quits, we do not warn on quit, even though that's what you would expect from the UI.

There's an easy fix, and that's to not have the shutdown code check browser.startup.page==3; instead relying only on browser.warnOnQuit to determine if we show that dialog. I think that's what the expected behavior here would be, but want confirmation.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #9)
> There's an easy fix, and that's to not have the shutdown code check
> browser.startup.page==3; instead relying only on browser.warnOnQuit to
> determine if we show that dialog. I think that's what the expected behavior
> here would be, but want confirmation.

I think that would work, but you'd need to change the _onQuitRequest code to also set warnOnQuit in addition to setting browser.startup.page=3 when the "never ask" checkbox is checked and they select "save and quit". In fact it should probably be setting warnOnQuit instead of warnOnClose in the "Quit/Never ask" case too, right?

Also need to fix the comment in nsBrowserGlue about warnOnQuit being a "hidden global boolean".

Comment 11

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to comment #10)

I took a closer look at this and talked with mconnor while he was in MV, and it's looking like even that isn't the best way to go. In fact we decided this is really messy and there's no "easy" fix.

If we go with the above fix, we would probably need to change the dialog, since the path I mentioned would then show a dialog asking if you want to save your session or not, even though that's what you set in the prefs. That might not be too bad, but I'm not sure I would like that. Could lead to some confusion.

I also thought about going back to Firefox 2 behavior and simplifying that exit dialog to take out the resume session once, but that's no good either.

I'm going to think on this some more, but if anybody has a solution that might be good (even if it's not well developed), let me know. I think it's important to get this pref back in somehow, but it's much more difficult than I thought.

Comment 12

[eyal gruss]

invalid due to Bug 592822?

Comment 13

[:Gavin Sharp [email: [e-mail]]]

I think so.