The default bug view has changed. See this FAQ.

Closing multiple tabs in the last browser window with Private Browsing enabled yields no warning regardless of preference setting

RESOLVED FIXED in Firefox 16

Status

()

Firefox
Private Browsing
--
major
RESOLVED FIXED
8 years ago
4 years ago

People

(Reporter: tomhuff, Assigned: qheaden)

Tracking

({dataloss, regression})

Trunk
Firefox 16
dataloss, regression
Points:
---

Firefox Tracking Flags

(firefox10 affected, firefox11 affected, firefox12 affected, firefox13 affected, firefox-esr10 affected, status1.9.1 ?)

Details

(Whiteboard: [fixed by bug 722994])

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5

When private browsing is enabled, Firefox 3.5 will no longer warn you when closing multiple tabs regardless of the preference setting.

Reproducible: Always

Steps to Reproduce:
1. In the tabs preference, select "Warn me when closing multiple tabs".
2. In the privacy preference, select "Automatically start Firefox in a private browsing session.
3. Close and restart Firefox. Open multiple tabs and choose to close Firefox completely from the menu bar.
Actual Results:  
Firefox will not warn you when closing multiple tabs while using the private browsing preference.

Expected Results:  
Firefox should warn you when closing multiple tabs regardless of whether or not the private browsing feature is in use.
(Reporter)

Comment 1

8 years ago
With the private browsing feature not in use, Firefox warns you as expected when closing multiple tabs.
Version: unspecified → 3.5 Branch
This is a regression from the fix in bug 468565: we still need to display the tabbrowser warning prompt when closing the last window inside the private browsing window, because the browser glue prompt never gets shown inside the private browsing mode.

Taking.  Also requesting wanted1.9.1.x since this could be considered a dataloss issue: if a user unintentionally closes her last window inside the PB mode and the browser doesn't prompt here even though it's required to by the preferences, the user loses her tabs and all the work she's done on them with no way of getting them back at all.
Assignee: nobody → ehsan.akhgari
Blocks: 468565
Severity: normal → major
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: wanted1.9.1.x?
Keywords: dataloss, regression
OS: Mac OS X → All
Hardware: x86 → All
Summary: Closing multiple tabs with Private Browsing enabled yields no warning regardless of preference setting → Closing multiple tabs in the last browser window with Private Browsing enabled yields no warning regardless of preference setting
Version: 3.5 Branch → Trunk
Note to self: OR the condition on <http://mxr.mozilla.org/mozilla-central/source/toolkit/content/globalOverlay.js#19> with the private browsing mode current status.

Updated

8 years ago
Flags: in-litmus?
Created attachment 388086 [details] [diff] [review]
Patch (v1)
Attachment #388086 - Flags: review?(enndeakin)

Comment 5

8 years ago
I'm not following this patch. What happens when private browsing is enabled and only one tab is open? What happens on Mac (which doesn't check the state)?
(Reporter)

Comment 6

8 years ago
Neil,

I don't understand the one tab question? This bug report was created because no warning is issued when closing a browser session with multiple tabs open while in private browsing mode, regardless of the fact that the preference setting for warning when closing multiple tabs is set.

As for the platform, I am using a Mac running OS X v10.5.7 and Firefox v3.5
(Reporter)

Comment 7

