If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Chatzilla displays a bogus users line with (null) instead of the user counts for @ + and %

RESOLVED FIXED

Status

Other Applications
ChatZilla
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: mcsmurf, Assigned: Robert Ginda)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

13 years ago
To reproduce you need a current CVS trunk build, so get one, then join a irc
server (here irc.mozilla.org) and join some channel then. Then when you look at
the upper right of the Chatzilla window, you get 
Users  	58,10,0,0, (null)@, (null)%, (null)+
The 10 would need to stand before the @ instead of the (null), same for the two 0. 
This regressed somewhere before 2004-08-07 and 2004-08-05 i think. Probably this
is not a ChatZilla Bug, but i couldn't find a bug for this yet, so filling a new
one for triage of this problem, feel free to dupe/move to another component :)

Comment 1

13 years ago
This is, I'm reliably informed, because "someone broke instanceof". The gist is
that "foo instanceof Array" is no longer working if |foo| comes from a different
window... which is exactly what's going on here (output window code vs. chrome
code).

This should be marked dependant or dup of the relevant bug... when I find it. :)

For those really interested, the issue is that
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/extensions/irc/xul/content/output-window.js&mark=373-375#361
calls
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/extensions/irc/js/lib/message-manager.js&mark=199#192
and that check fails so you end up with the replacemen parameter list just
containing a isngle item - the toString() of the actual list.
(Reporter)

Comment 2

13 years ago
Looks like this comes from Bug 254067, maybe window.opener.foo needs to be used
(just a guess so)?!

Comment 3

13 years ago
This sucks much. I can't believe they've /deliberately/ broken |foo instanceof
Array| when foo is definately a normal Array.

Updated

13 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 2000 → All
Hardware: PC → All

Updated

13 years ago
Flags: blocking1.8a4?
> This sucks much. I can't believe they've /deliberately/ broken |foo instanceof
> Array| when foo is definately a normal Array.

No one broke any such thing, as should be clear from reading bug 254067.

foo instanceof top.Array or fooFromTop instanceof Array, where fooFromTop is a
reference to an Array constructed in another window, is what we broke, in
keeping with ECMA sanity, future security, and other testable
object/prototype/constructor relationships.

Sorry it broke chatzilla, but the fix shouldn't be hard.

/be

Comment 5

13 years ago
(In reply to comment #4)
> foo instanceof top.Array or fooFromTop instanceof Array, where fooFromTop is a
> reference to an Array constructed in another window, is what we broke, in
> keeping with ECMA sanity, future security, and other testable
> object/prototype/constructor relationships.

No use defending it now, I'd already read the bug and knew what had changed.

> Sorry it broke chatzilla, but the fix shouldn't be hard.

You're welcome to fix it, then - I don't know enough about the mysterious JS
engine to fix this.
(Assignee)

Comment 6

13 years ago
I don't think brendan is suggesting that the fix would be part of the js engine.
 Just a chatzilla workaround.
Silver, if you want the JS fix backed out, make a better case than whining like
a nine year old.  If it's all mysterious and you can only act hard done by, take
it elsewhere, find more sympathetic marks.  Around here, unintended breakage
happens, and the right fix is not always to go back to the status quo ante, or
act like a brat until someone else does.

/be
Created attachment 157122 [details] [diff] [review]
possible fix

There are other instanceof uses in extensions/irc/js/lib that might need
similar fixing -- rginda, can you say which do not?

/be

Comment 9

13 years ago
(In reply to comment #7)
> Silver, if you want the JS fix backed out

I never suggested that or thought about that as an option.

I suggested you fix it because *you* are the one here that knows what SM
provides that might be useful, like __parent__ (which seems to be undocumented
and deprecated, but that's neither here nor there).
Would it be better to check |typeof(params) == "object" && "length" in params|?
 That's our recommended "looks like a duck" Array-detection pattern, IIRC.
shaver: 'length' in params might lie -- the code wanted to know whether params
was an Array, not an object with a length property, and it seems better not to
relax that constraint.

Silver: sorry I misread you.  __parent__ is not deprecated, AFAIK.  Did someone
say it was?  It's peculiar to SpiderMonkey, but then so was the joining of all
constructors, in an instanceof sense, across window objects.

Anyone: what other instanceof usage in js/lib should change?

/be

Comment 12

13 years ago
brendan: mighty funny. jsengine strict warnings complain when you access .__parent__

>xpcshell.exe -w -s
js> a={}
typein:1: strict warning: assignment to undeclared variable a
[object Object]
js> a.__parent__
typein:2: strict warning: deprecated __parent__ usage
[object global]
js> build()
built on Aug 18 2004 at 08:30:02

believe me, we're familiar w/ that here, as we use it a lot and that warning
gets annoying while using venkman.

Comment 13

13 years ago
(In reply to comment #11)
> Silver: sorry I misread you.  __parent__ is not deprecated, AFAIK.  Did
> someone say it was?

Not in SpiderMonkey, but then I couldn't find any SM-specific reference to it
(which is where the undocumented comment came from). The (deprecated) comment
came from a page about Rhino (http://www.mozilla.org/rhino/overview.html) which
says "The deprecated features are the __proto__ and __parent__ properties, and
the constructors With, Closure, and Call." (about 1/4th of the way down). I
guess these are only deprecated in Rhino, then, from your reaction (which is
good, as they're very useful :) ).
Created attachment 157223 [details] [diff] [review]
remove strict warning for get of __proto__/__parent__

For crying out loud, *I* added strict warnings deprecating __proto__ and
__parent__ (maybe the Rhino mafia was pressuring me ;-).

This removes the strict warnings for the get case.

I'm likely to check it into the 1.8a4 trunk with very little encouragement from
timeless.

/be

Comment 15

13 years ago
Comment on attachment 157223 [details] [diff] [review]
remove strict warning for get of __proto__/__parent__

well, that'd certainly make some of us happy especially if you landed it on 1.7
branch :)
Attachment #157223 - Flags: review?(shaver)
Comment on attachment 157223 [details] [diff] [review]
remove strict warning for get of __proto__/__parent__

Let's not have this fight in front of the children. r=shaver
Attachment #157223 - Flags: review?(shaver) → review+

Updated

13 years ago
Attachment #157223 - Flags: approval1.7.x?

Comment 17

13 years ago
This was fixed in the landing of ChatZilla 0.9.65 (bug 255081).

I'll leave the bug open, though, as the JS engine strict warning patch is still
waiting for approval for 1.7.x landing (has it landed on trunk, either?).
Depends on: 255081
Comment on attachment 157223 [details] [diff] [review]
remove strict warning for get of __proto__/__parent__

This patch was checked into 1.7 and aviary branches already -- stealthed as
part of a larger jsobj.c patch.  Sorry I forgot to note the approvals.

/be
Attachment #157223 - Flags: approval1.7.x?
Attachment #157223 - Flags: approval1.7.x+
Attachment #157223 - Flags: approval-aviary+
Marking fix per Silver's comment 17.

/be
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Updated

13 years ago
Flags: blocking1.8a4?
Product: Core → Other Applications
You need to log in before you can comment on or make changes to this bug.