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

RESOLVED FIXED

Status

--
major
RESOLVED FIXED
13 years ago
13 years ago

People

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

Tracking

({regression})

Trunk
regression

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [cz-0.9.69.1])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

13 years ago
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.
(Assignee)

Comment 1

13 years ago
Created attachment 204976 [details] [diff] [review]
Squash stupid tree errors for now
Attachment #204976 - Flags: review?(samuel)

Comment 2

13 years ago
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+

Comment 3

13 years ago
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.
(Assignee)

Comment 4

13 years ago
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!
(Assignee)

Comment 5

13 years ago
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.
(Assignee)

Comment 6

13 years ago
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.
(Assignee)

Comment 7

13 years ago
(FWIW, tree.builderView == tree.contentView is true)
(Assignee)

Comment 8

13 years ago
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.
(Assignee)

Updated

13 years ago
Attachment #204976 - Attachment is obsolete: true
Attachment #204976 - Flags: review?(samuel)
(Assignee)

Comment 9

13 years ago
Created attachment 205029 [details] [diff] [review]
Workaround for tree bug
Assignee: rginda → silver
Status: NEW → ASSIGNED
Attachment #205029 - Flags: review?(samuel)
(Assignee)

Comment 10

13 years ago
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.
(Assignee)

Updated

13 years ago
Summary: Content tree fails when some items not visible → Tree includes -1 in selection range

Updated

13 years ago
Attachment #205029 - Flags: review?(samuel) → review+
(Assignee)

Comment 11

13 years ago
Checked in --> FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 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)

Comment 12

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

Comment 13

13 years ago
*** Bug 320749 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

13 years ago
Whiteboard: [cz-0.9.69.1]
You need to log in before you can comment on or make changes to this bug.