406316 - " Show mode as symbol " has unpredictable results on Firefox and SeaMonkey trunk builds

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[worms]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9b2pre) Gecko/2007113005 Minefield/3.0b2pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9b2pre) Gecko/2007113005 Minefield/3.0b2pre

Changing the userlist setting to "show mode as symbol" is broken on trunk builts of Firefox and SeaMonkey.
The images representing the symbol sometimes show, sometimes do not show, moving the mouse over the nicks makes them alternatively appear and disappear.

This appears with the default motif or any motif, on 0.9.78.1 as well as 0.9.79, as long as the host application is Trunk.


Reproducible: Always

Steps to Reproduce:
1. Use a trunk built of Firefox or SeaMonkey (Fx 30b2 nightly or SeaMonkey 2.0a1pre)
2. Launch ChatZilla and join a channel
3. Change the userlist mode to "Symbol"
Actual Results:  
Watch the color (colour) blobs appear /disappear depending on where you put the mouse.

Expected Results:  
Symbols should be shown for all users and NOT JUMP :-)

OS tested for both Fx and SM: Windows 2000 SP4

Comment 1

[:Gijs (he/him)]

This doesn't happen with the JS userlist. One reason more...

- sev: min as you can workaround by switching tabs and back again, and most users don't change this preference a lot (it only breaks if you switch it, not if it is like this all the time)

- regression, used to work.

- almost got a regression range, and will move to Core when I do. This bug is *old* though.

Comment 2

[:Gijs (he/him)]

Regression range:

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-07-13+03%3A00%3A00&maxdate=2006-07-14+05%3A00%3A00&cvsroot=%2Fcvsroot

Comment 3

[:Gijs (he/him)]

Moving to Core, CC-ing Neil Deakin who might know what's going on here. Things that jump out at me in the bonsai query are:

bug 335122
bug 344571 (Cairo upgrade)


bug 344583 is somewhat near but as it's "general cleanup" it's not that likely...

Comment 4

[Neil Deakin]

Not sure why this was a sort service regression, a simple workaround should be to call tree.treeBoxObject.clearStyleAndImageCaches() after sorting. I'll continue to investigate.

Comment 5

[Neil Deakin]

The reason, it used to work is because the sort service removed the tree and reinserted it again after sorting, which no longer happens. Now, the tree doesn't realize that the rows have changed uses old cached style data.
Two possibilities here:

1. Clear the tree style cache after sorting
2. Clear the tree style cache in the AttributeChanged/ContentAppended/etc calls within nsTreeContentView.

Comment 6

[Neil Deakin]

Attachment: try clearing the cache when an attribute on the tree/treechildren changes

Really, we need a means for the tree to just clear this automatically when the style could be affected, but should be good enough to fix the issue here.

Comment 7

[[e-mail]]

Comment on attachment 291902 [details] [diff] [review]
try clearing the cache when an attribute on the tree/treechildren changes

>+  if (aContent == mRoot || aContent == mBody)
>+    mBoxObject->ClearStyleAndImageCaches();
Don't you need to null-check mBoxObject?

>+    mBoxObject->ClearStyleAndImageCaches();
Out of interest, when does changing a column need this?

Comment 8

[Neil Deakin]

(In reply to comment #7)
> (From update of attachment 291902 [details] [diff] [review])
> >+  if (aContent == mRoot || aContent == mBody)
> >+    mBoxObject->ClearStyleAndImageCaches();
> Don't you need to null-check mBoxObject?
> 

Yes, I originally had that and took it out for some reason.

> >+    mBoxObject->ClearStyleAndImageCaches();
> Out of interest, when does changing a column need this?

Looking further, it probably doesn't. I thought maybe there might be some obscure case where the attributes on the treecol would affect the style, but I don't think it's possible to construct a style rule that refers to both a treecol and the treechildren, so this isn't necessary.

Comment 9

[Neil Deakin]

Attachment: updated patch

Comment 10

[:Gijs (he/him)]

According to Neil on IRC, we need to invalidate() the tree as well, or nothing will change on the screen. :-)

</passing-on-the-message>

Comment 11

[Damon Sicore (:damons)]

Comment on attachment 291908 [details] [diff] [review]
updated patch

a=endgame drivers