458061 - Opening Venkman starts Java

Status: RESOLVED FIXED

(Other Applications Graveyard :: Venkman JS Debugger, defect)

Description

[:Gijs (he/him)]

See bug 435772.

Request for keyword: sucky-external-software. Ugh. :-(

Comment 1

[:Gijs (he/him)]

Al: can I get r+ for you to apply attachment 323141 [details] [diff] [review] to the utils.js we have in Venkman? :-)

Cheers!

Comment 2

[Alex Vincent [:WeirdAl]]

+// Netscape/Mozilla security manager, for gaining priviledges with consent.

See bug 106386.

 jsenv.HAS_XPCOM = ((typeof Components == "object") &&
+                   (typeof Components.classes == "object") &&
+                   (typeof Components.interfaces == "object"));

Yeeeuck.  Object detection.  If you're going to go this way, you should probably add this:

Components instanceof Components.interfaces.nsIXPCComponents

http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/idl/xpccomponents.idl#226

+var ci = jsenv.HAS_XPCOM ? Components.interfaces : {};
+jsenv.HAS_STREAM_PROVIDER = ("nsIStreamProvider" in ci);
+jsenv.HAS_SERVER_SOCKETS = ("nsIServerSocket" in ci);
+jsenv.HAS_THREAD_MANAGER = ("nsIThreadManager" in ci);
+delete ci;

Why this change?  I understand if we don't have XPCOM, then Components.interfaces doesn't exist... but the original code for these three lines was a lot clearer.  I'd prefer a block like this:

if (jsenv.HAS_XPCOM) {
  jsenv.HAS_STREAM_PROVIDER = ("nsIStreamProvider" in Components.interfaces);
  // ...
} else {
  jsenv.HAS_STREAM_PROVIDER = false;
  // ...
}

Sorry, r-.

Comment 3

[:Gijs (he/him)]

(In reply to comment #2)
> +// Netscape/Mozilla security manager, for gaining priviledges with consent.
> 
> See bug 106386.
> 
>  jsenv.HAS_XPCOM = ((typeof Components == "object") &&
> +                   (typeof Components.classes == "object") &&
> +                   (typeof Components.interfaces == "object"));
> 
> Yeeeuck.  Object detection.  If you're going to go this way, you should
> probably add this:
> 
> Components instanceof Components.interfaces.nsIXPCComponents
> 
> http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/idl/xpccomponents.idl#226
> 
> +var ci = jsenv.HAS_XPCOM ? Components.interfaces : {};
> +jsenv.HAS_STREAM_PROVIDER = ("nsIStreamProvider" in ci);
> +jsenv.HAS_SERVER_SOCKETS = ("nsIServerSocket" in ci);
> +jsenv.HAS_THREAD_MANAGER = ("nsIThreadManager" in ci);
> +delete ci;
> 
> Why this change?  I understand if we don't have XPCOM, then
> Components.interfaces doesn't exist... but the original code for these three
> lines was a lot clearer.  I'd prefer a block like this:
> 
> if (jsenv.HAS_XPCOM) {
>   jsenv.HAS_STREAM_PROVIDER = ("nsIStreamProvider" in Components.interfaces);
>   // ...
> } else {
>   jsenv.HAS_STREAM_PROVIDER = false;
>   // ...
> }
> 
> Sorry, r-.

All good points, thanks!

However, it turns out that the venkman-utils.js file is anyway very different from the IRC one by now... :-(
Sorry.

So what I'll do is just remove the Java detection, because the whole of Vnk will be massive fail when we don't have XPCOM anyway... Patch in a sec.

Comment 4

[:Gijs (he/him)]

Attachment: Patch

So, removing a bunch of dead code, per:

http://mxr.mozilla.org/seamonkey/search?string=jsenv&find=%2Fextensions%2Fvenkman%2F

we don't use this anyway.

Comment 5

[Alex Vincent [:WeirdAl]]

Comment on attachment 341299 [details] [diff] [review]
Patch

I'll buy that.  :)

I hope the Chatzilla code can be amended per my suggestions, but that's not something I can review on.

Comment 6

[:Gijs (he/him)]

Checking in mozilla/extensions/venkman/resources/content/venkman-utils.js;
/cvsroot/mozilla/extensions/venkman/resources/content/venkman-utils.js,v  <--  venkman-utils.js
new revision: 1.38; previous revision: 1.37
done

Comment 7

[Serge Gautherie (:sgautherie)]

(In reply to comment #5)
> I hope the Chatzilla code can be amended per my suggestions

Did you file a bug on this ?

Comment 8

[Alex Vincent [:WeirdAl]]

Serge: no, no one did.