Closed
Bug 722993
Opened 13 years ago
Closed 12 years ago
WindowsJumpLists.jsm uses global Private Browsing state to make decisions
Categories
(Firefox :: Shell Integration, defect)
Tracking
()
VERIFIED
FIXED
Firefox 20
People
(Reporter: jdm, Assigned: andreshm)
References
Details
(Whiteboard: [appcoast])
Attachments
(2 files)
(deleted),
patch
|
jdm
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
ehsan.akhgari
:
review+
jdm
:
feedback+
|
Details | Diff | Splinter Review |
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•13 years ago
|
OS: Mac OS X → Windows 7
Reporter | ||
Comment 1•13 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•13 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•13 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'.
Comment 4•13 years ago
|
||
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.
Comment 5•12 years ago
|
||
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]
Comment 6•12 years ago
|
||
Attachment #671010 -
Flags: ui-review?(madhava)
Attachment #671010 -
Flags: review?(josh)
Reporter | ||
Comment 7•12 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.
Comment 8•12 years ago
|
||
(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•12 years ago
|
Attachment #671010 -
Flags: review?(josh) → review+
Comment 9•12 years ago
|
||
Stephen, can you please take a stab at the ui-review here? Thanks!
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → andres
Assignee | ||
Updated•12 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 10•12 years ago
|
||
Attachment #683825 -
Flags: review?(josh)
Attachment #683825 -
Flags: review?(ehsan)
Reporter | ||
Comment 11•12 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+
Updated•12 years ago
|
Attachment #683825 -
Flags: review?(ehsan) → review+
Comment 12•12 years ago
|
||
Comment on attachment 671010 [details] [diff] [review]
String changes
Gavin says that this doesn't need ui-r.
Attachment #671010 -
Flags: ui-review?(madhava)
Comment 13•12 years ago
|
||
Comment 14•12 years ago
|
||
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
Comment 16•12 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•11 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.
Description
•