Closed Bug 550559 Opened 14 years ago Closed 5 years ago

Decouple Session Restore from Quit warning, warn when using keyboard shortcut on Mac & Linux

Categories

(Firefox :: Session Restore, defect)

x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 65
Tracking Status
blocking2.0 --- -
relnote-firefox --- -
firefox65 --- fixed

People

(Reporter: limi, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Keywords: feature)

Attachments

(3 files, 4 obsolete files)

This is especially noticable on a Mac since ⌘Q and ⌘W are right next to each other, but in general it violates the principle of least surprise:

We currently ask when you quit your browser whether that's what you wanted to do, and give you an option to "never ask again".

However, when turning on Session Restore, this "never ask again" behavior is automatically enabled, which is confusing. I appreciate the logic ("you're not losing anything by quitting since session restore is awesome"), but it still causes a long startup time if you do it accidentally.

We could do this only when you quit using a keyboard shortcut too, but that gets back into the "being too clever" side of things again, since you would have two slightly different "never ask me again" settings.

Since it's so easy to check the "never ask me again" on the quit dialog, we shouldn't assume that Session Restore implies that you don't want to be asked. So let's please disable this behavior.



PS: When one of our developers doesn't even know about this behavior and creates an extension to fix it, it doesn't speak very well to its obviousness. ;) https://addons.mozilla.org/en-US/firefox/addon/55824/
PPS: Yes, other applications & browsers have this same issue, but that doesn't mean we shouldn't make it better.
OK, I guess I changed my mind slightly here after living with it for a while — I now think we should ask when trying to quit using the keyboard (still with the "Do not ask me again" shortcut, but not when quitting using the menu or (on Windows) the close button.
Refining title, Ctrl-Q is present on Mac, Cmd-Q on OS X. As far as I know, Ctrl-Q doesn't work on Windows, so it's not a problem there. 

Alt-F4 closes a window on Windows, so it's not going to be accidentally triggered when closing a tab (Ctrl-W) — so we don't need to handle that in any way (unless there's an Alt-F3 or Alt-F5 shortcut I don't know about).
Summary: Decouple Session Restore from Quit warning → Decouple Session Restore from Quit warning, warn when using keyboard shortcut on Mac & Linux
With the removal of the Quit warning dialog on OS X this bug is kinda annoying because shortcuts to close tabs and the browser itself are directly beneath each other. It happens so often for me that I accidentally close the whole browser when I'm trying to close a tab. With a lot of tabs open it takes a long time to shutdown and restart, in most cases I loose about 4-5 minutes.

I will ask for blocking so we can sort out what should happen for Fx4 and what not.
Blocks: 592822
blocking2.0: --- → ?
To reduce confusion (people on IRC said it was still a bit unclear), here's what we're proposing:

* On Mac & Linux, since they have the Cmd-Q shortcut to quit the browser, we want to warn users that they're about to close all their tabs (or quit) when they do it using the keyboard shortcut *only*.

  * In that dialog, *do* offer the option to not ask again, for the people who just want this to quit automatically.

* Quitting the application by using the menu or using the window close button (Windows & Linux only) will not trigger any warning. (Restoring your previous session should be even more discoverable once the new about:home design lands, which seems to be on track to make Firefox 4.0.)

Let me know if anything is missing or unclear.
Should probably be a softblocker if so. (Adding d? to get driver input, let me know if this isn't the way to go about this :)
Whiteboard: [softblocker][d?]
paul,  can you also make a call here on difficulty of a possible fix. that would play into the possibility of this becoming a blocker.  It needs to get out of the nomination queue, one way or the other.
My current guess is not easy. That's just going off what I remember when I looked into this briefly before.
Version: 3.6 Branch → Trunk
blocking2.0: ? → -
Whiteboard: [softblocker][d?] → [d?]
Echoing comment 3.
(I'm on linux)
On comment 4 - '* Quitting the application by using the menu or using the window close button
(Windows & Linux only) will not trigger any warning. (Restoring your previous
session should be even more discoverable once the new about:home design lands,
which seems to be on track to make Firefox 4.0.)'

First, a general point.
What is the rationale between the close button not closing FF window with 40 tabs, only if there is another window which may only have one tab?

Addressing this feature specifically.

The above comment to me seems to miss several usability issues for those with disadvantaged net connections.

If the web were static, CPU and bandwidth were free, there isn't really any issue.

Unfortunately, when you get away from this ideal case, as is common in many parts of the world, restoring a large number of tabs gets painful.

To go into the usability issues I've hit.

A) A fair amount of content may not be cached - especially if the browser session has been up for some time. This can take a long time, cost money, or lose data that you were looking at in dynamic pages that have now changed.

Yesterday, I was out, and accidentally closed FF, and had to buy wifi access, rather than just use the data in my open tabs.

B) Network unreliability.
If the network isn't available all the time, then many of the tabs come back broken.

C) Stateful pages.
I'm a member of a subscription news site that limits my downloads to 20 per day, and sets the no-cache header. This means that when FF restarts, my tabs that I've opened as reference over the last 3-4 days all load at once, and take me over my 20 per day limit, meaning I need to wait a day.

