223735 - _search does not work as a target when using window.open

Status: RESOLVED WONTFIX

(Firefox :: General, defect)

Description

[Erik Arvidsson]

Mozilla Firebird supports using target="_search" in a link to open up the link
in the sidebar. This does not however work for window.open

Comment 1

[Erik Arvidsson]

Attachment: Two links, one uses href, the other uses window.open

Comment 2

[Erik Arvidsson]

Attachment: Fixed testcase

I forgot to remove the target attribute for the second anchor making the page
open in both the sidebar and in a new window.

Comment 3

[Ned Regina]

Same problem occurs with FORM elements with target="_search".

Comment 4

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

*** Bug 268556 has been marked as a duplicate of this bug. ***

Comment 5

[Honza Bambas (:mayhemer)]

Attachment: v1

Implements target='_search' and also window.open("...", "_search").

So far w/o tests, but heavily tested also with javascript: and chrome: URIs to check we may not get privilege escalation from content.

Comment 6

[Honza Bambas (:mayhemer)]

I'd say some offline application may use the sidebar to work in it and it would be easy (actually possible) for homepages to implement opening it there by one-click, user friendliness.

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 385773 [details] [diff] [review]
v1

Boris, I think you need to look at this one...

Comment 8

[Boris Zbarsky [:bzbarsky]]

Er... Do you really mean for frames named _search to not be targetable?  And worse yet for the behavior to be inconsistent depending on the nsIBrowserDOMWindow implementation?

For that matter, why the heck would I want web content to be able to open my sidebar, ever?

Why would this code actually do anything?  I see no one handling OPEN_SIDEBAR.

What's the deal with the name stuff in OpenWindowJSInternal?

_If_ we want to do this (which is a big if), the right way to do it would seem to me to have the chrome tree owner return the right docshell when targeting _search and be done with it...  No need for all this other hackery, no?

Comment 9

[Honza Bambas (:mayhemer)]

(In reply to comment #8)
> And worse yet for the behavior to be inconsistent depending on the
> nsIBrowserDOMWindow implementation?
> 

I'm not against looking for a different solution.

> For that matter, why the heck would I want web content to be able to open my
> sidebar, ever?

_search target is an undocumented standard that works in IE (more or less) and in Opera. In IE it opens a new window with shape of a sidebar (width of say 20% of the screen). In Opera it opens the sidebar with URI. We have option for web bookmarks to open the content in a sidebar. So, we already want allow a content to be opened in a sidebar someway.

My answer is a question: why not to let the content open a sidebar? It would be cool for web apps to open a sidebar with web content (an app) by just one-click and not forcing user to create a bookmark, placing it somewhere on a visible place and then use/click it. (one simple step against 3 or more complicated and for at least a more experienced user).

> 
> Why would this code actually do anything?  I see no one handling OPEN_SIDEBAR.

It's handled in browser/base/content/browser.js, the first hunk of the patch. I don't say it's correct but I don't see a different way to do this.

> 
> What's the deal with the name stuff in OpenWindowJSInternal?

You talk about loadInfo->SetTarget(name.get()) call? We have to pass the name '_search' to nsIDocShell::LoadURI method though loadInfo. There it has a special handling to open the sidebar.

> 
> _If_ we want to do this (which is a big if), the right way to do it would seem
> to me to have the chrome tree owner return the right docshell when targeting
> _search and be done with it...  No need for all this other hackery, no?

I'm not that much familiar with this code, but sidebar opening works the way we open (load) webPanels.xul and assign it an onload method that opens the URI in the sidebar's browser window when it finish load; sidebar opening is asynchronous.

See http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#4727 and http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#4551

Comment 10

[Boris Zbarsky [:bzbarsky]]

> We have option for web bookmarks to open the content in a sidebar.

But that's under user control, not page control, right?

> why not to let the content open a sidebar?

Because I don't want random web pages opening my sidebar all the time?  The same reason we don't allow web pages to remove parts of the chrome, to open popup windows, to resize the browser window, etc.

> It's handled in browser/base/content/browser.js, the first hunk of the patch.

Ah, I missed that somehow.

> You talk about loadInfo->SetTarget(name.get()) call?

Yes.  So the code in docshell is there to handle the windowwatcher thing?  That's completely backwards.

> sidebar opening is asynchronous.

Then this will be very broken no matter what for form posts with target="_search".  All the more reason to not let random targeted loads open the sidebar.

Comment 11

[Honza Bambas (:mayhemer)]

(In reply to comment #10)
> > We have option for web bookmarks to open the content in a sidebar.
> 
> But that's under user control, not page control, right?
> 
> > why not to let the content open a sidebar?
> 
> Because I don't want random web pages opening my sidebar all the time?  The
> same reason we don't allow web pages to remove parts of the chrome, to open
> popup windows, to resize the browser window, etc.

To both arguments: we can have an option to disallow, the same way as we do it for pop-ups.

> > You talk about loadInfo->SetTarget(name.get()) call?
> 
> Yes.  So the code in docshell is there to handle the windowwatcher thing? 
> That's completely backwards.
> 

I can try to find a better solution but I need to know first if this is acceptable. From what you say it looks like you _really_ don't want it and would rather close this bug as WONTFIX.

> > sidebar opening is asynchronous.
> 
> Then this will be very broken no matter what for form posts with
> target="_search".  All the more reason to not let random targeted loads open
> the sidebar.

Isn't this a solvable problem?

Comment 12

[Boris Zbarsky [:bzbarsky]]

> To both arguments: we can have an option to disallow, the same way as we do it
> for pop-ups.

That's extra user cognitive overhead for little benefit.  We need it for popups because of web compat issues, but this is not a web compat issue.

But that's a UI decision, which is why I added uiwanted.

And yeah, I personally would really rather we didn't do this....

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 385773 [details] [diff] [review]
v1

In any case, this is a basckwards way to do this.  If we need to change window provider or the interaction of windowwatcher with it, or something about the tree owner, we can do all that.  We shouldn't be changing docshell code here, though.

Comment 14

[Honza Bambas (:mayhemer)]

Looks like there is no motivation to finish this.