319066 - Failure switching tabs when first user is selected (tree includes -1 in selection range)

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, defect)

Description

[James Ross]

If you select an item or two in the userlist, then scroll it so at least one is not visible, then try to change tab, the content tree dies:

[ERROR]	Internal error dispatching command “set-current-view”.
[ERROR]	NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsITreeContentView.getItemAtIndex] @ <chrome://chatzilla/content/static.js> 1445

Apparently items that aren't visible aren't in its list. What sort of crap that is I don't know, but I propose just wrapping the call in a try/catch until we can re-implement a better solution.

Comment 1

[James Ross]

Attachment: Squash stupid tree errors for now

Comment 2

[:Gijs (he/him)]

Comment on attachment 204976 [details] [diff] [review]
Squash stupid tree errors for now

*mumbles something about this sort of defeating the purpose of this ugly piece of code I wrote.

Alas there's not much better.

addl. r+

Comment 3

[Samuel Sieb]

I don't see this problem on Seamonkey, but I noticed there's a builderView as well as a contentView.  Could you test if using the builderView instead works as you would expect?  It appears to work the same way for me.

Comment 4

[James Ross]

It seems there is a bit more to this that just visibility; if I select the top item, change view, change back, then try to change away again - it dies. If, after changing back to the view with 1 item selected, I deselect and reselect that one item, it works.

(In reply to comment #3)
> I don't see this problem on Seamonkey, but I noticed there's a builderView as
> well as a contentView.  Could you test if using the builderView instead works
> as you would expect?  It appears to work the same way for me.

The code is common to both Mozilla and Firefox:
http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/tree/src/nsTreeContentView.cpp#693

It doesn't make any sense that it would behave differently.

However, builderView seems to work fine - which also makes no sense. Either the damn content element exists or it doesn't!

Comment 5

[James Ross]

Even stranger is that the use of contentView in the *set* code is fine! I'll change that to a builderView too, and test, though.

Comment 6

[James Ross]

Hm, I accidentally tested the builderView with the try/catch change, so it appeared to work - the builderView fails with the same errors as the contentView, which makes a damn sight more sense.

Comment 7

[James Ross]

(FWIW, tree.builderView == tree.contentView is true)

Comment 8

[James Ross]

Hm, this is slightly weird.

The failure, with Firefox 1.5, only occurs if I select the top item, and something then changes the tree (switch to another view and back, I think joins do it too). After that, it becomes "broken", and I know why.

The tree's first select block becomes a range from -1 to A, instead of 0 to A!

Man oh man do I hate trees. I'm going to try running with a patch to correct only the -1 -> A range thing, and see if that helps.

Comment 9

[James Ross]

Attachment: Workaround for tree bug

Comment 10

[James Ross]

FWIW, it is the select(-1) in the code that is causing there to be a -1 to -1 range at all, and then for it to become a -1 to 0 range when selecting item 0. This was used instead of clearSelection() to (suprise suprise!) work around another tree bug, in bug 197667.

Comment 11

[James Ross]

Checked in --> FIXED.

Comment 12

[[e-mail]]

Maybe a cleaner solution would be to call .select(-1) then .clearSelection()?

Comment 13

[James Ross]

*** Bug 320749 has been marked as a duplicate of this bug. ***