288844 - irc channel calls onStartRequest synchronously and without onStopRequest

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, defect)

Description

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

see URL - the irc protocol handler calls onStartRequest on its listener
synchronously, and never calls onStopRequest.

All callbacks from asyncOpen should happen asynchronously; and onStartRequest
calls must be paired with onStopRequest.

Comment 1

[James Ross]

I really dislike having the channel impl. at all.

What do you suggest? Calling both with setTimeout?

Comment 2

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

I'm not sure... newChannel could return an inputstreamchannel with an empty
inputstream. or you could open the window from asyncOpen and throw from it (iirc
that's what external protocols do)

Comment 3

[Boris Zbarsky [:bzbarsky]]

Returning an empty stream is probably not as good an approach as just throwing
from AsyncOpen like the external protocol channel does.

Comment 4

[James Ross]

Attachment: Throw from asyncOpen

diff seems to have decided I moved the command line handler, but I really moved the content handler's launch code into a function at the top. The content handlers have both gone, as they are not needed - the channel throws from aynscOpen.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Throwing from asyncOpen will show an error page if an irc:// link is clicked in a web page.  Is that really the aim?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Oh, nevermind.  You're throwin NO_CONTENT; that should work.

Comment 7

[James Ross]

Checked in --> FIXED.

Comment 8

[James Ross]

*sigh*

Components.results needs to burn in hell. It doesn't actually have this code, so we're throwing undefined/null.

Comment 9

[:Gijs (he/him)]

Throwing 0x804b0011 (which == NS_ERROR_NO_CONTENT if http://twpol.dyndns.org/mozilla/misc/nserror is correct) in trunk from 2006-11-21 actually gets you this error message in the console still:

Error: [Exception... "'<error>' when calling method: [nsIChannel::asyncOpen]"  nsresult: "0x804b0011 (<unknown>)"  location: "<unknown>"  data: no]

Am I missing something really silly? Or does even defining a constant not actually solve the problem?

Comment 10

[James Ross]

I blame XPConnect, but throwing it as a constant is the right solution for our code AFAIK. Need a debug build to work out who's spamming that crap to the console.

Comment 11

[:Gijs (he/him)]

Attachment: Sucky patch

Blah. Throwing the value just pops up a different error in the js console. Setting components.returnCode to the same thing doesn't, though, and it still works.

Comment 12

[James Ross]

Comment on attachment 249342 [details] [diff] [review]
Sucky patch

> +const NS_ERROR_MODULE_NETWORK_BASE = 2152398848;

Nit: please use hex format for clarity.

> -    throw Components.results.NS_ERROR_NO_CONTENT;
> +    Components.returnCode = NS_ERROR_NO_CONTENT;

Please put a comment above this saying we don't throw so that we don't get a message on the JS console.

r=silver with those changes.

Comment 13

[:Gijs (he/him)]

Checking in mozilla/extensions/irc/js/lib/chatzilla-service.js;
/cvsroot/mozilla/extensions/irc/js/lib/chatzilla-service.js,v  <--  chatzilla-service.js
new revision: 1.42; previous revision: 1.41
done

Comment 14

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

I want to note that this is not Necko's fault. XPCOM/XPConnect doesn't really have a way to expose error codes to script (I don't count the hardcoded list it has). Compare bug 13423.

Comment 15

[James Ross]

Nobody said it was - we've been blaming XPConnect from the start. ;)

Comment 16

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

the comment in the patch says:
+//XXXgijs: Because necko is annoying and doesn't expose this error flag, [...]