Closed Bug 816524 Opened 7 years ago Closed 7 years ago

Do not store information about closed private windows

Categories

(Firefox :: Session Restore, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 20

People

(Reporter: ehsan, Assigned: andreshm)

References

Details

(Whiteboard: [appcoast])

Attachments

(1 file)

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.
Do we want the ability to reopen closed private windows while other private windows are open?
(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.
Assignee: nobody → andres
Attached patch Patch v1Splinter Review
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
Attachment #687271 - Flags: review?(ehsan)
(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 on attachment 687271 [details] [diff] [review]
Patch v1

LGTM, but I'd like Tim to look at it.
Attachment #687271 - Flags: review?(ehsan) → review?(ttaubert)
Attachment #687271 - Flags: review?(ttaubert) → review+
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!
Depends on: 817472
https://hg.mozilla.org/mozilla-central/rev/0902b7e89928
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
This change caused many sessionstore failures on birch; ehsan, it should perhaps be backed out.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
(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.
https://hg.mozilla.org/mozilla-central/rev/a938ae37b4ba
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Duplicate of this bug: 818601
You need to log in before you can comment on or make changes to this bug.