Closed Bug 722993 Opened 12 years ago Closed 12 years ago

WindowsJumpLists.jsm uses global Private Browsing state to make decisions

Categories

(Firefox :: Shell Integration, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 20

People

(Reporter: jdm, Assigned: andreshm)

References

Details

(Whiteboard: [appcoast])

Attachments

(2 files)

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.
OS: Mac OS X → Windows 7
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?
(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
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: fxPBnGen
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]
Attached patch String changesSplinter Review
Attachment #671010 - Flags: ui-review?(madhava)
Attachment #671010 - Flags: review?(josh)
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.
Attachment #671010 - Flags: review?(josh) → review+
Stephen, can you please take a stab at the ui-review here?  Thanks!
Assignee: nobody → andres
Status: NEW → ASSIGNED
Attached patch Patch v1Splinter Review
Attachment #683825 - Flags: review?(josh)
Attachment #683825 - Flags: review?(ehsan)
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/mozilla-central/rev/e138dea215c7
https://hg.mozilla.org/mozilla-central/rev/216a514344ea
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
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
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.

Attachment

General

Created:
Updated:
Size: