Dan Mosedale (:dmosedale, :dmose) Reporter
	Description • 15 years ago

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.

	Bryan Clark (DevTools PM) [:clarkbw]
	Comment 1 • 15 years ago

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.

	Bryan Clark (DevTools PM) [:clarkbw]
	Comment 2 • 15 years ago

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...

	David :Bienvenu Assignee
	Comment 3 • 15 years ago

Comment on attachment 397706 [details] [diff] [review]
[checked in] change name and layout of file -> new popup

sure, I'll review.

	David :Bienvenu Assignee
	Comment 4 • 15 years ago

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?

	Bryan Clark (DevTools PM) [:clarkbw]
	Comment 5 • 15 years ago

(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.

	David :Bienvenu Assignee
	Comment 6 • 15 years ago

(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

	Bryan Clark (DevTools PM) [:clarkbw]
	Comment 7 • 15 years ago

well put :)

	Bryan Clark (DevTools PM) [:clarkbw]
	Comment 8 • 15 years ago

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.

	Mark Banner (:standard8)
	Comment 9 • 15 years ago

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

	Mark Banner (:standard8)
	Comment 10 • 15 years ago

(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?

	Simon Paquet [:sipaq]
	Comment 11 • 15 years ago

now that the checkin was done, does this bug still have l10n impact?

	David :Bienvenu Assignee
	Comment 12 • 15 years ago

(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.

	David :Bienvenu Assignee
	Comment 13 • 15 years ago

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.

	Mark Banner (:standard8)
	Comment 14 • 15 years ago

(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...

	David :Bienvenu Assignee
	Comment 15 • 15 years ago

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.

	Bryan Clark (DevTools PM) [:clarkbw]
	Comment 16 • 15 years ago

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.

	Bryan Clark (DevTools PM) [:clarkbw]
	Comment 17 • 15 years ago

(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.

	David :Bienvenu Assignee
	Comment 18 • 15 years ago

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.

	Mark Banner (:standard8)
	Comment 19 • 15 years ago

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.

	Mark Banner (:standard8)
	Comment 20 • 15 years ago

Comment on attachment 399311 [details] [diff] [review]
make the 3-pane ui bring up the new auto config wizard

Per previous comment.

	Blake Winton (:bwinton) (:☕️)
	Comment 21 • 15 years ago

(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.

	David :Bienvenu Assignee
	Comment 22 • 15 years ago

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.

	Blake Winton (:bwinton) (:☕️)
	Comment 23 • 15 years ago

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.

	Mark Banner (:standard8)
	Comment 24 • 15 years ago

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.

	David :Bienvenu Assignee
	Comment 25 • 15 years ago

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.

	David :Bienvenu Assignee
	Comment 26 • 15 years ago

Bug 515479 filed for removing mail accounts from the old account wizard, for TB.