Closed Bug 319066 Opened 17 years ago Closed 17 years ago

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

Categories

(Other Applications :: ChatZilla, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bugzilla-mozilla-20000923, Assigned: bugzilla-mozilla-20000923)

References

Details

(Keywords: regression, Whiteboard: [cz-0.9.69.1])

Attachments

(1 file, 1 obsolete file)

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.
Attachment #204976 - Flags: review?(samuel)
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+
Attachment #204976 - Flags: review+
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.
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!
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.
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.
(FWIW, tree.builderView == tree.contentView is true)
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.
Attachment #204976 - Attachment is obsolete: true
Attachment #204976 - Flags: review?(samuel)
Assignee: rginda → silver
Status: NEW → ASSIGNED
Attachment #205029 - Flags: review?(samuel)
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.
Summary: Content tree fails when some items not visible → Tree includes -1 in selection range
Attachment #205029 - Flags: review?(samuel) → review+
Checked in --> FIXED.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Summary: Tree includes -1 in selection range → Failure switching tabs when first user is selected (tree includes -1 in selection range)
Maybe a cleaner solution would be to call .select(-1) then .clearSelection()?
*** Bug 320749 has been marked as a duplicate of this bug. ***
Whiteboard: [cz-0.9.69.1]
You need to log in before you can comment on or make changes to this bug.