Implement an API for opening new windows in private browsing

RESOLVED FIXED in Firefox 19

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

unspecified
Firefox 19
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

The goal is to implement the following feature:

window.openDialog("chrome://browser/content/", "_blank", "chrome,private");
Depends on: 795556
No longer depends on: 795556
Ack, un-warned collision :(
Depends on: 795556
(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.
Posted patch Part 1Splinter Review
Attachment #668944 - Flags: review?(bzbarsky)
Posted patch Part 2Splinter Review
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+
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.
(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.