Closed Bug 847959 Opened 7 years ago Closed 7 years ago

Consider making RecentWindow return private windows even when public is desired in perma-PB mode

Categories

(Firefox :: Private Browsing, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 22
Tracking Status
firefox20 --- fixed
firefox21 --- fixed

People

(Reporter: jdm, Assigned: jdm)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

This would sidestep any future issues where we only want public windows when the public/private distinction actually matters.
Yeah I think we should do this.
Comment on attachment 722257 [details] [diff] [review]
Make RecentWindow return any browser window regardless of requested privacy state when perma-PB mode is enabled.

>     function isSuitableBrowserWindow(win) {
>       return (!win.closed &&
>               win.toolbar.visible &&
>-              (!checkPrivacy ||
>+              (PrivateBrowsingUtils.permanentPrivateBrowsing ||
>+               !checkPrivacy ||
>                PrivateBrowsingUtils.isWindowPrivate(win) == aOptions.private));

make this:

  !checkPrivacy ||
  PrivateBrowsingUtils.permanentPrivateBrowsing ||
  ...

i.e. put the cheapest check first
OS: Linux → All
Hardware: x86_64 → All
Attachment #722257 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/a60d1c61af47
Assignee: nobody → josh
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
Comment on attachment 722257 [details] [diff] [review]
Make RecentWindow return any browser window regardless of requested privacy state when perma-PB mode is enabled.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: Potential for more undiscovered edge-case bugs when users use permanent private browsing mode.
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): No risk.
String or UUID changes made by this patch: None
Attachment #722257 - Flags: approval-mozilla-beta?
Attachment #722257 - Flags: approval-mozilla-aurora?
(In reply to Josh Matthews [:jdm] from comment #6)
> User impact if declined: Potential for more undiscovered edge-case bugs when
> users use permanent private browsing mode.

We're trying to suss out a known user impact here - is an example that within private browsing, recently closed tabs wouldn't be listed?
No. This patch does not fix any identified open issues at this point; it is a belt and suspenders patch that would have avoided bug 843247.
Comment on attachment 722257 [details] [diff] [review]
Make RecentWindow return any browser window regardless of requested privacy state when perma-PB mode is enabled.

We'll take a bit of risk now rather than a black eye post-release, given your risk evaluation. Thanks jdm.
Attachment #722257 - Flags: approval-mozilla-beta?
Attachment #722257 - Flags: approval-mozilla-beta+
Attachment #722257 - Flags: approval-mozilla-aurora?
Attachment #722257 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.