8 years ago
Sorry, just realized like an idiot you were speaking of the code in the patch! duh!
(In reply to comment #5)
> I'm not following this patch.

Let me clarify.  Normally, closeWindow doesn't prompt when only one window is open, because according to the user prefs, the browser glue code prompts whether to save the session or not.  We disabled that prompt from the browser glue code inside the private browsing mode because it didn't make any sense (i.e., no session was being saved anyway).  This caused the additional problem that this bug is about: now when closing a window with multiple tabs inside the PB mode, if it's not the last window, the tab close warning prompt gets shown like normal.  If that window is the only one remaining, closeWindow refrains from showing the prompt in hopes that the browser glue code shows its own prompt, which is doesn't, so effectively no prompt gets shown.  I merely make sure that if the PB mode is active, the number of windows doesn't affect this decision, because the browser glue code won't generate a prompt anyway.

> What happens when private browsing is enabled and
> only one tab is open?

Nothing special, no prompt is displayed.

> What happens on Mac (which doesn't check the state)?

I don't have a Mac, but by reading the code, Mac is not affected by thing bug, and the fix doesn't apply to it either, because a Mac app may remain open even if all of its windows have been closed.  Marcia, can you please confirm?

Updated

8 years ago
Attachment #388086 - Flags: review?(enndeakin) → review+

Updated

8 years ago
status1.9.1: --- → needstriage
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/89bf34c42f88
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.6a1

Updated

8 years ago
Attachment #388086 - Flags: approval1.9.1.2?

Updated

8 years ago
Duplicate of this bug: 505117
>Firefox should warn you when closing multiple tabs regardless of whether or not
>the private browsing feature is in use.

Just to confirm, this makes sense for the state where Firefox is in always on private browsing mode.  But in the case of a temporary private browsing session, the user may want to close the window very quickly, so we should place a dialog in their way (bug 468565).

Comment 12

8 years ago
(In reply to comment #11)
> >Firefox should warn you when closing multiple tabs regardless of whether or not
> >the private browsing feature is in use.
> 
> Just to confirm, this makes sense for the state where Firefox is in always on
> private browsing mode.  But in the case of a temporary private browsing
> session, the user may want to close the window very quickly, so we should place
> a dialog in their way (bug 468565).

It makes sense also to grey out options that cancel each other
(In reply to comment #11)
> >Firefox should warn you when closing multiple tabs regardless of whether or not
> >the private browsing feature is in use.
> 
> Just to confirm, this makes sense for the state where Firefox is in always on
> private browsing mode.  But in the case of a temporary private browsing
> session, the user may want to close the window very quickly, so we should place
> a dialog in their way (bug 468565).

So, do you suggest to modify the patch in this bug to that effect?

Comment 14

8 years ago
(In reply to comment #13)
> (In reply to comment #11)
> > >Firefox should warn you when closing multiple tabs regardless of whether or not
> > >the private browsing feature is in use.
> > 
> > Just to confirm, this makes sense for the state where Firefox is in always on
> > private browsing mode.  But in the case of a temporary private browsing
> > session, the user may want to close the window very quickly, so we should place
> > a dialog in their way (bug 468565).
> 
> So, do you suggest to modify the patch in this bug to that effect?

I suggest this elusive patch to include also the greying out of options that cancel each other out....

This is not a new bug and yet it has not been fixed in 5 releases of firefox or more.
(In reply to comment #14)
> I suggest this elusive patch to include also the greying out of options that
> cancel each other out....

Can you please elaborate what you mean here?
>So, do you suggest to modify the patch in this bug to that effect?

Yeah, in the case of always on private browsing there is a concern of potential dataloss.  In the case of a temporary private browsing session, letting the user quiet very quickly I think takes priority, so we shouldn't display the dialog.

>> I suggest this elusive patch to include also the greying out of options that
>> cancel each other out....
>
>Can you please elaborate what you mean here?

I'm afraid I'm not following that either, if we do display the dialog for always on private browsing mode, then the options aren't really canceling each other out anymore.  For a temporary session, the options don't really apply.
Comment on attachment 388086 [details] [diff] [review]
Patch (v1)

Apparently this patch doesn't have ui-r and needs it as per previous comments.

Alex: I understand what you're saying; quick exit of PB mode is hindered by this approach, and users perhaps want that. At the same time, I don't think we want to vary to wildly between "always PB" and "instance PB" modes as it makes testing and predicting behaviour pretty difficult.

My recommendation would be to not make any changes until we've heard more user feedback on this issue. It's unclear to me that this is a problem in need of a fix, or if we accidentally made the right design choice, here!
Attachment #388086 - Flags: approval1.9.1.2?
(Reporter)

Comment 18

8 years ago
Mike:

I would have to comment that if this is going to be a design feature of Firefox, and hence not be patched, that the following be changed to more logically represent the preferences:

In Firefox>Preferences>Privacy if "Automatically start Firefox in a private browsing session" is checked, then the condition of that check should be that it grays out the "Warn me when closing multiple tabs" option located in Firefox>Preferences>Tabs.

I don't quite understand the speed issue when closing a private browsing session? If someone is looking at porn at work, they could simply Command>Tab>Q to quit Firefox as fast as they could use the mouse on a Mac. And if using Windows, then make use of the Windows Key>M to minimize all Windows instantly.

With that in mind, and if the patch is going to be thrown out in favor of classifying the current actions as a design feature, then the option above to gray out closing multiple tabs when in private browsing should definitely be invoked to logically make more since out of the application preferences, and behavior thereafter.

Any thoughts?

Comment 19

8 years ago
To all the patchers.

I agree with Mike and that has been my line of reasoning all along, your so called "design feature" is indeed a bug, and should be handled by the way mike described it and I posted on my original bug report which you attavhed to this instead and yet has not been fixed, but rather just left to age with time passing.
(In reply to comment #17)
> Alex: I understand what you're saying; quick exit of PB mode is hindered by
> this approach, and users perhaps want that. At the same time, I don't think we
> want to vary to wildly between "always PB" and "instance PB" modes as it makes
> testing and predicting behaviour pretty difficult.
> 
> My recommendation would be to not make any changes until we've heard more user
> feedback on this issue. It's unclear to me that this is a problem in need of a
> fix, or if we accidentally made the right design choice, here!

By "not making any changes", do you mean to back this bug out so that we get the same behavior as 1.9.1 (no warning when closing multiple tabs in both always PB and instance PB modes), leaving things as they are (no warning when closing multiple tabs in both always PB and instance PB modes on 1.9.1, and the usual close multiple tabs warning on trunk in both always PB and instance PB modes), or incorporating Alex's suggestion in comment 16 (no warning when closing multiple tabs in both always PB and instance PB modes on 1.9.1, the usual close multiple tabs warning on trunk in the always PB mode, and no warning on trunk in the instance PB mode)?

Sorry if this is exceedingly confusing. :-)
Flags: wanted1.9.1.x?
>do you mean to back this bug out so that we get
>the same behavior as 1.9.1

I'm a little confused on what the current and patched behavior is, but here is the intended proposed behavior:

PB instance: no close multiple tab warning (in case the user needs to close the browser window quickly

PB always on mode: same behavior as normally running Firefox, with the close multiple tab warning.
Re-opening to implement the behavior Alex described in comment 21.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Created attachment 392900 [details] [diff] [review]
Patch (v2)

Requesting review from Dao on browser/ part and from enndeakin on toolkit/ part.
Attachment #388086 - Attachment is obsolete: true
Attachment #392900 - Flags: review?(enndeakin)
Attachment #392900 - Flags: review?(dao)
Comment on attachment 392900 [details] [diff] [review]
Patch (v2)

This implements comment 21.
Attachment #392900 - Flags: ui-review?(faaborg)
Comment on attachment 392900 [details] [diff] [review]
Patch (v2)

Thanks, sorry about all of the confusion.
Attachment #392900 - Flags: ui-review?(faaborg) → ui-review+

Updated

8 years ago
Attachment #392900 - Flags: review?(dao) → review+
Comment on attachment 392900 [details] [diff] [review]
Patch (v2)

>diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js

> #ifdef XP_MACOSX
>   // OS X doesn't quit the application when the last window is closed, but keeps
>   // the session alive. Hence don't prompt users to save tabs, but warn about
>   // closing multiple tabs.
>   return gBrowser.warnAboutClosingTabs(true);
> #else
>-  return true;
>+  // If we're inside auto-started private browsing mode, always warn
>+  // about closing multiple tabs (bug 502307).
>+  if (gPrivateBrowsingUI.privateBrowsingEnabled &&
>+      gPrivateBrowsingUI.autoStarted)
>+    return gBrowser.warnAboutClosingTabs(true);
>+  else
>+    return true;
> #endif
> }

nit: remove the else after return.

Maybe also consider re-organizing the code like this:

> #ifdef XP_MACOSX
>   // On OS X, always warn about closing multiple tabs, as closing the
>   // last window won't quit the application.
> #else
>   // If we're inside auto-started private browsing mode, always warn
>   // about closing multiple tabs (bug 502307).
>   if (gPrivateBrowsingUI.privateBrowsingEnabled &&
>       gPrivateBrowsingUI.autoStarted)
> #endif
>     return gBrowser.warnAboutClosingTabs(true);
> 
>   return true;

Updated

8 years ago
Attachment #392900 - Flags: review?(enndeakin) → review+
Comment on attachment 388086 [details] [diff] [review]
Patch (v1)

>+  var inPrivateBrowsing = false;
>+  try {
>+    var pbSvc = Components.classes["@mozilla.org/privatebrowsing;1"]
>+                          .getService(Components.interfaces.nsIPrivateBrowsingService);
>+    inPrivateBrowsing = pbSvc.privateBrowsingEnabled;
>+  } catch(e) {
>+    // safe to ignore
Doesn't help with the strict JS warning though.
Created attachment 394516 [details] [diff] [review]
Fix strict JS warning

(Also fixes the resulting XPConnect assertion caused by generating a console message while the JS console is closing.)
Attachment #394516 - Flags: review?(enndeakin)
Created attachment 394546 [details] [diff] [review]
Patch (v3)

This patch incorporates Dao's comment, and also applies Neil's patch to the new version.  Re-requesting Neil's review request from enndeakin.
Attachment #392900 - Attachment is obsolete: true
Attachment #394516 - Attachment is obsolete: true
Attachment #394546 - Flags: review?(enndeakin)
Attachment #394516 - Flags: review?(enndeakin)

Updated

8 years ago
Attachment #394546 - Flags: review?(enndeakin) → review+

Comment 30

8 years ago
please realise that its not just private browsing which causes this behaiviour history switch off causes same issue. Not only that the warning check boxes should be cross cancelling greing out the dependants settings to behave as expected in such events, its not just a question of pumping out a warning the UI's respective settings must IMO respect this and be disabled for ease of understanding.
http://hg.mozilla.org/mozilla-central/rev/405a715a4d81
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 3.6a1 → Firefox 3.7a1
(In reply to comment #30)
> please realise that its not just private browsing which causes this behaiviour
> history switch off causes same issue. Not only that the warning check boxes
> should be cross cancelling greing out the dependants settings to behave as
> expected in such events, its not just a question of pumping out a warning the
> UI's respective settings must IMO respect this and be disabled for ease of
> understanding.

If by "history switch off" you mean the "never remember history" option, it's merely an always-on private browsing mode.

Updated

8 years ago
Attachment #394546 - Flags: approval1.9.2?
Attachment #394546 - Flags: approval1.9.1.3?
I've backed this out due to the test failures on windows and linux:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1250665586.1250671232.12054.gz&fulltext=1
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1250665586.1250671250.12070.gz&fulltext=1
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 394546 [details] [diff] [review]
Patch (v3)

Failures:

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_248970_a.js | outside private browsing - sessionStore.js timestamp has not changed - Got 1250671929086, expected 1250671929102
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_354894.js | Timed out
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_394759_privatebrowsing.js | Recently Closed Windows are removed when entering Private Browsing - Got 1, expected 0
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_394759_privatebrowsing.js | Recently Closed Windows data is cleared when entering Private Browsing - Got [{"tabs":[{"entries":[{"url":"about:config","ID":226,"owner_b64":"SmIS26zLEdO3ZQBgsLbOywAAAAAAAAAAwAAAAAAAAEY=","scroll":"0,0","formdata":{"#textbox":""}}],"index":1,"attributes":{},"_formDataSaved":true},{"entries":[],"attributes":{}}],"selected":1,"_closedTabs":[],"extData":{"bug 394759 Non-PB":"unik1250671968045"},"_hosts":{},"width":994,"height":986,"screenX":4,"screenY":4,"sizemode":"normal","title":"about:config"}], expected []
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_394759_privatebrowsing.js | The correct number of recently closed windows were restored when exiting PB mode - Got 2, expected 1
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_394759_privatebrowsing.js | The data associated with the recently closed window was restored when exiting PB mode
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_448741.js | data contains our value?
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_448741.js | found and removed the specific tab value
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_448741.js | ready to check the cleaned state?
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/fuel/test/browser_Browser.js | Timed out
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/docshell/test/browser/browser_bug134911.js | Timed out
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/docshell/test/browser/browser_bug92473.js | Timed out
Attachment #394546 - Flags: approval1.9.2?
Attachment #394546 - Flags: approval1.9.1.3?

Updated

7 years ago
Duplicate of this bug: 566052

Comment 36

6 years ago
is this ever going to be fixed?

Updated

6 years ago
Duplicate of this bug: 638737

Comment 38

6 years ago
Reported: 2009-07-03
When it will be fixed???!!!
Have you tested the latest FF4 beta to see if its been fixed by another bug?

Updated

6 years ago
Duplicate of this bug: 650734

Comment 41

5 years ago
Reproduced:
Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.2.27) Gecko/20120216 Firefox/3.6.27
Mozilla/5.0 (X11; Linux x86_64; rv:10.0.2) Gecko/20100101 Firefox/10.0.2
Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (X11; Linux x86_64; rv:12.0a2) Gecko/20120227 Firefox/12.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120227 Firefox/13.0a1
status-firefox-esr10: --- → affected
status-firefox10: --- → affected
status-firefox11: --- → affected
status-firefox12: --- → affected
status-firefox13: --- → affected
Josh, do you know any contributors who could work on this?  This requires a bit of front-end hacking experience.  Basically someone needs to take the old patch, see what it breaks these days and figure out why...
(Assignee)

Comment 43

5 years ago
@Ehsan

I'm assigning myself to this one. I'll try the patch, see if it breaks anything, and try to fix any broken tests.

I will take this little pest of a bug off of your shoulders. :)
Assignee: ehsan → qheaden
This bug will be fixed by bug 722994.
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed by bug 722994]

Updated

5 years ago
Target Milestone: Firefox 3.7a1 → Firefox 16

Comment 45

4 years ago
Um, bad news folks: I think this bug still exists (OS X 10.8.2). In v19.0.2 if you have two tabs open in private browsing mode, cmd+q will close without prompting (I tested with TabMixPlus disabled and confirmed all warnings were set to true in about:config, including ShowQuitWarning).

The error also occurs in v20.0 (which I just upgraded to): if all you have open is one private-browsing window with two tabs, it will close without warning (undesired behaviour)

Workaround: luckily in 20.0, I see that private browsing opens a new window and keeps the non-private browsing window open (this is really cool and I applaud everyone who worked to make that happen!). One tab in a private browsing window and one tab in a non-private browsing window will force the quit warning to display (desired behaviour).

I haven't tested in safe mode etc, because the new private-browsing handling will make this error sufficiently unlikely for me.

- RG>

Updated

4 years ago
Flags: in-litmus?
You need to log in before you can comment on or make changes to this bug.