330167 - ChatZilla userlist is blank when swapped to the right-hand side (trunk only)

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Rob Marshall [tH]]

When the userlist is moved to the right (/pref userlistLeft false) it becomes empty. To get it back you need to put it back to the left and restart ChatZilla.

This doesn't happen in the 20060308 Windows Firefox nightly, but is broken in 20060309.
<http://bonsai.mozilla.org/cvsquery.cgi?date=explicit&mindate=2006-03-08+07%3A00%3A00&maxdate=2006-03-09+08%3A00%3A00>

Bug 329335 touched XUL templates in that time, but I don't know enough about the code in question and can't access the bug. Filing this in OtherApps/ChatZilla until we know any more.

Comment 1

[:Gijs (he/him)]

CC-ing bz and Neil.

[disclaimer: I don't know the code at all. I'm making assumptions here, which is bad. Please correct me ASAP :-)]

As far as I can see, that patch basically makes the template builder go 'okay, my root node is gone, so I can forget about this'. It seems to remove some kind of observer, and generally tell the template builder "don't care about this anymore".

Again, I'm not sure if I'm being correct here, but if I am, and this (nsXULTemplateBuilder) is used for XUL trees, then this is probably why we break, since we re-root (as in, remove the root and then add it again) the tree every time:
* the user joins a channel
* the user parts from a channel
* the user changes tabs (because there's a different channel, so a different set of data that should be represented).

We also resort the tree whenever someone else joins, parts or changes their nickname. I'm not sure if that sneakily re-roots the tree too. It sure does kill the selection, so I'm tempted to guess it does.

Why this only breaks if we're on the right side... no idea. And that's a pretty big hole in my logic, actually. Anyhow, commenting all this rambling anyway, maybe it can give People Who Know a better clue as to why we break.

On that matter - we switch the userlist to the right side by doing the following (http://chatzilla.rdmsoft.com/trunk/xul/content/static.js#l2580):
* add the splitter to the left of the user list (this removes it from its previous position).
* add the content deck's container to the left of the user list (this removes it from its previous position).
* setting the collapse property on the userlist to 'after'.

Comment 2

[James Ross]

What happens when a user joins/leaves/etc. is not related. That just, if anything, resets the root RDF node used by the tree, and doesn't touch the actual XUL itself.

The problem would look to be that of the addition of the ContentRemoved handler, which looks like it will nuke the template engine whenever any content element within the templated element is removed. I believe this notification will get triggered when removing the userlist from the document.

Comment 3

[Neil Deakin]

The template should be reapplied when a node is inserted into the document. A minimal testcase would be nice.

Also, it seems in this case that you are just reversing the list position, which can more easily be done just by setting the dir attribue on the container to reverse.

Comment 4

[James Ross]

FWIW, Robert Marshall has confirmed that it is the ContentRemoved code that is causing the problem. Clearly the template is not being reinstated on insertion.

Comment 5

[Rob Marshall [tH]]

Try <http://bugs.rdmsoft.com/xult/flip.xul>. Clicking the button once makes one listbox break, clicking it again breaks the other one.

Comment 6

[Boris Zbarsky [:bzbarsky]]

If the testcase could get attached to the bug (I guess attach the RDF, then point the XUL to it, then attach the XUL?) that would be great.

Enn, can you look into this?

Comment 7

[Rob Marshall [tH]]

Attachment: rdf/xml for testcase

Comment 8

[Rob Marshall [tH]]

Attachment: testcase

Fingers crossed...

Comment 9

[Neil Deakin]

Attachment: clear the lazy state, so the builder doesn't think the contents have already been built when reinserting the content

Comment 10

[Robert Johnston]

This doesn't appear to be resolved. See bug 367782.