(In reply to comment #24)
> 1) It allows a user to toggle a behaviour, without any UI to revert that
> behaviour. warnOnQuit has no UI, so users have no way of reverting the
> behaviour if they don't like it.
To be clear, my patch doesn't cause that bug - it just doesn't fix it. Your patch fixes this by using the existing exposed warnOnClose pref instead of warnOnQuit to control quit dialog behavior in the default case.
> 2) we continue to have a poorly inconsistent UI where we don't respect the
> global "warn me when closing multiple tabs" pref, as we did in Fx2. If I
> enable that warning, it doesn't happen on quit if session restore is enabled.
> That caused confusion here, and we should absolutely prevent that confusion and
> remove the regression in consistency.
My patch addresses the lack of prompting in the Quit case with session restore enabled, except that it uses the new warnOnQuit pref (togglable in visible UI only from the Quit dialog) rather than the existing pref used for per-window dialogs. If it was combined with a patch that would expose both the warnOnQuit and warnOnClose prefs in a reasonable way, I think it would be the best choice, but exposing them both in a reasonable way is probably difficult (and would require additional string changes).
> We ship betas to uncover user issues such as this, so I
> think any "we can't revert behaviour" argument is fundamentally flawed.
By "revert behaviour" I meant "lose functionality". I am disappointed that we're going to lose the behavior that lets users distinguish between guaranteed dataloss (closing a window with many tabs, "close all other tabs") and minor dataloss (whatever session restore doesn't save - form history on SSL pages, DOM state, etc.), especially when we already effectively ignore that minor dataloss when suggesting users restart for software update or add-on installation. I think this distinction is relevant, and that the current behavior allows the decision to be made implicitly (by offering to let them disable both the Quit dialog and the "close other tabs/close window" dialog separately).
Given that information I am inclined to agree that your patch is better.
(In reply to comment #24)
> 1) It allows a user to toggle a behaviour, without any UI to revert that
> behaviour. warnOnQuit has no UI, so users have no way of reverting the
> behaviour if they don't like it.
To be clear, my patch doesn't cause that bug - it just doesn't fix it. Your patch fixes this by using the existing exposed warnOnClose pref instead of warnOnQuit to control quit dialog behavior in the default case.
> 2) we continue to have a poorly inconsistent UI where we don't respect the
> global "warn me when closing multiple tabs" pref, as we did in Fx2. If I
> enable that warning, it doesn't happen on quit if session restore is enabled.
> That caused confusion here, and we should absolutely prevent that confusion and
> remove the regression in consistency.
My patch addresses the lack of prompting in the Quit case with session restore enabled, except that it uses the new warnOnQuit pref (togglable in visible UI only from the Quit dialog) rather than the existing pref used for per-window dialogs. If it was combined with a patch that would expose both the warnOnQuit and warnOnClose prefs in a reasonable way, I think it would be the best choice, but exposing them both in a reasonable way is probably difficult (and would require additional string changes).
> We ship betas to uncover user issues such as this, so I
> think any "we can't revert behaviour" argument is fundamentally flawed.
By "revert behaviour" I meant "lose functionality". I am disappointed that we're going to lose the behavior that lets users distinguish between guaranteed dataloss (closing a window with many tabs, "close all other tabs") and minor dataloss (whatever session restore doesn't save - form history on SSL pages, DOM state, etc.), especially when we already effectively ignore that minor dataloss when suggesting users restart for software update or add-on installation. I think this distinction is relevant, and that the current behavior allows the decision to be made implicitly (by offering to let them disable both the Quit dialog and the "close other tabs/close window" dialog separately).
Given that information I am inclined to agree that your patch is better.