Capture window.open calls to redirect them depending on their target

RESOLVED FIXED in mozilla13

Status

()

Core
DOM
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: vingtetun, Assigned: vingtetun)

Tracking

Trunk
mozilla13
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Created attachment 592152 [details] [diff] [review]
Capture window.open calls

This is a pre-intents patch. The code in shell.js can be replace by intents.
Attachment #592152 - Flags: feedback?(fabrice)
Why do you need to do this?  That interfaces is noscript for pretty good reasons (it's _really_ easy to end up with a footgun).  Does nsIWindowProvider not let you do what you want to do here?

Because as far as I can tell, the attached patch would break window.open() in the web browser (not to mention confusing the heck out of security code: openWindowJS is clearly documented as relying on the JS stack, which you're now inserting chrome code into).
Created attachment 592318 [details] [diff] [review]
Patch
Attachment #592152 - Attachment is obsolete: true
Attachment #592152 - Flags: feedback?(fabrice)
Attachment #592318 - Flags: review?(fabrice)
Attachment #592318 - Flags: feedback?(bzbarsky)
Comment on attachment 592318 [details] [diff] [review]
Patch

Do b2g.js and shell.js correspond to different browser processes?  Why does b2g.js want to divert everything to the window the load is started from?  Trying to understand how the pieces here work together.

For the nsBrowserAccess bits, it looks like it's assuming that the caller will always pass a null URI.  This is a warranted assumption as far as nsContentTreeOwner is concerned... for now.  You don't expect any other consumers of nsIBrowserDOMWindow to call you?  Should you at least warn if the URI is not null?
Vivien, we don't differentiate links in new tabs, new windows from "normal" links here. Is it just because the current ApplicationManager() doesn't deal with these cases?
(In reply to Boris Zbarsky (:bz) from comment #3)
> Comment on attachment 592318 [details] [diff] [review]
> Patch
> 
> Do b2g.js and shell.js correspond to different browser processes?  Why does
> b2g.js want to divert everything to the window the load is started from? 
> Trying to understand how the pieces here work together.

I don't have a strong comprehension of what the preferences means here. I have just tried to set the correct values to reach the point I was looking for. I have removed the browser.link.open_newwindow.restrictio preference.

> 
> For the nsBrowserAccess bits, it looks like it's assuming that the caller
> will always pass a null URI.  This is a warranted assumption as far as
> nsContentTreeOwner is concerned... for now.  You don't expect any other
> consumers of nsIBrowserDOMWindow to call you?  Should you at least warn if
> the URI is not null?

I have |let url = uri ? uri.spec : 'about:blank'| and use that.
(In reply to Fabrice Desré [:fabrice] from comment #4)
> Vivien, we don't differentiate links in new tabs, new windows from "normal"
> links here. Is it just because the current ApplicationManager() doesn't deal
> with these cases?

I have added the 'where' arguments to launch() so you will be able to distinguish between those.

But this ApplicationManager code with be replace by a webintent (bug 715814) right?
Created attachment 592327 [details] [diff] [review]
Patch v0.3
Attachment #592318 - Attachment is obsolete: true
Attachment #592318 - Flags: review?(fabrice)
Attachment #592318 - Flags: feedback?(bzbarsky)
Attachment #592327 - Flags: review?(fabrice)
Attachment #592327 - Flags: review?(bzbarsky)
(In reply to Boris Zbarsky (:bz) from comment #3)
 > Do b2g.js and shell.js correspond to different browser processes?  Why does
> b2g.js want to divert everything to the window the load is started from? 
> Trying to understand how the pieces here work together.

For the moment there is only one process in b2g, so I'm unsure of your question.

The structure of B2G/Gaia is:

<xul:window>
  ...
 <xul:browser> <--homescreen -->
    ...
   <html:iframe name="app1"/>
   <html:iframe name="app2"/>
   <html:iframe name="browserapp">
     ...
     <html:iframe nane="web-content" mozbrowser/>
   </html/iframe>
 </xul:browser>
</xulwindow>

What I'm trying to achieve here is to have |window.open| calls from the web-content beeing handle by the main <xul:window>.
The main window will fire an intent, let's say 'open-in-browser', that will be handle by the browser app.
> I don't have a strong comprehension of what the preferences means here.

They're documented in firefox.js, no?
(In reply to Boris Zbarsky (:bz) from comment #9)
> > I don't have a strong comprehension of what the preferences means here.
> 
> They're documented in firefox.js, no?

Sure. But I'm unsure of what 'divert' means in this context.
Comment on attachment 592327 [details] [diff] [review]
Patch v0.3

> For the moment there is only one process in b2g

Ok, in that case the patch here looks fundamentally broken.  Setting open_newwindow to 1 will force the call into your nsIBRowserDOMWindow impl, but will also set *aWindowIsNew to false.  That will crew up things like .opener on the new window to some extent, security permissions on the new window, and the load flags for the actual load.  Why _did_ you set it to 1 instead of 3?  Seems like 3 would have made more sense....
Attachment #592327 - Flags: review?(bzbarsky) → review-
> But I'm unsure of what 'divert' means in this context.

The usual meaning of "send somewhere different than where it was heading".  "where it was heading" being "a new toplevel window".

I recommend just changing that pref in Firefox and using window.open() with a URI in a web page and seeing what happens if you want to get a feel for how it ends up working there.

I assume you did read nsContentTreeOwner::ProvideWindow, which is what made you change that pref?
Created attachment 592329 [details] [diff] [review]
Patch v0.4
Attachment #592327 - Attachment is obsolete: true
Attachment #592327 - Flags: review?(fabrice)
Attachment #592329 - Flags: review?(fabrice)
Attachment #592329 - Flags: review?(bzbarsky)
(In reply to Boris Zbarsky (:bz) from comment #12)
> > But I'm unsure of what 'divert' means in this context.
> 
> The usual meaning of "send somewhere different than where it was heading". 
> "where it was heading" being "a new toplevel window".
> 
> I recommend just changing that pref in Firefox and using window.open() with
> a URI in a web page and seeing what happens if you want to get a feel for
> how it ends up working there.

Done.

> 
> I assume you did read nsContentTreeOwner::ProvideWindow, which is what made
> you change that pref?

I did.
Comment on attachment 592329 [details] [diff] [review]
Patch v0.4

OK, this I think I buy.  You do want to set newwindow.restriction to 0, though.  Otherwise opening a sized window will bypass your code, right?

r=me with that, I think.  Assuming I understood what you're trying to do here correctly....
Attachment #592329 - Flags: review?(bzbarsky) → review+
Attachment #592329 - Flags: review?(fabrice) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/4342b1492c38
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/4342b1492c38
Target Milestone: mozilla12 → mozilla13
You need to log in before you can comment on or make changes to this bug.