235746 - consider making nsIExternalHelperAppService::DoContent take no parameters

Status: NEW

(Firefox :: File Handling, defect)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

as things stand, DoContent is only useful for the use case that it's currently
used for - when you already have an opened channel and look for something to
dispatch it to.

However, it seems like this could be made more generally useful, by doing what
DoContent does currently in the stream listener's onstartrequest. This would
mean that DoContent basically only creates an nsExternalAppHandler and returns
it. It would not take any arguments.

That would easily allow, for example, to load an arbitrary url and make it open
the helper app dialog. as things are, you'd have to create your own
streamlistener and call doContent from its onstartrequest.

this is just an idea. comments?

Comment 1

[Boris Zbarsky [:bzbarsky]]

If we do this, what's the plan for implementing "View as Text"?  The helper app
dialog comes up from OStartR at the moment but I've been meaning to move it to
DoContent or something once we could suspend channels...

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #1)
>  The helper app
> dialog comes up from OStartR at the moment but I've been meaning to move it to
> DoContent or something once we could suspend channels...

how will that work? that dialog has to make some callbacks on the
nsIHelperAppLauncher (or what that's called), and you can't make the service
implement that, because you can have more than one helper app dialog at the same
time.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Hmm.... yeah.  I didn't claim I had it all worked out.  ;)

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

hmm, it turns out that doContent actually will need a parameter, namely the
window context. but null can be allowed for that.

(In reply to comment #1)
> If we do this, what's the plan for implementing "View as Text"?

the one that attachment 130410 [details] [diff] [review] shows....

OK; so I'd like to implement this now. It makes the API more useful. Any objections?

Comment 5

[Boris Zbarsky [:bzbarsky]]

So given that to create a helper app dialog you would need to pass in a window
context to the helper app handler anyway, why not just expose the helper app
handler (via contractid), give it an Init() method that takes a window context,
have URILoader use that, and kill DoContent altogether?

The plan for redispatch looks very spiffy.  I like.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #5)
> So given that to create a helper app dialog you would need to pass in a window
> context to the helper app handler anyway, why not just expose the helper app
> handler (via contractid), give it an Init() method that takes a window context,
> have URILoader use that, and kill DoContent altogether?

hmm...
that'd have to live on nsIHelperAppLauncher, this being the only modifiable
interface implemented by nsExternalAppHandler (nsIObserver and nsIStreamListener
are the others)

But, on that interface, it is mixed with functions that can only be called after
onStartRequest. it doesn't fit on this interface, in my opinion...

> The plan for redispatch looks very spiffy.  I like.

thanks!

Comment 7

[Boris Zbarsky [:bzbarsky]]

> that'd have to live on nsIHelperAppLauncher

Exactly.

> it is mixed with functions that can only be called after

I think it's pretty reasonable to require calling an Init() function before
using the object in any way....

In any case, you need something like that if you plan to allow random people to
use the helper app launcher by hand.

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Hmm, how about a different idea: Can we use the aContext parameter of the
streamlistener methods on the helperapplauncher to pass the window context?

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

That way, no Init method would be required.

(it looks like currently, nsnull is always passed)

Comment 10

[Boris Zbarsky [:bzbarsky]]

Hmm... that we could.  I _think_ that would work....