Closed Bug 511308 Opened 10 years ago Closed 10 years ago

make autoconfig the default new account UI

Categories

(Thunderbird :: Account Manager, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b4

People

(Reporter: dmose, Assigned: Bienvenu)

References

Details

(Whiteboard: [no l10n impact remaining])

Attachments

(2 files, 1 obsolete file)

My understanding from Vancouver is that the plan is to make autoconfig the default UI for configuring a new account.  Here's a bug to make that happen; giving to Bryan to specify exactly which UI changes need to happen to lead to that end state.
Flags: blocking-thunderbird3+
Whiteboard: [no l10n impact]
OS: Mac OS X → All
Hardware: x86 → All
I think an easy change we can put through here would be a change in menuitem names, so I'm marking this l10n impact.

Change the current menuitem name to remove the "(Quick Setup)" text and change the existing "Account" item to be "Other Accounts".  With this change we'll have two entries for account setup, "Mail Account" and "Other Accounts".

I'm not sure it's necessary at this stage to strip mail accounts from the "Other Accounts" wizard as it will be a good fall back in case of issues with the new account setup launching.

Perhaps, since this change is so simple, we can push this through now and either use this or another bug to watch for additional items that fall out from this change.
Whiteboard: [no l10n impact] → [l10n impact]
Whiteboard: [l10n impact] → [has l10n impact]
Here's a patch that makes this change happen.  I've added in a separator since we now have two account type items in this menu.  ascii art below is what this patch provides.  Anyone want to review?

File -> New ->

Message
Folder/Sub-folder...
Saved Search...
---
Mail Account...
Other Accounts...
---
Address Book Contact...
Comment on attachment 397706 [details] [diff] [review]
[checked in] change name and layout of file -> new popup

sure, I'll review.
Attachment #397706 - Flags: review?(bienvenu)
Status: NEW → ASSIGNED
Version: unspecified → Trunk
Comment on attachment 397706 [details] [diff] [review]
[checked in] change name and layout of file -> new popup

shouldn't we also remove the mail/gmail pop&imap options (at least for TB) from the old account wizard?
Attachment #397706 - Flags: review?(bienvenu) → review+
(In reply to comment #4)
> (From update of attachment 397706 [details] [diff] [review])
> shouldn't we also remove the mail/gmail pop&imap options (at least for TB) from
> the old account wizard?

I suppose the gmail ones could definitely go but I figured we could wait until the 3.1 time frame to remove everything else.  No particular reason other than it would give us a backup to the new dialog as we get in the hands of more users.
(In reply to comment #5) 
> I suppose the gmail ones could definitely go but I figured we could wait until
> the 3.1 time frame to remove everything else.  No particular reason other than
> it would give us a backup to the new dialog as we get in the hands of more
> users.

Hmm - http://www.youtube.com/watch?v=P-hFbuWIKAA
marking this checkin-needed but please keep the bug open as I'll put in another patch to remove the email UI bits from the Other Accounts wizard.
Keywords: checkin-needed
Comment on attachment 397706 [details] [diff] [review]
[checked in] change name and layout of file -> new popup

Checked in: http://hg.mozilla.org/comm-central/rev/07682562b562
Attachment #397706 - Attachment description: change name and layout of file -> new popup → [checked in] change name and layout of file -> new popup
(In reply to comment #8)
> marking this checkin-needed but please keep the bug open as I'll put in another
> patch to remove the email UI bits from the Other Accounts wizard.

It is unclear from what's been said on this bug - will this patch include either changing the default account wizard that is shown on startup with a new profile, or making it so that from that wizard selecting "email accounts" will go to the new autoconfig UI?
now that the checkin was done, does this bug still have l10n impact?
(In reply to comment #10)
> (In reply to comment #8)

> 
> It is unclear from what's been said on this bug - will this patch include
> either changing the default account wizard that is shown on startup with a new
> profile, or making it so that from that wizard selecting "email accounts" will
> go to the new autoconfig UI?

Ah, yes, good point, it should replace the account wizard on startup as well, and it does not.

Changing that won't have l10n impact, however.
I'm going to try to the new profile case use the new autoconfig wizard.  I would think we would go straight to it, instead of indirectly going through the wizard. This turns out to be a bit tricky because the code that brings up the account wizard for a new profile is shared with SM. Also, the autoconfig wizard is going to need to provide an ok callback mechanism, if it doesn't already.
(In reply to comment #13)
> I'm going to try to the new profile case use the new autoconfig wizard.  I
> would think we would go straight to it, instead of indirectly going through the
> wizard. This turns out to be a bit tricky because the code that brings up the
> account wizard for a new profile is shared with SM. Also, the autoconfig wizard
> is going to need to provide an ok callback mechanism, if it doesn't already.

My only thought with going straight into it is what if they don't want a email, but do want a newsgroup/rss or other account? I know that is rare but...
They can cancel the wizard and use the file menu choice to create a new account. I'm interested in what others (clarkbw in particular) think, but I think it's rare enough that we should let the vast majority of users who want to configure an e-mail account go straight there.
Whiteboard: [has l10n impact] → [no l10n impact remaining]
Giving this over david for the remaining piece.  The removal of the old email account code isn't really blocking the release but it does depend on getting the new autoconfig ui finished so marking as such.
Assignee: clarkbw → bienvenu
Depends on: 506290
(In reply to comment #15)
> They can cancel the wizard and use the file menu choice to create a new
> account. I'm interested in what others (clarkbw in particular) think, but I
> think it's rare enough that we should let the vast majority of users who want
> to configure an e-mail account go straight there.

Agreed  I think we could try something in the "cancel account setup" dialog that offers "Other Accounts" as an option.  That would just be to help out that case so people don't have to look through the menu.  But I wouldn't block this release for that option.
deferring review to Blake since he's the most up to speed on the auto config code now.

This should leave SM using the old account wizard in the no account case, and both TB and SM using the old account wizard in the case of the compose window coming up with no identities. I'm not so concerned with that since it's a fairly rare occurrence.
Attachment #399311 - Flags: superreview?(bugzilla)
Attachment #399311 - Flags: review?(bwinton)
Whiteboard: [no l10n impact remaining] → [no l10n impact remaining][has patch for review]
I've been testing this and found a few issues with it:

STR:

1) Start up TB with new profile.
2) Use autoconfig to create an account with insecure servers.
3) Click Create account

Expected Results: Moves onto insecure warning.

Actual Results: Moves onto insecure warning and brings up the dialog for setting default client in the main mail window bringing the main mail window in front of the autoconfig wizard.

Then ignore default client dialog, select accept on the insecure dialog select create account and get

###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().: 'mRawPtr != 0', file ../../../mozilla/dist/include/xpcom/nsCOMPtr.h, line 796

If I accept the default client dialog, then complete the account creation I don't get any local folders created (or even its top level account).

I think I can also get multiple default client dialogs to come up if my configuration fails to log in and I have to go back to the beginning.

Second problem:

1) Create new profile, start up TB
2) Cancel out of the account wizard

(no local folders at this step, no default client dialog).

3) Try and create RSS or Newsgroup account

Both fail with errors like

JavaScript strict warning: chrome://messenger/content/AccountWizard.js, line 461: reference to undefined property srcServer["ServerType-" + srcServer.type]
srcServer.ServerType-rss = undefined

Third Problem:

1) Create new profile & startup
2) Enter some details into the account config wizard and detect configuration, enter some details manually.
3) Select "Create and enter account manually" (or whatever it is called)
4) Finish tweaking in account manager.
5) Select OK

