If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

WindowsJumpLists.jsm uses global Private Browsing state to make decisions

VERIFIED FIXED in Firefox 20

Status

()

Firefox
Shell Integration
VERIFIED FIXED
6 years ago
4 years ago

People

(Reporter: jdm, Assigned: andreshm)

Tracking

unspecified
Firefox 20
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [appcoast])

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
The global state is going away. If possible, the state should be queried on a docshell by docshell basis, or we might need to create a window-level PB status.
(Reporter)

Updated

5 years ago
OS: Mac OS X → Windows 7
(Reporter)

Comment 1

5 years ago
This will need a bit of thought - I don't use Windows, so I'm not entirely certain what the jumplists look like/act like. Jim, do you have any thoughts about how the current task of entering/exiting PB mode should change when the PB mode becomes per-window?

Comment 2

5 years ago
(In reply to Josh Matthews [:jdm] (travelling until June 25th) from comment #1)
> This will need a bit of thought - I don't use Windows, so I'm not entirely
> certain what the jumplists look like/act like. Jim, do you have any thoughts
> about how the current task of entering/exiting PB mode should change when
> the PB mode becomes per-window?

I think we should do away with the "enter pb mode" option completely, and add a new "Open new private browsing window" option below "Open new window".
Depends on: 568816

Comment 3

5 years ago
Changes should be pretty minor - 

http://mxr.mozilla.org/mozilla-central/source/browser/modules/WindowsJumpLists.jsm#127

some new strings and we get rid of the hiding feature added by bug 601255. Is there a command line arg for "open a new private browsing window"? Currently this jl option uses '-private-toggle'.
I think this is sort of one of the last pieces that needs to go in, when we're ready to switch the UI.

I think -private-toggle should go away completely, and -private should open a new window in PB mode.
Assignee: nobody → chrislord.net
Depends on: 769460
Blocks: 797256
The first thing that needs to be done here is to add a new command which opens a new private browsing window.  In order to do that, we need to put the private-toggle command here <http://mxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserContentHandler.js#514> behind a #ifndef MOZ_PER_WINDOW_PRIVATE_BROWSING guard, and in the #else block, implement a new command, -private-window.  The implementation needs to set preventDefault to true and then use a call to openWindow similar to the one further below this function but with "private" added to the window features, and with the uri.spec parameter taken out.

Once that works by testing firefox with -private-window on the command line, we need to modify WindowsJumpLists.jsm and put all of the stuff using the global PB mode service and the "private-browsing" notification behind #ifndef MOZ_PER_WINDOW_PRIVATE_BROWSING guards, and if that macro is defined, just add a single menu entry which adds a New Private Window command (which is the same name we have for the File menu entry.)

In the interest of getting the new string to our localizers sooner, I'll attach a patch which adds the string so that we can land it sooner.  The real patch here can use the strings I'm adding.
Assignee: chrislord.net → nobody
Whiteboard: [appcoast]
Created attachment 671010 [details] [diff] [review]
String changes
Attachment #671010 - Flags: ui-review?(madhava)
Attachment #671010 - Flags: review?(josh)
(Reporter)

Comment 7

5 years ago
Comment on attachment 671010 [details] [diff] [review]
String changes

I'm not sure there's a good reason to pre-land strings at this point.
(In reply to comment #7)
> I'm not sure there's a good reason to pre-land strings at this point.

It would get the strings in hands of our localizers sooner.
(Reporter)

Updated

5 years ago
Attachment #671010 - Flags: review?(josh) → review+
Stephen, can you please take a stab at the ui-review here?  Thanks!
(Assignee)

Updated

5 years ago
Assignee: nobody → andres
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 10

5 years ago
Created attachment 683825 [details] [diff] [review]
Patch v1
Attachment #683825 - Flags: review?(josh)
Attachment #683825 - Flags: review?(ehsan)
(Reporter)

Comment 11

5 years ago
Comment on attachment 683825 [details] [diff] [review]
Patch v1

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

This looks ok to me.
Attachment #683825 - Flags: review?(josh) → feedback+
Attachment #683825 - Flags: review?(ehsan) → review+
Comment on attachment 671010 [details] [diff] [review]
String changes

Gavin says that this doesn't need ui-r.
Attachment #671010 - Flags: ui-review?(madhava)
https://hg.mozilla.org/integration/mozilla-inbound/rev/e138dea215c7
https://hg.mozilla.org/integration/mozilla-inbound/rev/216a514344ea
https://hg.mozilla.org/mozilla-central/rev/e138dea215c7
https://hg.mozilla.org/mozilla-central/rev/216a514344ea
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
Duplicate of this bug: 820136

Comment 16

5 years ago
Verified as fixed on Mozilla/5.0 (Windows NT 6.1; rv:21.0) Gecko/20130115 Firefox/21.0 and Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20130115 Firefox/20.0.

The "New Private Window" option is offered in the jumplist.
Status: RESOLVED → VERIFIED

Comment 17

4 years ago
Can someone look at Bug 942938.   Jumplist items are opening a new instance of firefox when using application.ini file to rebrand the application and they can't be disabled.   This causes the users firefox profile to clobbered by the profile settings from our custom app.
You need to log in before you can comment on or make changes to this bug.