1518659 - JavaScript error: chrome://messenger/content/chat/imAccounts.js, line 665: TypeError: accountList._scrollbox is undefined

Status: RESOLVED FIXED

(Thunderbird :: Instant Messaging, enhancement)

Description

[Jorg K (CEST = GMT+2)]

We need to change _scrollbox to scrollbox at least in chat.

Comment 1

[Jorg K (CEST = GMT+2)]

Attachment: 1518659-chat-scrollbox.patch - patch doesn't help

With this, you can drag the accounts around in the chat status panel again.

Comment 2

[Jorg K (CEST = GMT+2)]

Assuming it passes review, it's ready to go.

Comment 3

[:aceman]

This seems to be currently the single user of _scrollbox. But we copied the same field to our forked copy of richlistbox.xml. Ths some widgets use the forked richlistbox version and may start to use its _scrollbox.
So the question is whether to preventively rename it too so that we need not to always wonder which of the bindings each richlistbox element uses.

Comment 4

[Jorg K (CEST = GMT+2)]

JavaScript error: chrome://messenger/content/chat/imAccounts.js, line 665: TypeError: accountList.scrollbox is undefined

:-(

Comment 5

[Jorg K (CEST = GMT+2)]

Well, I don't see what checkForMagicScroll() is doing. Broken or not, the dragging works.

Comment 6

[Jorg K (CEST = GMT+2)]

Well, backing out bug 1454360 and my patch, I get:
JavaScript error: chrome://messenger/content/chat/imAccounts.js, line 665: TypeError: accountList._scrollbox is undefined

So as far as I can see, this has always been broken and no one noticed.

Comment 7

[:aceman]

Yes, we mixed up richlistbox (which does not seem to have any *scrollbox) and <scrollbox>, which is what m-c has changed now.
I don't know what that line is supposed to do. But it seems "accountlist" was a richlistbox from the beginning (not migrated from listbox recently).

Comment 8

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+1) from comment #6)

So as far as I can see, this has always been broken and no one noticed.

Not quite. No error in TB 60. Maybe we missed something in bug 1470371?

Comment 9

[Florian Quèze [:florian]]

(In reply to Jorg K (GMT+1) from comment #5)

Well, I don't see what checkForMagicScroll() is doing. Broken or not, the dragging works.

It's strange indeed that this seems to work despite throwing so many errors. Maybe this behavior got implemented somewhere else and this code is not useful anymore? We can probably just remove checkForMagicScroll entirely, but it would be good to understand what happened though :-/

Comment 10

[:aceman]

(In reply to Jorg K (GMT+1) from comment #8)

Not quite. No error in TB 60. Maybe we missed something in bug 1470371?

No, we forgot this in bug 1516813 as a result of bug 1472557 which removed _scrollbox from richlistbox. It is only a few days old.

Comment 11

[Jorg K (CEST = GMT+2)]

I see it now ;-)

Comment 12

[Jorg K (CEST = GMT+2)]

Attachment: 1518659-chat-scrollbox.patch (v2)

Comment 13

[:aceman]

Comment on attachment 9035419 [details] [diff] [review]
1518659-chat-scrollbox.patch (v2)

Review of attachment 9035419 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, no error now.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2bda8c04fa47
Bug 1516813 follow-up: Port bug 1472557: remove use of _scrollbox in imAccounts.js. r=aceman