Implement an API for opening new windows in private browsing

RESOLVED FIXED in Firefox 19

Status

()

Firefox
Private Browsing
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

unspecified
Firefox 19
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
The goal is to implement the following feature:

window.openDialog("chrome://browser/content/", "_blank", "chrome,private");
(Assignee)

Updated

6 years ago
Depends on: 795556
No longer depends on: 795556
Ack, un-warned collision :(
Depends on: 795556
(Assignee)

Comment 2

6 years ago
(In reply to Ehsan Akhgari [:ehsan] from comment #0)
> The goal is to implement the following feature:
> 
> window.openDialog("chrome://browser/content/", "_blank", "chrome,private");

Oh, and also: OpenBrowserWindow({private: true}) in browser.js.
(Assignee)

Comment 3

6 years ago
Created attachment 668944 [details] [diff] [review]
Part 1
Attachment #668944 - Flags: review?(bzbarsky)
(Assignee)

Comment 4

6 years ago
Created attachment 668945 [details] [diff] [review]
Part 2
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #668945 - Flags: review?(josh)
Comment on attachment 668945 [details] [diff] [review]
Part 2

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

::: browser/base/content/browser.js
@@ +3519,4 @@
>    var defaultArgs = handler.defaultArgs;
>    var wintype = document.documentElement.getAttribute('windowtype');
>  
> +  var extraFeatures = '';

Why single quotes, when you use double below?

@@ +3522,5 @@
> +  var extraFeatures = '';
> +#ifdef MOZ_PER_WINDOW_PRIVATE_BROWSING
> +  if (typeof options == "object" &&
> +      "private" in options &&
> +      !!options.private) {

No need for the !!
Comment on attachment 668945 [details] [diff] [review]
Part 2

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

r=me with Ms2ger's nits addressed.
Attachment #668945 - Flags: review?(josh) → review+
Comment on attachment 668944 [details] [diff] [review]
Part 1

r=me
Attachment #668944 - Flags: review?(bzbarsky) → review+

Comment 9

6 years ago
https://hg.mozilla.org/mozilla-central/rev/099477f00862
https://hg.mozilla.org/mozilla-central/rev/718453ec2c8a
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Changes like this (to a widely-used "core" browser API) should generally get some form of review or signoff from a browser peer, in the future.
(Assignee)

Comment 11

5 years ago
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #10)
> Changes like this (to a widely-used "core" browser API) should generally get
> some form of review or signoff from a browser peer, in the future.

Sorry!

Is there anything that you would like to see changed in this API?  I'd be glad to make any changes that you think would be appropriate.
No, I think the end result is fine (bug 822056 is what brought this to my attention, but that's a minor style thing, not a big deal).
You need to log in before you can comment on or make changes to this bug.