738487 - JS warning 'reference to undefined property server.spamSettings' after saving changes to settings of an IM account

Status: RESOLVED FIXED

(Thunderbird :: Instant Messaging, defect)

Description

[:aceman]

Just starting Thunderbird I get all these messages in the Error console:

Timestamp: 22.03.2012 23:27:21
Error: Attempting to set a null username to an imIncomingServer
Source File: file:///components/imIncomingServer.js
Line: 180

Timestamp: 22.03.2012 23:27:22
Warning: reference to undefined property MailServices.accounts.accounts
Source File: resource:///modules/mailInstrumentation.js
Line: 224

(should probably be only one .accounts?)

Timestamp: 22.03.2012 23:27:22
Warning: reference to undefined property am.accounts
Source File: chrome://messenger/content/accountUtils.js
Line: 130

Timestamp: 22.03.2012 23:27:22
Warning: reference to undefined property acctMgr.accounts
Source File: chrome://messenger/content/folderPane.js
Line: 1233

Timestamp: 22.03.2012 23:27:22
Warning: reference to undefined property accountManager.allServers
Source File: resource:///modules/MailUtils.js
Line: 72

Timestamp: 22.03.2012 23:27:23
Warning: reference to undefined property accountManager.allServers
Source File: chrome://messenger/content/mailWindowOverlay.js
Line: 1195

Timestamp: 22.03.2012 23:27:25
Warning: reference to undefined property mgr.accounts
Source File: chrome://messenger/content/mail3PaneWindowCommands.js
Line: 412

Timestamp: 22.03.2012 23:27:52
Warning: reference to undefined property mgr.accounts
Source File: chrome://messenger/content/mailTabs.js
Line: 496

Timestamp: 22.03.2012 23:27:57
Warning: reference to undefined property mgr.accounts
Source File: chrome://messenger/content/messengercompose/MsgComposeCommands.js
Line: 3240

Removing the IM account in the Account manager and restarting TB makes the warnings disappear.

Is this a problem of the 'JS test' type of IM account or a general problem of IM accounts?

Comment 1

[Florian Quèze [:florian]]

Can you try with the patch from bug 738367 and maybe the patch from bug 736434 too and tell me if there are still warnings caused by the existence of the chat account?

Comment 2

[:aceman]

Yes I will.

Comment 3

[:aceman]

It looks very good now.

It seems only this one is left:
Timestamp: 23.03.2012 23:22:45
Warning: reference to undefined property server.spamSettings
Source File: chrome://messenger/content/AccountManager.js
Line: 543

It happens after changing any setting in the JS test account and OKing account manager.

Comment 4

[:aceman]

Can I fix this with a currentServer.type != "im" check too?

Comment 5

[Florian Quèze [:florian]]

(In reply to :aceman from comment #4)
> Can I fix this with a currentServer.type != "im" check too?

Given that the spamSettings getter is implemented in http://mxr.mozilla.org/comm-central/source/mail/components/im/imIncomingServer.js#197, this doesn't seem like the right fix to me.
I'm not sure of why we have this warning, but if I wanted to fix it, I think the first thing I would try is to add a line like this in the object the getter returns:
QueryInterface: XPCOMUtils.generateQI([Ci.nsISpamSettings])

Comment 6

[:aceman]

Ah, you can fix it on the IM server side? Then please do.

Comment 7

[Florian Quèze [:florian]]

I didn't want to steal it from you ;).

Comment 8

[:aceman]

I didn't have it yet, so no problem :)
And the server side solution is better.

Comment 9

[Florian Quèze [:florian]]

Found another warning caused by the IM backend:
JavaScript strict warning: chrome://messenger/content/mailWindowOverlay.js, line 1207: reference to undefined property currentServer.loginAtStartUp

Comment 10

[Florian Quèze [:florian]]

Attachment: Patch

Yet another patch to remove some warnings caused by the imIncomingServer.js/imProtocolInfo.js implementations.

Comment 11

[David :Bienvenu]

Comment on attachment 610146 [details] [diff] [review]
Patch

a comment as to why canLoginAtStartup should be false might be useful.

Comment 12

[Florian Quèze [:florian]]

http://hg.mozilla.org/comm-central/rev/a80766e8b581

Comment 13

[Florian Quèze [:florian]]

Comment on attachment 610146 [details] [diff] [review]
Patch

[Approval Request Comment]
We took for aurora a similar patch in bug 738367 so maybe we should take this one too. I don't feel strongly about it.

Comment 14

[David :Bienvenu]

Comment on attachment 610146 [details] [diff] [review]
Patch

Since this is extremely low risk, I think it's better to keep the repos in sync, for each of applying future patches, and consistency of testing.

Comment 15

[Mark Banner (:standard8)]

Note: this failed to apply to comm-aurora. Is it missing another patch or something?

Comment 16

[Mark Banner (:standard8)]

That's because it had already landed:

http://hg.mozilla.org/releases/comm-aurora/rev/5d0a6ebdbd33