D) CPU - I don't have a very fast CPU, even neglecting the above issues, it would take a while to load.

A-C could in principle be addressed by by making it possible to set the cache up in such a way that prerequisites of pages are always cached, and on restoring state, the state is restored solely from cache, not touching the network at all.
blocking2.0: - → ?
This still doesn't block; existed in 3.6, will continue to exist in 4.0.
blocking2.0: ? → -
Whiteboard: [d?]
It existed in 3.6, yes.
But for users who are trying migrating from multiple windows to tab groups, it will hit for the first time (as it did for me)
Limi:
should we also change the behaviour in Private Browsing Mode? Currently if you cmd+Q while in PBM, no quit dialog is ever shown. Based on http://blog.mozilla.com/metrics/2010/08/23/understanding-private-browsing/ it doesn't seem like the user would be losing much data if we preserved the current behaviour.
This still relevant now that we've changed the quit dialog to be ... not so much there ...?
I think the keyboard stuff is relevant, but the dialog stuff less so.
" Quitting the application by using the menu or using the window close button
(Windows & Linux only) will not trigger any warning."

Absolute. garbage. 

This has made FF4 unusable for me. Good luck keeping up with the competition by de-feature-izing this browser into yet another Chrome clone. 

At this point, I'd rather start putting up with Chrome, since it seems to be adding things, not taking them away.

not warning when there is more than one tab open is insane, and due to the ridiculous amount of time it takes to restart FF, its even more annoying.

What is wrong with this project? Make it use less CPU, make it use less memory, but dont sit there are rip features out.
(In reply to comment #14)
> " Quitting the application by using the menu or using the window close button
> (Windows & Linux only) will not trigger any warning."
> 
> Absolute. garbage. 
> 
> This has made FF4 unusable for me. 

This bug is not about the feature change.  Your looking for bug 592822 and bug 629485.  You know, I get tired of hearing that session store don't work, but most people never check Firefox to see how many ways there are to restore sessions, or find out how to turn the dialogs on which I'm not going to do here since this bug is different altogether.
I'm not that comfortable with this part of the codebase, but this seems like a fairly trivial fix.
Attachment #594216 - Flags: feedback?(limi)
Blocks: 565567
Blocks: 52821
Comment on attachment 594216 [details] [diff] [review]
Don't disable warn-on-quit when using session restore

This seems fairly simple, but I can currently only test on Windows. The concerned platforms for this bug (IIRC) are mac and linux, so those need a brief test.

Try push at: https://tbpl.mozilla.org/?tree=Try&rev=e6235a00b6db
Attachment #594216 - Flags: feedback?(limi) → review?(limi)
I don't think we want to show a quit warning when we're restarting, e.g. for an extension install. I think that case is covered by browser.sessionstore.resume_session_once==true, which you're removing.
Thanks Gavin. That looks like the case.
Attachment #594216 - Attachment is obsolete: true
Attachment #594363 - Flags: review?(limi)
Attachment #594363 - Flags: feedback?(gavin.sharp)
Attachment #594216 - Flags: review?(limi)
Comment on attachment 594363 [details] [diff] [review]
Don't disable warn-on-quit when session restore is enabled

You probably want Gavin to review the code; +1 from a UI standpoint from me. If there's a tryserver build attached, I can help test and see if there are any edge cases, since I can't read the code changes that well. :)
Attachment #594363 - Flags: ui-review+
Attachment #594363 - Flags: review?(limi)
Attachment #594363 - Flags: review?(gavin.sharp)
Attachment #594363 - Flags: feedback?(gavin.sharp)
Attachment #594363 - Flags: feedback?
Attachment #594363 - Flags: feedback?
Blocks: 502908
Comment on attachment 594363 [details] [diff] [review]
Don't disable warn-on-quit when session restore is enabled

I think Paul gets to handle this one, since he really loves quitting/prompting code.

(I apologize for the unreasonable delay in getting to this)
Attachment #594363 - Flags: review?(gavin.sharp) → review?(paul)
Huh, I guess I hadn't CC'd myself here.
Assignee: nobody → jgilbert
Status: NEW → ASSIGNED
Comment on attachment 594363 [details] [diff] [review]
Don't disable warn-on-quit when session restore is enabled

Review of attachment 594363 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay. I've had this on my mind for a while though.

My main problem here is that this doesn't really do what the end goal of this bug is. This simply removes a check from the dialog path, but it doesn't handle the keyboard case at all, which is what we really want.

