816524 - Do not store information about closed private windows

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[(no longer active)]

Providing the ability to undo closed tabs in private windows is fine, but providing the ability to reopen closed windows seems risky, since by pressing Cmd+Shift+N, the user can see what the previous user was browsing, and we also show this information in the History menu.  So, we should not keep any information about the closed private windows.

Comment 1

[Josh Matthews [:jdm]]

Do we want the ability to reopen closed private windows while other private windows are open?

Comment 2

[(no longer active)]

(In reply to comment #1)
> Do we want the ability to reopen closed private windows while other private
> windows are open?

No, I don't think so.  We don't want to support reopening some of the times and not the others.

Comment 3

[Andres Hernandez [:andreshm]]

Attachment: Patch v1

With this patch, the browser_394759_privatebrowsing.js test fails. However, it should be migrated, since is based in the global PB. Is there a bug for that?  

http://mxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/test/browser_394759_privatebrowsing.js

Comment 4

[(no longer active)]

(In reply to Andres Hernandez [:andreshm] from comment #3)
> Created attachment 687271 [details] [diff] [review]
> Patch v1
> 
> With this patch, the browser_394759_privatebrowsing.js test fails. However,
> it should be migrated, since is based in the global PB. Is there a bug for
> that?  
> 
> http://mxr.mozilla.org/mozilla-central/source/browser/components/
> sessionstore/test/browser_394759_privatebrowsing.js

No, can you please do that here, so that when we land this patch the test passes?  Thanks!

Comment 5

[(no longer active)]

Comment on attachment 687271 [details] [diff] [review]
Patch v1

LGTM, but I'd like Tim to look at it.

Comment 6

[(no longer active)]

On the second thought, I'd like to land this sooner in order for it to go into the hands of our testers sooner.

I filed bug 817472 about the test in question.  Since this patch will turn Birch orange, I'd appreciate if you can please prioritize that test, Andres.  Thanks!

Comment 7

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/0902b7e89928

Comment 8

[Josh Matthews [:jdm]]

This change caused many sessionstore failures on birch; ehsan, it should perhaps be backed out.

Comment 9

[Josh Matthews [:jdm]]

https://hg.mozilla.org/mozilla-central/rev/077e43a8668b

Backed out. We should be able to get birch green, and then it will be easier to catch similar failures in the future.

Comment 10

[(no longer active)]

(In reply to comment #9)
> https://hg.mozilla.org/mozilla-central/rev/077e43a8668b
> 
> Backed out. We should be able to get birch green, and then it will be easier to
> catch similar failures in the future.

Thanks, Josh, I concur.

Comment 11

[(no longer active)]

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