521955 - Can't edit server settings when check is cancelled

Status: RESOLVED FIXED

(Thunderbird :: Account Manager, defect, P1)

Description

[Ludovic Hirlimann [:Usul]]

STR:

 1) Tools -> Account settings
 2) New Account
    i) name l
    ii) email address : [e-mail]
    iii) password l
 3) press create account
 4) press the stop button
 5) edit settings manually

Expected results :
I can enter my server settings

results :
I can't the item Server setting is not present under the account name.

Comment 2

[David :Bienvenu]

Blake, does this look like a dup?

Comment 3

[Blake Winton (:bwinton) (:☕️)]

Yeah, this does look like a dupe of bug 490430.

Ludovic, were you using a nightly or 3.0b4 to reproduce this?  (And if it was 3.0b4, could you try to reproduce it using a nightly, because I can't seem to using the steps you provide.)

(A screencast might also show me something that I'm missing, if you wanted to try recording one, and emailing it to me.)

Thanks,
Blake.

Comment 4

[Blake Winton (:bwinton) (:☕️)]

It's not actually a dupe, and I've got a screencast from Ludovic which I'll post tomorrow morning that demonstrates the problem.

Comment 5

[Blake Winton (:bwinton) (:☕️)]

http://bwinton.latte.ca/temp/bug521955.swf

It's an interesting one.

Comment 6

[Phil Ringnalda (:philor)]

The plot drags a bit after opening the account manager, though ;)

I was able to repro once, and when I did closing and reopening the account manager made the missing bits show up, but then after that I lost whatever the key trick is and couldn't repro again.

Comment 7

[Blake Winton (:bwinton) (:☕️)]

The key trick seems to be starting from the account manager, then trying to create a new account from the dropdown, and then hitting manual settings to go back to the account manager.

At least, that's how I've been repro-ing it.

Comment 8

[Blake Winton (:bwinton) (:☕️)]

I can also repro it on Ubuntu, so it's not a Mac-only thing.

Comment 9

[Blake Winton (:bwinton) (:☕️)]

Attachment: A patch to fix that.

So, there are a couple of bits to this patch, but they're kind of mixed together.

The main thing that fixes this bug is that I've reversed the order of the lines:
  account.addIdentity(identity);
and
  account.incomingServer = inServer;
because setting the incomingServer seems to trigger the account manager to reload its list of servers (as per mailnews/base/prefs/content/AccountWizard.js line 438-ish, which says:
    // we mark the server as invalid so that the account manager won't
    // tell RDF about the new server - it's not quite finished getting
    // set up yet, in particular, the deferred storage pref hasn't been set.
) and if the identity isn't there, the account manager won't show the server settings.  (I didn't bother digging into why.)

Of course, if I was going to make that change, it made sense to push the lines that initialized the identity up above the place we set it in the account, so that's the second part of the patch.  Philor, if you'ld like to see just the first change, I could post a new patch with that easily enough.

Thanks,
Blake.

Comment 10

[David :Bienvenu]

Comment on attachment 406274 [details] [diff] [review]
A patch to fix that.

looks good, but a comment would be helpful.

Comment 11

[Blake Winton (:bwinton) (:☕️)]

Cool.  I've added the following:
  // Note: Setting incomingServer will cause the AccountManager to refresh
  // itself, which could be a problem if we came from it and we haven't set
  // the identity (see bug 521955), so make sure everything else on the
  // account is set up before you set the incomingServer.

Thanks,
Blake.

Comment 12

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/comm-central/rev/44dff0d7bae4

Comment 13

[Phil Ringnalda (:philor)]

Grr, and http://hg.mozilla.org/comm-central/rev/29697808ed64 since I forgot that you adding the comment doesn't do me any good if you don't add the patch with the added comment to the bug.