So I think we should move discussion about this specifically to a new bug. Since this is (mostly) only going to affect people who manually gone & changed showQuitWarning to true & have sessionstore on, it shouldn't be too disruptive. Though, we'll show the quit dialog which asks about saving tabs when session restore is already on... so that's not ok. Also, all the people quitting by closing the last window will suddenly have a dialog in their face, event without showQuitWarning (I think I take back "shouldn't be too disruptive"). We can discuss more in a new bug if you file it
Attachment #594363 - Flags: review?(paul) → review-
Let me add my two cents (scroll down for TLDR) (I'm a Windows user BTW).

I'm very close to @Ian Stirling opinion from #comment 8. I want to have an option to *absolutely* ask on quit, whatever makes me do so (shortcut, click, menu item); isn't there any 'onbeforequit'-like hook in the codebase, whatever was the root cause that triggered it?

Let me explain the point.

For me, Firefox is The Centre Of Management of The Universe. I regularly have 100+ tabs open, with their content being changed by me using some Greasemonkey scripts that add additional content when I click this and that (and this is gone after restart). I can't allow browser just to quit accidentally because of some misfortunate events happening. And there are zillions of things that can accidentally go wrong. I'm a power user, yet it still happens from time to time and you can't help:
- I can hit ALT-F4 while I was sure the focus was in the different app. Or hit ALT-F4 more than once, one of this happening in Firefox, because the OS switched the focus after closing previous app to FF which I didn't expect.
- I can have a different application (say Notepad) on top of Firefox on the desktop stack. And now two cases:
-- when both are maximized, I can hit 'X' twice by chance, because for instance my mouse is malfunctioning, or my touchpad too sensitive etc etc.
-- when at least one of them (say Notepad on top) is not maximized, I want to try to close the Notepad, but I move my mouse (or especially touchpad) 2 pixels too far on the right, and, voila, Firefox goes to hell.
-- I open a 'File' menu menu via LEFT_ALT, DOWN_ARROW, I see the options, I iterate them via DOWN_ARROW, UP_ARROW, and I push enter on exit, but I wanted one item higher in the menu (theoretical, but can happen).
-- Also, I may confuse shortcuts, because I'm overworked and simply tired!

All of the above are very unlikely, but still happen from time to time, and each of those is a *DISASTER*, when you have 100+ tabs opened, and (God prevent) you're out of mobile range / your mobile network provider sucks / your phone-as-a-modem sucks etc. while Firefox still thinks you're online.

It's especially painful when you buffered than damn 15-minutes YouTube video to watch it offline, then you accidentally close Fx, and you're screwed.

Also, I might be downloading a huge file from a server that doesn't support restarts-in-the-middle. Default Firefox behavior (with All-in-one-sidebar) is to close the actual window, and leave only the downloads opened! This is ridiculous. I can now either wait 1h till the download ends, or restart the Firefox, and screw the download. (it doesn't affect me, as I use AiOS add-on; just pointing it).

When I tell the browser I want a warning, I REALLY want it, and don't be overly clever to not show it to me when you think it's not necessary. Period. Hope I'm convincing enough.

Please, always display me a warning when I want to quit Firefox and I stated it in about:config. Actually I only quit Fx to install upgrades/add-ons, or when the memory consumption is too big. I hibernate my comps, and have Fx opened for whole days.

______
TL;DR:
- I may have 100+ tabs
- I may have slow connection (laptop, with mobile phone as a modem)
- I may sometimes go offline
- I may use dynamic content (Greasemonkey, Flash videos loaded) that will be lost after restart
- I might be tired and do something stupid by mistake! I'm a human after all

Then, session restore is only helpful in the way that you don't lose URLs you were browsing.
*in the previous post, it should be: 'Default Firefox behavior (*without* All-in-One Sidebar)'
Attached patch WIP for keyboard quit (obsolete) — Splinter Review
This just adds a notification for when we quit via the keyboard. This could be used to popup a new window & presumably spin the event loop or something.

We can't quite do what (for example) Chrome is doing and capture everything down in Cocoa-land because we construct the menu from XUL and then pass everything straight back there without really processing the event in Cocoa. The Cocoa part of this is for when quit is sent via some other way, for example cmd+q while in cmd-tab, activity monitor.

This isn't a well formed idea yet but I had _something_ to put up. If somebody (Frank?) wants to take this and run, feel free - I don't plan on working on this in the near future.
OS: Mac OS X → All
Despite having browser.showQuitWarning and browser.warnOnQuit set to true (as well as browser.tabs.warnOnClose), pressing Ctrl+Q (or using File → Exit) closes the program with no confirmation.
This is very annoying when you were using the browser and just intended to close a tab instead (Ctrl+W), since it leaves you with no browser for 7 minutes (bug 806109) while it is busy closing a program you didn't want to close at all!.


