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

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Capture window.open calls

This is a pre-intents patch. The code in shell.js can be replace by intents.

Comment 1

[Boris Zbarsky [:bzbarsky]]

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).

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

Comment 3

[Boris Zbarsky [: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?

Comment 4

[[:fabrice] Fabrice Desré]

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?

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(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.

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(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?

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.3

Comment 8

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(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.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> I don't have a strong comprehension of what the preferences means here.

They're documented in firefox.js, no?

Comment 10

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(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 11

[Boris Zbarsky [:bzbarsky]]

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....

Comment 12

[Boris Zbarsky [:bzbarsky]]

> 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?

Comment 13

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.4

Comment 14

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(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 15

[Boris Zbarsky [:bzbarsky]]

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....

Comment 16

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4342b1492c38

Comment 17

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4342b1492c38