326291 - Provide a way for chrome to safely open content windows

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Embedding: APIs, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Right now, opening a content window from chrome is a pain.  See bug 323810 comment 27 for details.

Note that chrome _can_ open content windows if it opens a window without the nsIWebBrowserChrome::CHROME_OPENAS_CHROME flag.  But in that case the window.opener of the content window will be the chrome window.  Which seems to me like a really bad idea.

So as I see it the goals are:

1)  Define what targeted links in chrome should do.
2)  Define what a "content" window.open in chrome should do.
3)  Make sure that a chrome window is never the window.opener of a content
    window.... or at least that such an mOpener is never exposed to content
    script.
4)  Have a way for this procedure to make use of nsIWindowProvider as needed 
   (which one?  That is, which content tree owner?)

Thoughts?

Comment 1

[Boris Zbarsky [:bzbarsky]]

I suspect we need this for 1.8.1.  If nothing else, this may involve modification of nsIWindowProvider, conceivably.

Comment 2

[Benjamin Smedberg]

I think we have four classes of links to deal with:

1) untargeted links
  <a href="url"> or target="_self" or
  window.open(url, "_self")

Always open in the same docshell.

2) links targeted to the default browser
   <a href="url" rel="external"> window.open(url, "_blank", "external");

This is a new/special class of link that should always open in the default browser. The target is ignored. The code checks to see (on nsIExternalProtocolService) if the protocol is exposed: if it is (Firefox and Seamonkey), the load is run through nsIURILoader (which obeys tabbed-browsing preferences). Otherwise (Tbird and most xulrunner apps), the load is passed through nsIExternalProtocolService to load in the default browser.

2) links targeting content docshells <a href="url" target="namedtarget">
   or window.open(url, "namedtarget")

nsIWindowProvider should be used to search for named content docshells and/or create a new window. How do we get to an appropriate nsIWindowProvider? Can we ask for a "global" windowprovider (perhaps on the same object that implements nsIWindowCreator)? The newly-opened window should not have window.opener set.

4) links targeting chrome docshells <a href="url" target="namedtarget" rel="chrome"> or window.open(url, "namedtarget", "chrome")

Do what we do now, I think (if rel="chrome" sounds like a good idea, need to implement it)

Comment 3

[Boris Zbarsky [:bzbarsky]]

> 2) links targeted to the default browser
>    <a href="url" rel="external"> window.open(url, "_blank", "external");

Those don't behave the same way.  While for window.open a target of "" is equivalent to "_blank", for an anchor it's equivalent to "_self".

I suppose we could mess with this for chrome docshells, but I'd really rather not...

Does URILoader already obey tabbed browsing preferences?  If you pass the chrome docshell as the original context?  Or is that a proposed change?

I agree with your classification in general; I'm not sure how to implement it yet.  In particular, there are really no provisions for named targets meaning different things in different cases; I doubt we can do that without API changes.

> nsIWindowProvider should be used to search 

nsIWindowProvider doesn't search....  I'd really rather not change that.  That's handled by the window watcher.

Frankly, I can't see a use case for chrome needing to call window.open on a chrome window and target a non-chrome window with it except for _blank as the target... can you?

Comment 4

[Benjamin Smedberg]

uriloader (or more specifically the browsercontenthandler) alreay obeys the tabbed-browsing preferences, because that's the codepath we use to open URLs from the commandline and other external sources.

Comment 5

[[e-mail]]

Copied from bug 323810, sorry.

What I would really like is for chrome docshells not to accept link loads.
That way, you could just write <label><a href="http://themes.mozdev.org/">Get
New Themes</a></label> (in XUL) or window.QueryInterface(nsIInterfaceRequestor)
.getInterface(nsIWebNavigation).loadURI("http://themes.mozdev.org",
nsIWebNavigation.LOAD_FLAGS_IS_LINK, null, null, null); (in JS).

Comment 6

[[e-mail]]

(In reply to comment #33)
>That sounds like possible nsIURIContentListener2 work.  Comment in that bug?
No, I mean globally for all chrome docshells.

Comment 7

[Benjamin Smedberg]

Neil, I do not think it is a good idea for a link to behave drastically differently in a chrome docshell and a content docshell. I think it would be much better for the link to specify what its desired behavior actually is.(In reply to comment #3)

> > 2) links targeted to the default browser
> >    <a href="url" rel="external"> window.open(url, "_blank", "external");
> 
> Those don't behave the same way.  While for window.open a target of "" is
> equivalent to "_blank", for an anchor it's equivalent to "_self".

Please note that I'm proposing a new DOM feature here, "external"... which overrides and ignores link targeting and always loads the URL in the default app.

> Frankly, I can't see a use case for chrome needing to call window.open on a
> chrome window and target a non-chrome window with it except for _blank as the
> target... can you?

Hrm, no, I don't think I can.

Comment 8

[Boris Zbarsky [:bzbarsky]]

> Please note that I'm proposing a new DOM feature here, "external"...

Oh, I see.  Yeah, that may be worth doing...  Exposed to content also, or only chrome?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Having "chrome docshells" (type chrome?  Or having the chrome tree listener?  Or what?) not accept link loads might work, actually.  I'm not sure it wouldn't break things like Help, etc, though.

Comment 10

[Benjamin Smedberg]

Exposed to content also, for the purposes of my webrunner gmail-in-xulrunner thing.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Ccing people who should be in on any decisions regarding how content behaves...

Comment 12

[[e-mail]]

(In reply to comment #9)
>Having "chrome docshells" (type chrome?  Or having the chrome tree listener? Or what?)
I don't know what a chrome tree listener is. I was thinking of the root shell and any frames except those with type="content" [-primary].

>not accept link loads might work, actually.  I'm not sure it wouldn't
>break things like Help, etc, though.
Help currently uses a content docshell (<browser type="content-primary">).

Comment 13

[Boris Zbarsky [:bzbarsky]]

Er, I meant chrome tree owner.  At the moment we only use it for the root chrome shell, but that's a bug, imo.

> Help currently uses a content docshell (<browser type="content-primary">).

In that case it'll have to take special steps to not have it act just like any content shell...

Comment 14

[[e-mail]]

(In reply to comment #13)
>>Help currently uses a content docshell (<browser type="content-primary">).
>In that case it'll have to take special steps to not have it act just like any content shell...
That's bug 324747, but my point is that making chrome docshells not accept link loads won't affect Help.

Comment 15

[Mike Connor [:mconnor]]

Not going to block on this at this point.  Will consider a patch if something comes along.

Comment 16

[Benjamin Smedberg]

Marking a bunch of bugs in the "Embedding: APIs" component INCOMPLETE in preparation to archive that component. If I have done this incorrectly, please reopen the bugs and move them to a more correct component as we don't have "embedding" APIs any more.