Note: I'm duping here a bunch of related bugs.
Listed on bug 565567, there is https://addons.mozilla.org/en-US/firefox/addon/always-ask/ as workaround.
Seems I don't have appropiate permissions. Someone able to do it may like to dupe here bug  565567, bug 633504, bug 691297 and bug 702478. Related but a bit different, there are also bug 298802 and bug 298803.
(In reply to Linas from comment #27)

If Mac OS X, because bug 938303 is fixed now, Command+Q is killed by "other shortcut assignment to Quit Firefox by OS" from Firefox 32, as it was possible unill Firefox 2. So, Mac OS X/Firefox user can freely press Command+W pretty safely. User can press Command+Q any time safely. Thus, severity of this bug will be changed to lower in Mac OS X after Firefox 32.
FYI.
Reason why all of bug 457973, bug 515395, bug 624881, bug 646362, bug 938303 were hidden and killed :
    All of these bugs were closed as dup of bug 429824.
In fact, session restore won't do the user any good if Firefox is running in Private Browsing mode. So the assumption that the warning can be turned off if we do session restores is inherently flawed.
(In reply to Ambrose Li from comment #31)
> In fact, session restore won't do the user any good if Firefox is running in
> Private Browsing mode. So the assumption that the warning can be turned off
> if we do session restores is inherently flawed.

Yes, that's true.  And Private Browsing mode is made still more vulnerable by a bug where the 'close multiple tabs' warning doesn't work - just filed as Bug 1096350, but an old issue.  Another bug (Bug 1096437) means no 'close multiple tabs' warning if you have a single non-private window.  Just for good measure, the old quit dialog long available via browser.showQuitWarning has finally been killed off.

Session Restore was a nice idea horribly implemented.  The answer was always a simple one: add support for multiple sessions, as suggested by me 3½ years ago (Bug 648081).  Not actually a huge undertaking.  A session is just a set of files, and if you can save and restore one you can accommodate 2 - or 20, come to that.
(In reply to Linas from comment #27)
> Listed on bug 565567, there is
> https://addons.mozilla.org/en-US/firefox/addon/always-ask/ as workaround.

There's also https://addons.mozilla.org/en-US/firefox/addon/warn-before-quit/ , they both work for me in Firefox 41 on Ubuntu 15.04

I have browser.showQuitDialog, browser.showQuitWarning, and browser.warnOnQuit all set to true in about:config. It seems _insane_ that Firefox doesn't do what I tell it.
See Also: → 1167998
While the add-ons work, this should be build-in to in the browser as it's highly annoying!
(In reply to wojtek from comment #34)

> this should be build-in to in the browser as it's highly annoying!

Stopped using Firefox, moved to Chromium.  That this issue should go nowhere for 6½ years was one issue of many behind my reason to switch.
For the Session Manager extension: 

Bug 26384 – Not compatible with firefox 57+?
<https://www.mozdev.org/bugs/show_bug.cgi?id=26384>
why is this taking a decade to fix?
Firefox warns on quit just fine if you have at least one active tab with an unsubmitted form. That dialog lets the user abort a quit. Could we use the same mechanism for this?
Flags: needinfo?(jgilbert)
If you'd like to implement what Paul is talking about in Comment 38 you can do this :

> The workaround that I've been using is to leave a tab pinned at all times with code that uses the "onbeforeunload" javascript function.
> 
> https://github.com/gene1wood/sandbox/blob/master/docs/prevent_browser_quitting.html
> 
> With this tab open and pinned it forces the browser to prompt before shutting down giving me a chance to click "Stay on page" and stop Firefox from exiting.

https://bugzilla.mozilla.org/show_bug.cgi?id=52821#c303
I don't know. Matt? Can you route this to someone to provide direction towards a solution, or establish WONTFIX?
Flags: needinfo?(jgilbert) → needinfo?(MattN+bmo)
Flags: needinfo?(MattN+bmo) → needinfo?(mdeboer)
After going through the thread and related bugs, it seems to make sense to have a confirmation dialogue when quitting the browser. Also it looks like a confirmation dialogue does appear in Windows/Linux when "closing"  multiple tabs and not when "quitting" which is confusing. There is no confirmation at all in Mac. I've attached a screenshot of the confirmation for when closing multiple tabs on Linux/Windows. This should also exist for Mac when quitting/closing.

Rational for having a confirmation even if we have session restore:
1. Session Restore doesn't restore dev tools. Accidentally quitting and losing progress.
2. When bandwidth is limited and you don't want to reload your tabs after accidentally quitting.
Attached image Linux_Windows_Close.png
Reference for above comment
To clarify this confirmation should exist when quitting the browser in all menus (not only when using shortcut keys)
Assignee: jgilbert → amlee
I've attached a spec for Preferences when Session Restore is Enabled and Disabled. The warning will not show up by default if Session Restore is enabled but I've added a pref that allows users to turn the warning message on when Session Restore is enabled if desired. 

NI Meridel to review the copy. Thanks!
Flags: needinfo?(mwalkington)
Attached image Preferences - Close Tab Warning.png (obsolete) —
Spec for Restore Previous Session Setting in Preferences
Prefs copy looks good and hasn't changed from what we have currently.

Edits to close warning dialogue:
-Numbers less than ten should be spelled out. So if the user is closing 2 - 9 tabs, spell out the number of tabs.
-Button copy should be capitalized
-The checkbox string should be in second person and consistent with language in Prefs

So, confirm close copy should be:

**Confirm close**
You are about to close seven tabs. Are you sure you want to continue?

Warn me when closing multiple tabs

Cancel  Close Tabs
Flags: needinfo?(mwalkington)
Great! Thanks Meridel. Can we reassign this bug to someone that can implement the changes? Currently this bug is assigned to me.
Blocks: 1438499
(In reply to Meridel from comment #46)
> Prefs copy looks good and hasn't changed from what we have currently.
> 
> Edits to close warning dialogue:
> -Numbers less than ten should be spelled out. So if the user is closing 2 -
> 9 tabs, spell out the number of tabs.

This is really annoying to implement, and requires localizers to provide translations for all these numbers. It also makes it harder to scan for the numbers when reading the warning. It's also not clear to me that any convention in (American?) English that you're using here applies in other languages (eg I grew up in the Netherlands and the rule I got taught was to write out numbers under 20, as well as any numbers divisible by 10 (under 100) or 100 (under 1000), etc.). So it would be "five thousand tabs" but "5341 tabs". I would be unsurprised if other languages used other conventions still. It also doesn't seem related to this bug. If you feel strongly that this is necessary, can you file a separate bug for this, maybe with more details about how that should work?

For the changes to the prefs:

(In reply to Amy Lee [:amylee] UX from comment #44)
> I've attached a spec for Preferences when Session Restore is Enabled and
> Disabled. The warning will not show up by default if Session Restore is
> enabled but I've added a pref that allows users to turn the warning message
> on when Session Restore is enabled if desired. 

I have a several concerns here.

First, now if users have session restore enabled there is 1 pref that governs both window closing and quitting. Unfortunately, there isn't a pref to turn on warnings for quit but not have warnings when closing multiple tabs otherwise. This would be desirable because it's easy to mis-hit cmd-q when trying to hit cmd-w (to close a tab). You have to try pretty hard to mis-hit cmd-shift-w (to close a window) or to close multiple tabs using the mouse. So I want a warning when quitting but not when closing a window, but I can't have that either today or with the implementation you're suggesting. That seems like something we should fix.

Second, users who are used to the current behavior with session restore enabled (esp. on Windows, where mis-hitting ctrl-shift-q is also not a major concern) will start getting warning dialogs when we implement the new behavior. That seems disruptive. But we can't turn off the warning everywhere because then they won't get a warning when closing a window, which they might have relied upon. Any thoughts about how to address that?
Flags: needinfo?(mwalkington)
Flags: needinfo?(amlee)
(In reply to :Gijs (Not available 3-19 Aug; he/him) from comment #48)
> (In reply to Meridel from comment #46)
> > Prefs copy looks good and hasn't changed from what we have currently.
> > 
> > Edits to close warning dialogue:
> > -Numbers less than ten should be spelled out. So if the user is closing 2 -
> > 9 tabs, spell out the number of tabs.
> 
> This is really annoying to implement, and requires localizers to provide
> translations for all these numbers. It also makes it harder to scan for the
> numbers when reading the warning. It's also not clear to me that any
> convention in (American?) English that you're using here applies in other
> languages (eg I grew up in the Netherlands and the rule I got taught was to
> write out numbers under 20, as well as any numbers divisible by 10 (under
> 100) or 100 (under 1000), etc.). So it would be "five thousand tabs" but
> "5341 tabs". I would be unsurprised if other languages used other
> conventions still. It also doesn't seem related to this bug. If you feel
> strongly that this is necessary, can you file a separate bug for this, maybe
> with more details about how that should work?
> 
> For the changes to the prefs:
> 
> (In reply to Amy Lee [:amylee] UX from comment #44)
> > I've attached a spec for Preferences when Session Restore is Enabled and
> > Disabled. The warning will not show up by default if Session Restore is
> > enabled but I've added a pref that allows users to turn the warning message
> > on when Session Restore is enabled if desired. 
> 
> I have a several concerns here.
> 
> First, now if users have session restore enabled there is 1 pref that
> governs both window closing and quitting. Unfortunately, there isn't a pref
> to turn on warnings for quit but not have warnings when closing multiple
> tabs otherwise. This would be desirable because it's easy to mis-hit cmd-q
> when trying to hit cmd-w (to close a tab). You have to try pretty hard to
> mis-hit cmd-shift-w (to close a window) or to close multiple tabs using the
> mouse. So I want a warning when quitting but not when closing a window, but
> I can't have that either today or with the implementation you're suggesting.
> That seems like something we should fix.

I think we are getting very specific for an issue that I think affects a small population of users. It starts to become overkill if we try to provide such specific checkbox options (ie. warn me when quitting, warn me when closing a window, etc.).  

> 
> Second, users who are used to the current behavior with session restore
> enabled (esp. on Windows, where mis-hitting ctrl-shift-q is also not a major
> concern) will start getting warning dialogs when we implement the new
> behavior. That seems disruptive. But we can't turn off the warning
> everywhere because then they won't get a warning when closing a window,
> which they might have relied upon. Any thoughts about how to address that?

The default settings of Session Restore would be what we have currently (not having warnings). The user can turn on the warning when closing multiple tabs in Prefs. If they accidentally close a window, it's less of a destructive action because Session Restore is enabled so if that does accidentally happen, their tabs are restored anyways when they reopen. Again, I believe this affects a small population of users. If they are that worried about accidental closing, they are given the option to turn the warnings back on.
Flags: needinfo?(amlee)
(In reply to Amy Lee [:amylee] UX from comment #49)
> (In reply to :Gijs (Not available 3-19 Aug; he/him) from comment #48)
> > Second, users who are used to the current behavior with session restore
> > enabled (esp. on Windows, where mis-hitting ctrl-shift-q is also not a major
> > concern) will start getting warning dialogs when we implement the new
> > behavior. That seems disruptive. But we can't turn off the warning
> > everywhere because then they won't get a warning when closing a window,
> > which they might have relied upon. Any thoughts about how to address that?
> 
> The default settings of Session Restore would be what we have currently (not
> having warnings).

That's not really what happens today. Today, there are warnings when closing windows, or using "close tabs to the right", or using "close other tabs" - but not when quitting, if and only if session restore is enabled.

Specifically, the preference you're suggesting we surface "under" Restore Previous Session already exists, and its default is true (ie warn when closing tabs). However, there are hardcoded checks for "does the user have session restore enabled?" in the code that avoid warning when quitting, irrespective of whether the "warn when closing tabs" checkbox is checked.

In your new design, we'd remove the hardcoded checks to ask session restore in the code that actually implements warning, and cosmetically we'd move the checkbox in about:preferences, and add some logic to tick/untick (and set/unset the pref) when the user toggles restoring their session. But that doesn't solve the conundrum here - the pref would be turned on by default, and we'd remove the checks, so then the user will start getting warnings.

We could have a one-time migration that turns off the "warn when closing tabs" pref for people who have session restore turned on, but that also seems like it'd potentially confuse users who would no longer see warnings when closing windows / multiple tabs (e.g. via "close tabs to the right") where they did see those before.

What would you prefer we do? Disable the warnings everywhere for session restore users, or accept that we'll now start warning session restore users who might not have expected this?


(In reply to Amy Lee [:amylee] UX from comment #49)
> I think we are getting very specific for an issue that I think affects a
> small population of users.

While I sympathise with the sentiment here, we already made a change in bug 1438499 (to show the same "closing multiple tabs" warning when quitting when session store is not enabled, removing a separate option to show an alternative dialog that people had to opt in to via about:config), and already people on Nightly are basically saying that that removed a "fail-safe":

(In reply to Kestrel from bug 1438499 comment #47)
> browser.showQuitWarning was a good fail-safe to avoid data loss but now that
> it has been removed a suitable replacement is needed, hopefully Bug 550559
> can be expedited to avoid a regression.

... because the "closing multiple tabs" warning on quit is enabled/disabled together with the same warning when closing windows/other/right-hand tabs, there's no way to force the quit warning to come up but not warn for closing windows / multiple tabs, and that's burning people (it seems).

(In reply to Amy Lee [:amylee] UX from comment #49)
>  It starts to become overkill if we try to provide
> such specific checkbox options (ie. warn me when quitting, warn me when
> closing a window, etc.).

FWIW, I think the only requirement here is quitting vs. everything else, and I think an about:config pref would be fine. I don't think this is going to be a slippery slope. Quitting is by its nature *much* more destructive than closing a single window, and as noted, easier to do by accident (esp. on macOS/Linux) than all the other ways of closing multiple tabs.

If you're convinced that we don't need to provide an option here, that works, but I think we need to tread carefully and deliberately here.
Flags: needinfo?(amlee)
@Gijs Okay, we can use the numerical version of numbers and NOT spell them out. 

**Confirm close**
You are about to close 7 tabs. Are you sure you want to continue?

Warn me when closing multiple tabs

Cancel  Close Tabs
Flags: needinfo?(mwalkington)
(In reply to :Gijs (Not available 3-19 Aug; he/him) from comment #50)
> (In reply to Amy Lee [:amylee] UX from comment #49)
> > (In reply to :Gijs (Not available 3-19 Aug; he/him) from comment #48)
> > > Second, users who are used to the current behavior with session restore
> > > enabled (esp. on Windows, where mis-hitting ctrl-shift-q is also not a major
> > > concern) will start getting warning dialogs when we implement the new
> > > behavior. That seems disruptive. But we can't turn off the warning
> > > everywhere because then they won't get a warning when closing a window,
> > > which they might have relied upon. Any thoughts about how to address that?
> > 
> > The default settings of Session Restore would be what we have currently (not
> > having warnings).
> 
> That's not really what happens today. Today, there are warnings when closing
> windows, or using "close tabs to the right", or using "close other tabs" -
> but not when quitting, if and only if session restore is enabled.
> 
> Specifically, the preference you're suggesting we surface "under" Restore
> Previous Session already exists, and its default is true (ie warn when
> closing tabs). However, there are hardcoded checks for "does the user have
> session restore enabled?" in the code that avoid warning when quitting,
> irrespective of whether the "warn when closing tabs" checkbox is checked.
> 
> In your new design, we'd remove the hardcoded checks to ask session restore
> in the code that actually implements warning, and cosmetically we'd move the
> checkbox in about:preferences, and add some logic to tick/untick (and
> set/unset the pref) when the user toggles restoring their session. But that
> doesn't solve the conundrum here - the pref would be turned on by default,
> and we'd remove the checks, so then the user will start getting warnings.
> 
> We could have a one-time migration that turns off the "warn when closing
> tabs" pref for people who have session restore turned on, but that also
> seems like it'd potentially confuse users who would no longer see warnings
> when closing windows / multiple tabs (e.g. via "close tabs to the right")
> where they did see those before.
> 
> What would you prefer we do? Disable the warnings everywhere for session
> restore users, or accept that we'll now start warning session restore users
> who might not have expected this?

I would say accept that we'll now start warning session restore users about quitting. They have the option on the warning message to disable it if they want to. I've changed the wording slightly in the preference to include "quitting" when enabling warning on closing multiple tabs. If a user quits the browser with multiple tabs opened they should get this warning as well cause technically they are still closing multiple tabs when quitting. I've attached an updated spec.
Flags: needinfo?(amlee)
See Also: → 1483935
I think :Gijs is a good mentor for this as well. I think we'd both happily take review and feedback requests!
Flags: needinfo?(mdeboer)
No longer blocks: 502908
How is it going? In the meantime I've installed an add on to restore tabs. :-(
Hope this is not going to be passed into the standard version of FF, because at the moment the problem is only in the Developer Edition.
This bug is going to hit the standard edition... as soon as it become v63... and there is only 1 number left :-/
I think when the bug will hit the standard edition, support.mozilla.org will explode :-)
PS: are you sure this bug is the same that hide the dialog "Do you want Firefox to save your tabs for the next time it starts?"?

Because I still see the question "You are about to close 2 tabs. Are you sure you want to continue?" but I don't see the other dialog that once allowed to save the tabs.

I've checked all the flag in about:config and nothing works anymore. The question "Do you want Firefox to save your tabs for the next time it starts?" is gone.
Amy, this bug is still assigned to you - is there more that needs to happen design-wise or is this ready to be implemented?
Flags: needinfo?(amlee)
(In reply to :Gijs (he/him) from comment #59)
> Amy, this bug is still assigned to you - is there more that needs to happen
> design-wise or is this ready to be implemented?

The design is ready to be implemented. Can you please reassign the bug to someone that implement?
Flags: needinfo?(amlee)
Assignee: amlee → gijskruitbosch+bugs
Keywords: uiwanted
Attachment #621206 - Attachment is obsolete: true
Attachment #594363 - Attachment is obsolete: true
Attachment #8995330 - Attachment is obsolete: true
Comment on attachment 9021147 [details]
Bug 550559 - also show close warnings when session restore is active, r?jaws

(In reply to Amy Lee [:amylee] UX from comment #61)
> (In reply to :Gijs (he/him) from comment #59)
> > Amy, this bug is still assigned to you - is there more that needs to happen
> > design-wise or is this ready to be implemented?
> 
> The design is ready to be implemented. Can you please reassign the bug to
> someone that implement?

OK. I attached a patch. Here are some builds to test:

Linux64: https://queue.taskcluster.net/v1/task/Zd3pnRF4QzatNAU9w4kgyw/runs/0/artifacts/public/build/target.tar.bz2

Mac: https://queue.taskcluster.net/v1/task/fNm10L8cQ_6MS4FuBly_Ag/runs/0/artifacts/public/build/target.dmg

Windows: https://queue.taskcluster.net/v1/task/TsxrJ8xgQe-pRSlpV7rgbg/runs/0/artifacts/public/build/target.zip

Can you let me know if this matches your expectations? Thank you!
Attachment #9021147 - Flags: ui-review?(amlee)
Comment on attachment 9021147 [details]
Bug 550559 - also show close warnings when session restore is active, r?jaws

Hi, 

I installed the mac version and I don't see an update to about:preferences under the Tabs section (“Warn you when quitting and closing multiple tabs”)
Flags: needinfo?(jaws)
(In reply to Amy Lee [:amylee] UX from comment #65)
> Comment on attachment 9021147 [details]
> Bug 550559 - also show close warnings when session restore is active, r?jaws
> 
> Hi, 
> 
> I installed the mac version and I don't see an update to about:preferences
> under the Tabs section (“Warn you when quitting and closing multiple tabs”)
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Amy Lee [:amylee] UX from comment #65)
> Comment on attachment 9021147 [details]
> Bug 550559 - also show close warnings when session restore is active, r?jaws
> 
> Hi, 
> 
> I installed the mac version and I don't see an update to about:preferences
> under the Tabs section (“Warn you when quitting and closing multiple tabs”)

The checkbox is only visible when the user opts out of the warning. This is the existing UX today. I'm happy to change it, but the comments there implied that this was intentional, with the idea that the dialogs themselves offer a checkbox to turn this off, at which point it can be turned back on in the preferences.

Do you think the checkbox should always be visible?
Flags: needinfo?(jaws)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(amlee)
(In reply to :Gijs (he/him) from comment #67)
> (In reply to Amy Lee [:amylee] UX from comment #65)
> > Comment on attachment 9021147 [details]
> > Bug 550559 - also show close warnings when session restore is active, r?jaws
> > 
> > Hi, 
> > 
> > I installed the mac version and I don't see an update to about:preferences
> > under the Tabs section (“Warn you when quitting and closing multiple tabs”)
> 
> The checkbox is only visible when the user opts out of the warning. This is
> the existing UX today.

Specifically, this happened in bug 1063625, and there's some discussion about it there.
(In reply to :Gijs (he/him) from comment #68)
> (In reply to :Gijs (he/him) from comment #67)
> > (In reply to Amy Lee [:amylee] UX from comment #65)
> > > Comment on attachment 9021147 [details]
> > > Bug 550559 - also show close warnings when session restore is active, r?jaws
> > > 
> > > Hi, 
> > > 
> > > I installed the mac version and I don't see an update to about:preferences
> > > under the Tabs section (“Warn you when quitting and closing multiple tabs”)
> > 
> > The checkbox is only visible when the user opts out of the warning. This is
> > the existing UX today.
> 
> Specifically, this happened in bug 1063625, and there's some discussion
> about it there.

Hi, 

I mean the warning text hasn't been updated to "Warn you when quitting and closing multiple tabs", right now it still reads as "Warn you when closing multiple tabs"
Flags: needinfo?(amlee)
(In reply to Amy Lee [:amylee] UX from comment #69)
> (In reply to :Gijs (he/him) from comment #68)
> > (In reply to :Gijs (he/him) from comment #67)
> > > (In reply to Amy Lee [:amylee] UX from comment #65)
> > > > Comment on attachment 9021147 [details]
> > > > Bug 550559 - also show close warnings when session restore is active, r?jaws
> > > > 
> > > > Hi, 
> > > > 
> > > > I installed the mac version and I don't see an update to about:preferences
> > > > under the Tabs section (“Warn you when quitting and closing multiple tabs”)
> > > 
> > > The checkbox is only visible when the user opts out of the warning. This is
> > > the existing UX today.
> > 
> > Specifically, this happened in bug 1063625, and there's some discussion
> > about it there.
> 
> Hi, 
> 
> I mean the warning text hasn't been updated to "Warn you when quitting and
> closing multiple tabs", right now it still reads as "Warn you when closing
> multiple tabs"

Uh, I don't really understand how that's possible. https://hg.mozilla.org/try/rev/b7b641a61f5e#l3.14 shows the change to the string - the old string is no longer present. I just checked the builds produced by try myself, and I can see the new string. The string identifier also changed, so I don't really understand how it's even possible for it to show the old string. Are you sure you ran the .app from the .dmg I linked? Can you try it with a new profile?
Flags: needinfo?(amlee)
Amy, have you had a chance to check the build again? These changes otherwise have r+ and are ready to land.
Comment on attachment 9021147 [details]
Bug 550559 - also show close warnings when session restore is active, r?jaws

Checked and looks good.
Flags: needinfo?(amlee)
Attachment #9021147 - Flags: ui-review?(amlee) → ui-review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/fd42da5a8e1a
also show close warnings when session restore is active, r=jaws,flod
https://hg.mozilla.org/mozilla-central/rev/fd42da5a8e1a
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Yay, thanks so much!
No longer blocks: 1506173
Depends on: 1506173
Depends on: 1506069
No longer depends on: 1506069
Keywords: feature
Gijs, could you request an addition to our release notes? Thanks!
Flags: needinfo?(gijskruitbosch+bugs)
Note: we're tweaking this in bug 1506173, so subject to change, but:

Release Note Request (optional, but appreciated)
[Why is this notable]: long-standing behavior changes subtly
[Affects Firefox for Android]: no
[Suggested wording]: The "You're closing multiple tabs" warning now applies to quitting Firefox with automatic session restore turned on.
[Links (documentation, blog post, etc)]: n/a
relnote-firefox: --- → ?
Flags: needinfo?(gijskruitbosch+bugs)
Added to 65 nightly release notes with this wording:
The "Warn you when quitting and closing multiple tabs" warning now applies to quitting Firefox with automatic session restore turned on

We are keeping the 'relnote?' flag set as this is for now a nightly only note and the final wording may be different.
Thanks a lot for fixing this at last!

I am gladly verifying that this is implemented in latest Nightly 65 on Linux (x86_64)

Build ID 	20181205102000
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:65.0) Gecko/20100101 Firefox/65.0
QA Whiteboard: [bugday-20181205]
Hi.

Does the fix not consider pinned tabs? I use multiple windows with many pinned tabs of websites I regularly use (mail, etc). Ctrl+Q only warns before quitting if there are at least 2 *unpinned* open tabs. Should it not consider pinned tabs in the count as well?

I'm using FF Developer edition 65.0b4 on Linux.

Thanks.
(In reply to sh.siddhartha from comment #81)
> Does the fix not consider pinned tabs? I use multiple windows with many
> pinned tabs of websites I regularly use (mail, etc). Ctrl+Q only warns
> before quitting if there are at least 2 *unpinned* open tabs. Should it not
> consider pinned tabs in the count as well?

I think so. Please file a follow-up bug.
Thanks for the reply. I opened https://bugzilla.mozilla.org/show_bug.cgi?id=1515061.
Depends on: 1515061
See Also: 1167998
Blocks: 1692205
No longer blocks: 1692205
You need to log in before you can comment on or make changes to this bug.