1293241 - "Show Accounts" shows a empty list

Status: RESOLVED FIXED

(Thunderbird :: Instant Messaging, defect)

Description

[Richard Marti (:Paenglab)]

With newest Daily I get by clicking "Show Accounts" an empty list and the Account Wizard opens with empty Chat type list.

Directly opening the Account list shows no errors. A bit later I have this error 99 times:
Mon Aug 08 2016 14:08:39
Error: ReferenceError: AppContants is not defined
Source file: chrome://messenger/content/chat/chat-messenger-overlay.js
Line: 1143

In normal account manager the IM accounts are still listed and the auto connect accounts do connect and are working.

Comment 1

[WaltS48 [:walts48]]

Shouldn't AppContants be AppConstants?

var imServices = {};
Components.utils.import("resource:///modules/chatNotifications.jsm");
Components.utils.import("resource:///modules/imServices.jsm", imServices);
Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
Components.utils.import("resource://gre/modules/AppConstants.jsm");

Comment 2

[Richard Marti (:Paenglab)]

It's a typo, yes. But it belongs to bug 1293302.

Comment 3

[aleth [:aleth]]

Probably fallout from Bug 1292569.

Comment 4

[:aceman]

So did patch in 1293302 fix this?

If not, you can try https://hg.mozilla.org/try-comm-central/rev/e4f2e0c8d5aab0f82f15500231e6184cc826e440 . It does contain changes in gloda core code outside of tests. But I am not sure if chat accounts would be affected by gloda.

Comment 5

[Richard Marti (:Paenglab)]

Unfortunately no change when opening the "Show Accounts" dialog.

Comment 6

[:aceman]

Attachment: WIP patch

So this makes the accounts list populated. It may be caused by my patch in bug 1292569 where I changed the getAccount() to proper function*.

After "connecting" an account I still get:
ReferenceError: reference to undefined property this.imAccount imXPCOMUtils.jsm:89:5

Not sure if that is related or not.

Hopefully you can now proceed from here what else is not working.

Comment 7

[Richard Marti (:Paenglab)]

Comment on attachment 8779035 [details] [diff] [review]
WIP patch

"Show Accounts" is working again. Thank you for the fix.

Comment 8

[Jorg K (CEST = GMT+2)]

Comment on attachment 8779035 [details] [diff] [review]
WIP patch

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

Standard change.
r=jorgk.
(We must get the show back on the road ;-))

Comment 9

[:aceman]

I'd still like aleth to see it as the are at least 2 getAccounts() in 2 imAccounts.js files in IM code. I tried to only change the mail/components/im/content one.

Comment 10

[Roberto]

Comment on attachment 8779035 [details] [diff] [review]
WIP patch

># HG changeset patch
># User aceman <[e-mail]>
># Parent  e4f2e0c8d5aab0f82f15500231e6184cc826e440
>Bug 1293241 - fix getAccounts() in imAccounts.js to be used as a generator. r?aleth
>
>diff --git a/mail/components/im/content/imAccounts.js b/mail/components/im/content/imAccounts.js
>--- a/mail/components/im/content/imAccounts.js
>+++ b/mail/components/im/content/imAccounts.js
>@@ -33,17 +33,17 @@ var gAccountManager = {
>   disableTimerID: 0,
>   _connectedLabelInterval: 0,
>   load: function am_load() {
>     // Wait until the password service is ready before offering anything.
>     Services.logins.initializationPromise.then(() => {
>       this.accountList = document.getElementById("accountlist");
>       let defaultID;
>       Services.core.init(); // ensure the imCore is initialized.
>-      for (let acc in this.getAccounts()) {
>+      for (let acc of this.getAccounts()) {
>         var elt = document.createElement("richlistitem");
>         this.accountList.appendChild(elt);
>         elt.build(acc);
>         if (!defaultID && acc.firstConnectionState == acc.FIRST_CONNECTION_CRASHED)
>           defaultID = acc.id;
>       }
>       for (let event of events)
>         Services.obs.addObserver(this, event, false);
>@@ -75,17 +75,17 @@ var gAccountManager = {
>   unload: function am_unload() {
>     clearInterval(this._connectedLabelInterval);
>     for (let event of events)
>       Services.obs.removeObserver(this, event);
>   },
>   _updateAccountList: function am__updateAccountList() {
>     let accountList = this.accountList;
>     let i = 0;
>-    for (let acc in this.getAccounts()) {
>+    for (let acc of this.getAccounts()) {
>       let oldItem = accountList.getItemAtIndex(i);
>       if (oldItem.id != acc.id) {
>         let accElt = document.getElementById(acc.id);
>         accountList.insertBefore(accElt, oldItem);
>         accElt.restoreItems();
>       }
>       ++i;
>     }
>@@ -475,17 +475,17 @@ var gAccountManager = {
>     window.openDialog(aUrl, "", "chrome,modal,titlebar,centerscreen", aArgs);
>     this.modalDialog = false;
>   },
>   setAutoLoginNotification: function am_setAutoLoginNotification() {
>     var as = Services.accounts;
>     var autoLoginStatus = as.autoLoginStatus;
>     let isOffline = false;
>     let crashCount = 0;
>-    for (let acc in this.getAccounts())
>+    for (let acc of this.getAccounts())
>       if (acc.autoLogin && acc.firstConnectionState == acc.FIRST_CONNECTION_CRASHED)
>         ++crashCount;
> 
>     if (autoLoginStatus == as.AUTOLOGIN_ENABLED && crashCount == 0) {
>       let status = Services.core.globalUserStatus.statusType;
>       this.setOffline(isOffline || status == Ci.imIStatusInfo.STATUS_OFFLINE);
>       return;
>     }hahahahahah

Comment 11

[aleth [:aleth]]

Comment on attachment 8779035 [details] [diff] [review]
WIP patch

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

This looks correct to me, but I suspect it's incomplete. It's not just that im/ and chat/ need their own version of the legacy generator removal patch, but also that bug 292569 changed not just getAccounts but also getIter, getProtoOptions, etc and all of these will probably have callers in for...in loops that should be for...of loops now.

Comment 12

[:aceman]

Attachment: patch v2

Thanks, this seems to be better. Makes also options in TB Account manager to be displayed.

I looked in /chat/* and it seems there everything is already fine with function*.
In /im/* there are some conversions needed. It has some stuff in /mail duplicated (or the other way round). If that is Instantbird, you will probably need to convert it in a new bug as I can't seriously test it.

Comment 13

[aleth [:aleth]]

Thanks for taking care of this!

(In reply to :aceman from comment #12)
> I looked in /chat/* and it seems there everything is already fine with
> function*.
> In /im/* there are some conversions needed. It has some stuff in /mail
> duplicated (or the other way round). If that is Instantbird, you will
> probably need to convert it in a new bug as I can't seriously test it.

Yeah, we should take care of that asap as this patch will break IB due to the chat/ change.

Comment 14

[Patrick Cloke [:clokep]]

I think these are bug 1293004 and bug 1293005.

Comment 15

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/adc56f0cd5b5

Comment 16

[:aceman]

(In reply to Patrick Cloke [:clokep] from comment #14)
> I think these are bug 1293004 and bug 1293005.

No, I think those are different change. The problem here was with changes from function to function* (generators), not with iterators.