Expected Results: Account + Local folders display, default client dialog displayed.

Actual Results: Main Mail Window is not changed - no accounts/folders shown, no default client dialog.
Comment on attachment 399311 [details] [diff] [review]
make the 3-pane ui bring up the new auto config wizard

Per previous comment.
Attachment #399311 - Flags: superreview?(bugzilla) → superreview-
Whiteboard: [no l10n impact remaining][has patch for review] → [no l10n impact remaining][needs new patch]
(In reply to comment #19)
> I've been testing this and found a few issues with it:
> Then ignore default client dialog, select accept on the insecure dialog select
> create account and get
> ###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().:
> 'mRawPtr != 0', file ../../../mozilla/dist/include/xpcom/nsCOMPtr.h, line 796

I was getting some of those with my patch for bug 506290, but I believe I fixed the ones I was seeing with the changes to mailnews/base/prefs/content/accountcreation/verifyConfig.js in the patch https://bugzilla.mozilla.org/attachment.cgi?id=399462&action=diff

(It looks like that code might leak a listener sometimes.  Perhaps that's what's causing this?)

Later,
Blake.
I moved the callback to the onclose handler for the dialog, and then tried to reproduce the issues you found, and could not. I understand why it would fix the first issue, but the others, not so much.
Attachment #399311 - Attachment is obsolete: true
Attachment #399485 - Flags: superreview?(bugzilla)
Attachment #399485 - Flags: review?(bwinton)
Attachment #399311 - Flags: review?(bwinton)
Whiteboard: [no l10n impact remaining][needs new patch] → [no l10n impact remaining][needs review, sr]
Attachment #399485 - Flags: review?(bwinton) → review+
Comment on attachment 399485 [details] [diff] [review]
move callback to onclose handler for dialog

After further testing, I can't replicate Standard8's bugs.

>+function AutoConfigWizard(okCallback)
>+{
>+  NewMailAccount(msgWindow, okCallback);
>+}

Should we have a doxygen comment for this function?
(It's a callback, so perhaps not, but then, where is the callback interface
documented?)

>+function verifyAccounts(wizardCallback, needsIdentity, wizardOpen) 

nit - trailing space.

Apart from that, it looks good.  r=me.

Later,
Blake.
Whiteboard: [no l10n impact remaining][needs review, sr] → [no l10n impact remaining][needs sr]
Comment on attachment 399485 [details] [diff] [review]
move callback to onclose handler for dialog

This is better. I would note that creating an account on SeaMonkey's default dialog creates and shows local folders whereas with the new account autoconfig we don't - so not quite sure what is happening there, but it seems to work fine if a bit confusing.
Attachment #399485 - Flags: superreview?(bugzilla) → superreview+
I fixed the trailing space and added some text to the doxygen comment for verifyAccounts to describe the callbacks. I'm going to spin-off a new bug for removing the mail accounts from the old account wizard, for TB at least.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [no l10n impact remaining][needs sr] → [no l10n impact remaining]
Bug 515479 filed for removing mail accounts from the old account wizard, for TB.
You need to log in before you can comment on or make changes to this bug.