Places UI makes decisions based on global Private Browsing state

RESOLVED FIXED in Firefox 19

Status

()

Firefox
Bookmarks & History
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: jdm, Assigned: andreshm)

Tracking

unspecified
Firefox 19
x86
Mac OS X
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [appcoast])

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
The global state is going away. Either the UI should not change based on the status, and the importing (http://mxr.mozilla.org/mozilla-central/source/browser/components/places/content/places.js#1405) should happen in such a way that no information is leaked, or there needs to be some window-level status that reflects the presence of PB docshells in a window.
(Reporter)

Comment 1

7 years ago
http://mxr.mozilla.org/mozilla-central/source/browser/components/places/content/controller.js also uses PlacesUIUtils.privateBrowsing for these sorts of decisions.
(Reporter)

Comment 2

6 years ago
Marco, do you know why we hide "Forget About This Site" and "Import From Another Browser" when in PB mode? Do you think we could just always allow those operations?

Comment 3

6 years ago
(In reply to Josh Matthews [:jdm] from comment #2)
> Marco, do you know why we hide "Forget About This Site" and "Import From
> Another Browser" when in PB mode? Do you think we could just always allow
> those operations?

See bug 463607 for the former, and bug 464736 for the latter.  Basically we disabled these UIs because there was no easy way to make them work in the global private browsing mode.  But they can both be re-enabled when we switch to per-window PB.

The thing which sucks here is that we can't land a patch here until everything related is ready, which makes me think whether we should add an --enable-per-winodw-pb build time option and hide this kind of stuff behind that (and turn it on by default when per-window PB is ready).
yes, anything that removes information and doesn't store information automatically can be re-enabled once we don't use anymore the global PB status.

Updated

6 years ago
Assignee: nobody → chrislord.net
Depends on: 769460

Updated

6 years ago
Blocks: 797256

Comment 5

6 years ago
We don't need any special handling here, so the fix is really simple, just take all of the mentions of gPrivateBrowsingListener plus its definition and put them behind #ifndef MOZ_PER_WINDOW_PRIVATE_BROWSING chunks.
Assignee: chrislord.net → nobody
Whiteboard: [appcoast]
(Assignee)

Comment 6

6 years ago
Created attachment 674798 [details] [diff] [review]
Patch v1
Assignee: nobody → andres
Status: NEW → ASSIGNED
Attachment #674798 - Flags: review?(ehsan)

Comment 7

6 years ago
Comment on attachment 674798 [details] [diff] [review]
Patch v1

Review of attachment 674798 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #674798 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/0f22d98c98b0
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
You need to log in before you can comment on or make changes to this bug.