854926 - Firefox prompts a warning when closing private windows with multiple tabs open

Status: VERIFIED FIXED

(Firefox :: Private Browsing, defect)

Description

[Ioana (away)]

This issue is reproducible on Firefox 20 beta 7 (20130325214615), as well as on the 03/26 Aurora and Nightly. It most probably landed with the Per-Window PB (it reproduces back to a 04/12/2012 build).

STR:
1. Open a private window.
2. Open multiple tabs in that private window.
3. Close the private window.

Expected Results:
No warnings are displayed.

Actual Results:
The warning about closing multiple tabs is displayed.

Comment 1

[Josh Matthews [:jdm]]

I can't reproduce this in nightlies after bug 844561 landed.

Comment 2

[Josh Matthews [:jdm]]

Specifically, I'm on Linux. Can you give some platform details?

Comment 3

[Ioana (away)]

(In reply to Josh Matthews [:jdm] from comment #2)
> Specifically, I'm on Linux. Can you give some platform details?

I'm reproducing this issue on Windows 7 32bit and 64 bit, and on Ubuntu 12.10 32bit.

No reproducible on Mac OSX 10.7.5.

Comment 4

[Gabriel Chadwick]

This does not happen on Mozilla/5.0 (Windows NT 6.2; WOW64; rv:21.0) Gecko/20130331 Firefox/21.0

But on Mozilla/5.0 (Windows NT 6.2; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0 buildid 20130325214615 it is not fixed.

Comment 5

[Ioana (away)]

I just reproduced this on every try on:
Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20130331 Firefox/21.0
Mozilla/5.0 (X11; Linux i686; rv:22.0) Gecko/20130331 Firefox/22.0

Mozilla/5.0 (Windows NT 6.1; rv:21.0) Gecko/20130331 Firefox/21.0
Mozilla/5.0 (Windows NT 6.1; rv:22.0) Gecko/20130331 Firefox/22.0

Mozilla/5.0 (Windows NT 6.2; WOW64; rv:21.0) Gecko/20130331 Firefox/21.0
Mozilla/5.0 (Windows NT 6.2; WOW64; rv:22.0) Gecko/20130331 Firefox/22.0

Comment 6

[(no longer active)]

Can you please start with a clean profile and give us an exact set of steps to reproduce, including the URLs that you open in the private tabs?  Thanks!

Comment 7

[Ioana (away)]

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #6)
> Can you please start with a clean profile and give us an exact set of steps
> to reproduce, including the URLs that you open in the private tabs?  Thanks!

I mostly reproduced this on clean profiles, and I only do the exact steps in comment 0:

1. Open a private window  (Ctrl+Shift+P or file->New Private Window - worked with both)
2. Open multiple tabs in that private window (Ctrl+T or click the [+] button on the tabs bar - there's no need to load URLs in the tabs).
3. Close the private window (Click the [x] button).

Comment 8

[Alex Keybl [:akeybl]]

At the moment, it's not clear that this should track until users report it as a major nuisance. Once fixed, we can consider a low risk uplift nomination.

Comment 9

[Gabriel Chadwick]

(In reply to Ioana Budnar [QA] from comment #5)
> I just reproduced this on every try on:
> Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20130331 Firefox/21.0
> Mozilla/5.0 (X11; Linux i686; rv:22.0) Gecko/20130331 Firefox/22.0
> 
> Mozilla/5.0 (Windows NT 6.1; rv:21.0) Gecko/20130331 Firefox/21.0
> Mozilla/5.0 (Windows NT 6.1; rv:22.0) Gecko/20130331 Firefox/22.0
> 
> Mozilla/5.0 (Windows NT 6.2; WOW64; rv:21.0) Gecko/20130331 Firefox/21.0
> Mozilla/5.0 (Windows NT 6.2; WOW64; rv:22.0) Gecko/20130331 Firefox/22.0

I can't reproduce on todays aurora build. I maintain comment #4.

(In reply to Alex Keybl [:akeybl] from comment #8)
> At the moment, it's not clear that this should track until users report it
> as a major nuisance. Once fixed, we can consider a low risk uplift
> nomination.

This is the second time this regression occurs. People just won't come chiming in claiming that they got caught doing  whatever they where doing while trying to close a private browsing window. This should track. I don't understand how this regressed again while it was in the testsuite?

Comment 10

[(no longer active)]

Josh, can you take a look please?  Thanks!

Comment 11

[Nicolas Barbulesco]

Is a different behaviour expected in private mode ?

In normal mode, Firefox prompts the user when closing a window having several tabs. So, in private mode, Firefox has to prompt the user too. For the sake of consistency. The user is used to have this last chance. If Firefox closes the window at once *in private mode*, it - mostly badly - surprises the user. And predictability is the best friend of usability.

Comment 12

[(no longer active)]

(In reply to comment #11)
> Is a different behaviour expected in private mode ?
> 
> In normal mode, Firefox prompts the user when closing a window having several
> tabs. So, in private mode, Firefox has to prompt the user too. For the sake of
> consistency. The user is used to have this last chance. If Firefox closes the
> window at once *in private mode*, it - mostly badly - surprises the user. And
> predictability is the best friend of usability.

You don't want Firefox to pop up dialogs when you're trying to close your private window quickly when someone is passing by...

Comment 13

[Josh Matthews [:jdm]]

Ugh, I can now reproduce this on beta.

Comment 14

[Josh Matthews [:jdm]]

Attachment: Never request confirmation to close the last private browsing window.

No wonder it was intermittent to reproduce. browser-lastwindow-close-requested in nsBrowserGlue only pops up a confirmation box if warnOnQuit is true or your session will not be automatically restored. It looks safe to skip that notification if we're closing a private window.

Comment 15

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

Comment on attachment 732553 [details] [diff] [review]
Never request confirmation to close the last private browsing window.

Whether browser-lastwindow-close-requested/granted is fired or not shouldn't depend on the window's private browsing state. Seems like you should instead fix the browser-lastwindow-close-requested observer (i.e. nsBrowserGlue's _onQuitRequest when aQuitType=="lastwindow") to not prompt in this situation.

Comment 16

[Nicolas Barbulesco]

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #12)

> You don't want Firefox to pop up dialogs when you're trying to close your
> private window quickly when someone is passing by...

Yes, I want this dialog. I expect it, because in normal mode it comes. So if I want to close a window quickly I will quickly click the "OK" button of this dialog.

Anyway, a private window is not supposed to be invisible - it is not invisible -, it is supposed not to leave traces after it neither outside of it.

Anyway, if I have a private window to hide quickly when someone is passing by, I will not close it quickly, I will hide (minimize) it quickly.

This issue is a data loss issue. The person using Firefox is used to have this last chance dialog, so s/he will expect it, and... not have it. This would be bad.

Comment 17

[Josh Matthews [:jdm]]

Attachment: Never request confirmation to close the last private browsing window.

Comment 18

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

Comment on attachment 732904 [details] [diff] [review]
Never request confirmation to close the last private browsing window.

This looks fine for implementing the desired behavior, but given that this is marked as a regression (from per-window PB?), I wonder how this quit dialog used to be suppressed before.

(As an aside, I'm also not entirely sure I like this design choice - I've personally been annoyed by accidentally closing private windows before and losing my tabs without warning, and it's not clear to me that the "going to get caught need to close window quickly" use case is actually common enough to be worth the dataloss risk.)

Comment 19

[Josh Matthews [:jdm]]

The prompt was suppressed before by this code: http://hg.mozilla.org/mozilla-central/annotate/e53044984f1b/browser/components/nsBrowserGlue.js#l704

Comment 20

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

That code's still there, though. What was stopping the warnAboutClosingTabs call in the lastwindow case (which is what this patch is fixing, AIUI)?

Comment 21

[Josh Matthews [:jdm]]

I don't understand the question. Before per-window PB, there was a check of the global PB mode value and an early return. After per-window PB, it turned into a check to see whether all of the open windows were private instead, so we no longer early-returned.

Comment 22

[Josh Matthews [:jdm]]

The warnAboutClosingTabs call isn't the problem; the remainder of _onQuitRequest pops up a dialog asking if you really want to continue.

Comment 23

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

I don't understand. If aQuitType == "lastwindow" (which seems to be the problematic case given your last patch), and that window is a private browsing window, allWindowsPrivate should be true, and we should continue to return early.

Comment 24

[Josh Matthews [:jdm]]

allWindowsPrivate is determined by enumerating all open browser windows. In the case where you have a public window in addition to the private one, it is false.

Comment 25

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

In that case, we wouldn't be hitting the "lastwindow" case. Unless you're referring to a non-browser, non-private window? Or did your previous patch just not fix the issue?

Comment 26

[Josh Matthews [:jdm]]

Attachment: Never request confirmation to close the last private browsing window.

Hooray for synchronous communication.

Comment 27

[Dão Gottwald [:dao]]

Comment on attachment 733038 [details] [diff] [review]
Never request confirmation to close the last private browsing window.

isPBWindow ? true : gBrowser.warnAboutClosingTabs(true) is equivalent to isPBWindow || gBrowser.warnAboutClosingTabs(true).

Comment 28

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

Comment on attachment 733038 [details] [diff] [review]
Never request confirmation to close the last private browsing window.

r=me with dao's tweak

Comment 30

[Josh Matthews [:jdm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bc837398007e

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bc837398007e

Comment 32

[Josh Matthews [:jdm]]

Comment on attachment 733038 [details] [diff] [review]
Never request confirmation to close the last private browsing window.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): pb-per-window
User impact if declined: Inconsistent prompting behaviour when closing private windows, based on user profile preferences.
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): Risk-free. 
String or IDL/UUID changes made by this patch: None.

Comment 33

[bhavana bajaj [:bajaj]]

Comment on attachment 733038 [details] [diff] [review]
Never request confirmation to close the last private browsing window.

Low risk, avoids a unexpected warning prompt when user is trying to close a private window.

Requesting QA verification on beta/aurora once this lands.

Comment 34

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/05e799145245
https://hg.mozilla.org/releases/mozilla-beta/rev/218e76c7cad9

Comment 35

[Gabriel Chadwick]

Verified on aurora.

Comment 36

[Mitza]

Mozilla/5.0 (Windows NT 6.2; WOW64; rv:21.0) Gecko/20100101 Firefox/21.0

Verified as fixed on Firefox 21(Build ID:20130408165307).

Comment 37

[Mitza]

Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20100101 Firefox/21.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:21.0) Gecko/20100101 Firefox/21.0

Also verified on Linux and Mac.

Comment 40

[Ioana (away)]

Verified as fixed on Windows 7 64bit, Ubuntu 12.10 32bit and Mac OSX 10.7.5 with Firefox 21 beta 3 - 20130416200523.

Comment 42

[Maria Grazia Alastra [:mga]]

Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:23.0) Gecko/20130428 Firefox/23.0
Mozilla/5.0 (X11; Linux x86_64; rv:23.0) Gecko/20130428 Firefox/23.0

Verified as fixed on Windows 7 x64 and Ubuntu 12.04 x86_64 with latest Nightly 23.0a1 (Build ID: 20130429030926)

Comment 43

[Virgil Dicu [:virgil] [QA]]

In addition to comment 42, verified on Mac OS 10.8 - latest Nightly using steps from comment 0.

Comment 45

[Ioana (away)]

Verified as fixed on Firefox 22 beta 1: 20130514181517.
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20100101 Firefox/22.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:22.0) Gecko/20100101 Firefox/22.0
Mozilla/5.0 (X11; Linux i686; rv:22.0) Gecko/20100101 Firefox/22.0

Comment 46

[Ben]

Interesting to discover the issue of data loss for one group of users is overridden if another group fears getting caught on dodgy sites by their bosses!  (Presumably no one's told them their admin will pick up on that stuff regardless and Private Browsing's giving them a false sense of security.)

I have added Bug 1097149 for a separate, off-by-default warning that a user will be able to enable to help protect their Private Browsing session.  I would encourage those above unhappy with the current state to support it to increase the chances of it actually happening.

Comment 47

[Nicolas Barbulesco]

(In reply to Ben from comment #46)

> Interesting to discover the issue of data loss for one group of users is
> overridden if another group fears getting caught on dodgy sites by their
> bosses!  (Presumably no one's told them their admin will pick up on that
> stuff regardless and Private Browsing's giving them a false sense of
> security.)

I agree with Ben. Data loss is more important than hide-and-seek. In addition, I expect the dialog, for these 2 reasons : 

1) Consistency. In normal mode, Firefox asks the question. So I am used to having the question. Not having the question in private mode would be inconsistent, so would surprise me.

2) Doing what we say. Firefox has a check box saying "Warn me when I attempt to close multiple tabs". As long as this check box remains ticked, Firefox says that he will warn me. That's why I expect to be warned.