506290 - [autoconfig] Layout changes to quick account setup

Status: RESOLVED FIXED

(Thunderbird :: Account Manager, defect)

Description

[Andreas Nilsson (:andreasn)]

Attachment: account setup sketch by mpt after discussion at Desktop Summit

The current quick account setup could need some layout and wording changes in order to look more appealing and be more friendly to first time users.

Comment 1

[Andreas Nilsson (:andreasn)]

Attachment: more detailed sketch by me with more details and workflow

Comment 2

[rsx11m]

This certainly looks more compact and also makes a better impression than the current dialog. The idea of letting the window expand to its full size after entering the initial parameters avoids the huge empty space in the current dialog (though that space could be used for some description to guide the user for what's supposed to happen). A couple of remarks to the proposed dialog:

 - The "verification" phase is missing, I assume that's still the case once
   account settings have been identified?

 - Additional server fields for "secure connection" and "secure authentication"
   are needed (the latter is also missing in the current design already).

 - The pencil button is for editing, should one be added next to the user name?

As for entering the password upfront, it is my understanding that the actual authentication information is only utilized at the final verification step.
Some have voiced concerns whether their authentication information is sent to Mozilla (which obviously isn't the case, but the dialog suggests it), thus:

 - Why not asking for the password after the server parameters have been
   determined and the first connection is made for verifying those settings?
   The same password dialog could be used as for regular opening of a folder
   without using saved passwords, along with the "Save password" checkbox.

Comment 3

[Bryan Clark (DevTools PM) [:clarkbw]]

If we can maintain the size requirements and still expand out to the bottom this could work.  In the first revision of this dialog we tried a similar layout but went well beyond 600px in height.  The current dialog is doing side by side because we always have more horizontal space than vertical.

Also the ( manual setup ) would have to follow even down to the bottom during the autoconfig setup as we always need to give people the option to jump out to the adv. setup.

The user details area still looks enabled during auto-detection.  We made this disabled in the current dialog because it's really hard to know what to do if we're detecting a mail server while someone changes their email address.  It's much easier to have a form they have to "enter" to give us a switch for stopping detection.

Comment 4

[Andreas Nilsson (:andreasn)]

Attachment: slightly revisited mockup

here is a slightly updated mockup. Manual setup at the bottom and insensitive top widgets at step2.

Comment 5

[Ludovic Hirlimann [:Usul]]

Why would you ant to try again when everything is green ? shouldn't we disable the button then ?

Comment 6

[rsx11m]

Unless the verification phase is integrated into the status indicators you may get green lights for the connections but nevertheless get a failed setup during verification if authentication doesn't proceed (e.g., due to bug 491202).

Comment 7

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

Attachment: The big warning message.

When you're calculating the minimum size, don't forget the big warning message which is displayed when both the incoming and outgoing servers are insecure...

Comment 8

[Andreas Nilsson (:andreasn)]

Attachment: mockup using real widgets

We wanted to figure out if it would fit on a 800x600 screen using native widgets.
This is Ubuntu in virtualbox running 800x600. It seems like the dialog in this case is taking less space than the current one.

Comment 9

[Andreas Nilsson (:andreasn)]

Attachment: expanded mockup using real widgets

Comment 10

[Andreas Nilsson (:andreasn)]

It bugs me a bit that there is a big empty area in the top right corner (because the error messages go there), but David suggested using a backdrop of the TB logo, so I think that will take care of that.

Comment 11

[rsx11m]

Are you going to look into the suggestions/remarks I've made in comment #2?
I don't see any of those addressed yet in your current mockups...

Comment 12

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to comment #2)

Sorry, I didn't mean to skip these over!

> This certainly looks more compact and also makes a better impression than the
> current dialog. The idea of letting the window expand to its full size after
> entering the initial parameters avoids the huge empty space in the current
> dialog (though that space could be used for some description to guide the user
> for what's supposed to happen). A couple of remarks to the proposed dialog:
> 
>  - The "verification" phase is missing, I assume that's still the case once
>    account settings have been identified?

I'm not sure what you mean by this.  Compared to the final stage of the wizard?  The autoconfig is supposed to show all the settings with green lights to indicate the final state of things before you hit ( Create ), there isn't another stage after that for review.  I'm not sure if that's what you're talking about though.

>  - Additional server fields for "secure connection" and "secure authentication"
>    are needed (the latter is also missing in the current design already).

For secure connection I believe that's what is in the "blah" column, though I'm not sure what is shown is the right order of things.

We have an issue with these detection/editing areas because we could have detected multiple ways of connecting to a server; particularly POP vs. IMAP.  And then beyond just those choices are allowing for custom entry.

AFAIK secure authentication is detected during the final stage of connection and we didn't want to include some kind of check box for it because it's a common error that people turn on and their server doesn't support.

>  - The pencil button is for editing, should one be added next to the user name?

There should be, yes.

> As for entering the password upfront, it is my understanding that the actual
> authentication information is only utilized at the final verification step.
> Some have voiced concerns whether their authentication information is sent to
> Mozilla (which obviously isn't the case, but the dialog suggests it), thus:
> 
>  - Why not asking for the password after the server parameters have been
>    determined and the first connection is made for verifying those settings?
>    The same password dialog could be used as for regular opening of a folder
>    without using saved passwords, along with the "Save password" checkbox.

There were a couple of reasons for this if I remember correctly.  If we used the existing password prompt we couldn't do the inline warning message as shown in attachment 391111 [details] that stops you just before creating the account.

We could, and I believe tried, to ask for the password after we had already done the detection.  However this experience was a little awkward, perhaps it could have been improved.  It worked out that you enter your name, email, then start the config/probing and then we opened up the password entry for use once we detected the settings we needed.  Alternately we could have popped up a dialog... none of those seemed like good options.

I was thinking that we could at least use some space (to the right of the password entry?) to indicate that we don't send the password until you hit the ( Create ) button.  Just so people know we're not sending it off to every server we probe.  I don't want people thinking that we send their password off to Mozilla during this process; though in reality if that was being done there are plenty of other times/places to do it.

Any ideas on how to make people feel safer about the password while keeping the password entry where it is?  I'm kind of stuck.

Comment 13

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to comment #8)
> Created an attachment (id=391576) [details]
> mockup using real widgets
> 
> We wanted to figure out if it would fit on a 800x600 screen using native
> widgets.
> This is Ubuntu in virtualbox running 800x600. It seems like the dialog in this
> case is taking less space than the current one.

It's looking good. It actually makes me think we should be taking up more space than the default that you have shown since it doesn't "feel" like an account setup dialog as it's so small.

(In reply to comment #10)
> It bugs me a bit that there is a big empty area in the top right corner
> (because the error messages go there), but David suggested using a backdrop of
> the TB logo, so I think that will take care of that.

Agreed, I think it could look cool to have a large TB logo that bleeds down such that more of it appears as the dialog expands.

We might also want to use that space for a couple quick points about what this dialog does.  Those could disappear once it changes into the larger detection mode to be used by error messages instead.

- other points -

I feel like the empty text should be showing examples of what to enter.  e.g. [e-mail] instead of repeating the labels that are now beside the entries.

The getting back to the email/password entry doesn't seem clear yet.  I'm not sure if that would mean changing the second ( Cancel ) button into a ( Back ) such that you hit ( Back ) and get to the first stage, then ( Cancel ) to exit.

Thanks for getting this done, it's looking a lot better!

Comment 14

[rsx11m]

Attachment: current dialog: servers ok, authentication failed

(In reply to comment #12)
> Sorry, I didn't mean to skip these over!

No problem, just making sure that nothing get lost... :-)

>  The autoconfig is supposed to show all the settings with green lights to
> indicate the final state of things before you hit ( Create ), there isn't
> another stage after that for review.

Actually, there is, and this also motivated my suggestion for the separate password prompt. The current workflow is shown in the attached picture, for which I've use a non-existent authentication to illustrate that.

After entering the Account Information the connection parameters get determined without user intervention, both "receiving" and "sending" show a green light. After that, you can click on "Create Account". Only then the authentication is actually needed, and obviously failed here, thus I get the error message only after this very last step (inset). Thus, if this two-step process is kept, it would be natural to ask for the authentication only after clicking "Create".

> the existing password prompt we couldn't do the inline warning message as shown
> in attachment 391111 [details] that stops you just before creating the account.

This would work in that case as the certificate warnings would only be shown during the connection-parameter phase, the password prompt comes later.

Comment 15

[Bryan Clark (DevTools PM) [:clarkbw]]

Here are some additional comments regarding the dialog.

Error Message Area

We need to have some examples of how we're going to handle the error messages in the middle area.

Change the ( Try Again ) -> ( Start Over ) and make this a smaller button.


Incoming and Outgoing Server Selection

Selection in the incoming and outgoing server area is a tricky process.  For each system we may have optional values that we have detected which aren't the default best values.  e.g. we detect both IMAP and POP settings but we always prefer IMAP.

This idea came out of bug 422814 comment 201 but we didn't have time to go forward with it.

For the Outgoing Server we have the case where we might not be able to find a server but there are existing servers available which have already been configured for other accounts.  In this error instance I think we can show a drop down list of the available outgoing servers with an option for a custom/new server.

[                                  | v ]
| Default - [e-mail]       |
| Alternate - [e-mail]     |
| Custom...                        |
'----------------------------------'

When Custom is selected we show the individual entry boxes for each value we need to continue probing for that server.

[ Custom...                    | v ]
  [ /host name, mail.example.com/ ]  [ /port/ ] [ /security/ | v ]

If on the outgoing server we detected a viable server we should just show labels indicating that detection.

With the Incoming Server we have the problem that we mail have detected multiple possible incoming servers which are all valid.  e.g.

[ IMAP mail.example.com - 143 TLS  | v ]
| POP  mail.example.com - 67  TLS  |
| POP  mail.example.com - 43  None |
| Custom...                        |
'----------------------------------'

By default we always choose the secure IMAP without question.  However if someone wants to select POP or another incoming server option we should offer that selection as a complete option.

If the person wants to enter a custom host name and port selection we have the custom option available which would bring in the custom entry boxes.

[ Custom...                    | v ]
  [ /host name, mail.example.com/ ]  [ /type/ | v ] [ /port/ ] [ /security/ | v ]

If either of these error without other options (no other default outgoing, no found incoming) we can jump the combobox directly to the custom selection so the person can get started entering the manual settings.

Comment 16

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

Attachment: The first cut at the new error dialog from bug 503436.

Okay, here's the first screenshot(s) of the error dialog on OSX.  Let me know what you think of it, and once you're happy, I'll port the changes to Linux and Windows.

Thanks,
Blake.

Comment 17

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

Attachment: [checked in] The patch to update the warning dialog.

(In reply to comment #16)
> Once you're happy, I'll port the changes to Linux and Windows.

I had some spare time, so here's the patch for the new warning dialog with Linux and Windows working.  It's based on patch #5 for bug 503436 (https://bugzilla.mozilla.org/attachment.cgi?id=393409).

Thanks,
Blake.

Comment 18

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

(D'oh.  I've fixed the acknowledge_warning's oncommand's indentation, and the space before toggleDetails's arguments, and will post them as part of a future patch with any other changes y'all ask for.)

Comment 19

[Andreas Nilsson (:andreasn)]

Attachment: Another dialog mockup

Updated to reflect the discussion (did I miss anything important?)
In some of the dialogs I used the TB logo backdrop. Still trying to decide if it makes it look nicer or if it's starting to look to cluttered.

Comment 20

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 393460 [details]
The first cut at the new error dialog from bug 503436.

The only thing I'm uncertain about is if we should still be including that break down of the host, security settings, and port like we used to (under the technical settings expander).

However so far it's good so maybe we can get some thought on the last pieces and go with this for now.

Comment 21

[David :Bienvenu]

Comment on attachment 393548 [details] [diff] [review]
[checked in] The patch to update the warning dialog.

sr=me for the mailnews parts...

Comment 22

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

(The patch for this is based on top of the one for 503436, so I'm not marking this checkin-needed until that one is checked in.)

Comment 23

[Andreas Nilsson (:andreasn)]

Attachment: in case of password error mockup

After some discussion with Blake and Bryan the other day, we agreed on this one. Not optimal, but a lot better compared to the alternatives :)

Comment 24

[Mark Banner (:standard8)]

Comment on attachment 393548 [details] [diff] [review]
[checked in] The patch to update the warning dialog.

Checked in: http://hg.mozilla.org/comm-central/rev/0eb7935fcb9b

Comment 25

[Bryan Clark (DevTools PM) [:clarkbw]]

just to note that in bug 511308 we're changing names used for launching this dialog.  Instead of 'Quick Account Setup' for the title we're looking at using 'Mail Account Setup'; for the menu item we're looking at File -> New -> 'Mail Account' as the name.

Comment 26

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

In case anyone wants to track this, I'm posting my work in progress in a pbranch named 506290-main-layout-changes at http://bwinton.latte.ca/Programming/thunderbird/autoconfig/

I think it's looking better, but then I do have a slight bias.  :)

Later,
Blake.

Comment 27

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

Attachment: A screenshot for those who don't feel like cloning and building.

Comment 28

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

Attachment: A newer screenshot.

The code for this screenshot is in the pbranch.

Next up, porting the css changes from OSX to Windows and Linux.

Comment 29

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

Attachment: The first cut at the patch to re-layout the main dialog (and enable password editing).

Well, here's another tiny patch for review.  ;)

I think there are still some bugs when we can't find the server, or find it once but edit the settings, and change it to something we can't find, but since they have nothing to do with layout, and since this patch is really big enough already, I think they should probably be fixed in separate bugs.

Thanks,
Blake.

Comment 30

[Peter Weilbacher]

Thanks for adding the screenshots. The latest one leads me to question the point of insecureCleartext.description. It implies that if the configuration contains a SSL or STARTTLS setting (port?), nobody could read it. But AFAIU this really only secures the transit between the sender and that one SMTP server. All subsequent SMTP transactions are still likely to be insecure and there is no way for the user (or TB) to check that. So overall I think this warning will be confusing.

Comment 31

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

(In reply to comment #30)
> The latest one leads me to question the
> point of insecureCleartext.description. It implies that if the configuration
> contains a SSL or STARTTLS setting (port?), nobody could read it. But AFAIU
> this really only secures the transit between the sender and that one SMTP
> server. All subsequent SMTP transactions are still likely to be insecure and
> there is no way for the user (or TB) to check that. So overall I think this
> warning will be confusing.

That's true, but I think that the first SMTP transaction is probably the most important one to secure, since that'll be the one that requires authentication.

But the wording doesn't really convey that, so if you had a suggestion for a better way to say that, I'ld love to change it.

Thanks,
Blake.

Comment 32

[Andreas Nilsson (:andreasn)]

Some quick notes:
* 'Maunual Setup' seems to go to Account Options, where you then can go to 'add account', should it perhaps go to 'add account directly'.
* Trying with a hotmail.com account, it says password is wrong when I'm done, does this mean password isn't actually optional?
* When trying with a live.com account, it's successful, but says 'Undefined' where it should say 'Thunderbird successfully found your settings' (or whatever the exact term there is).

Comment 33

[Mark Banner (:standard8)]

(In reply to comment #32)
> Some quick notes:
> * 'Maunual Setup' seems to go to Account Options, where you then can go to 'add
> account', should it perhaps go to 'add account directly'.

When you go to manual, it adds the account to the account manager so you can tweak it there. Once the defaults are changed, add account directly would come back to the autoconfig setup (via the intermediate select account type screen).

Comment 34

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

When you went to manual, it used to add the account to the account manager, but since we want the Manual Setup button enabled before you enter any other data (if you don't want to go through the probing at all, say), we can't create the account first anymore.

Later,
Blake.

Comment 35

[Mark Banner (:standard8)]

(In reply to comment #34)
> When you went to manual, it used to add the account to the account manager, but
> since we want the Manual Setup button enabled before you enter any other data
> (if you don't want to go through the probing at all, say), we can't create the
> account first anymore.

Then I don't see how all of this will be hanging together. As far as I can understand it all, bug 511308 will replace the current Add Account -> Email Account option with this autoconfig dialog - We won't have the old create email wizard available.

So with this patch the route will be get autoconfig dialog, decide to manual set up, go to add account -> email account, get back to autoconfig dialog.

So either we need to rethink what's happening, or we need to disable the manual setup button until we have at least some details.

Comment 36

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

(In reply to comment #32)
> Some quick notes:

> * Trying with a hotmail.com account, it says password is wrong when I'm done,
> does this mean password isn't actually optional?

It did, but I fixed that.

> * When trying with a live.com account, it's successful, but says 'Undefined'
> where it should say 'Thunderbird successfully found your settings' (or
> whatever the exact term there is).

Fixed.

Thanks,
Blake.

Comment 37

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

And finally,

> * 'Manual Setup' seems to go to Account Options, where you then can go to 'add
> account', should it perhaps go to 'add account directly'.

Reverted to the behaviour described in comment #33.

Thanks,
Blake.

Comment 38

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 398963 [details] [diff] [review]
The first cut at the patch to re-layout the main dialog (and enable password editing).

The new layout looks pretty good.  I only have some string changes and then mostly theme changes.  The theme changes can be a separate followup polish bug after this one.

* Firstname Lastname -> First Last

Lets correct this

* [e-mail] -> [e-mail]

I don't think people really know what "server.domain" means so lets use an honest example

* 'Optional, will only be used to validate the username' -> ''

This text is confusing for people who don't understand it and I believe it's fairly difficult to understand.  Perhaps we could move this into a tooltip for the label or input but I don't like having it stand out like this.  We're balancing bewildering people who don't understand against people who are concerned about something bad happening with their password and I'm going to leave the latter a little unbalanced.

- Some theme changes -

* Edit, Stop buttons

I think we should be using the "smaller-button" class for these (I've only looked at it in Linux so far) but I find it a bit overwhelming with all the button choices available.

The retest button is fine as is because it is the primary button to advance.


* Start Over

I should have pushed for this before but like the previous comment I'd like to reduce the amount of button like items that fill the screen.  Perhaps we can change this to look like an <a>Start Over</a> link instead of a button.

something like:
#back_button { border: medium none ; -moz-appearance: none; color: -moz-nativehyperlinktext; text-decoration: underline; background-color: inherit; cursor: pointer; }

* config_status_title 

This text seems a little too large, I think the bold is enough emphasis.

Lastly a larger string change.  In followup bugs I'd like to address the text for the secure server.  "Congratulations!  This is a secure server." doesn't seem like the right thing to say. :)  I'm not concerned about this because I don't think this text will lead to anything bad, just that it could use some improvement.

Comment 39

[David :Bienvenu]

Might be nice to add a localization note as to what string is going to get inserted here, and a few other places...brandShortName, from what I can tell.
+looking_up_settings=%1$S is looking up the settings for your email account.

Comment 40

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

Attachment: The second cut at the patch, with Bryan's suggestions.

(In reply to comment #38)
> (From update of attachment 398963 [details] [diff] [review])
> The theme changes can be a separate followup polish bug after this one.

Uh, d'oh.  I didn't read this part, and so put most of the theme changes into this patch.  Hope you don't mind.  :)

> * Firstname Lastname -> First Last

Fixed.

> * [e-mail] -> [e-mail]

Fixed.

> * 'Optional, will only be used to validate the username' -> ''

Moved to a tooltip.

> - Some theme changes -
> 
> * Edit, Stop buttons

Changed to smaller-button.

> * Start Over

Changed to look like a link.

> * config_status_title 

Fixed.

> Lastly a larger string change.  In followup bugs I'd like to address the text
> for the secure server.  "Congratulations!  This is a secure server." doesn't
> seem like the right thing to say. :)  I'm not concerned about this because I
> don't think this text will lead to anything bad, just that it could use some
> improvement.

I didn't change these, or file followup bugs.  At least not yet.

Thanks,
Blake.

Comment 41

[Phil Ringnalda (:philor)]

Comment on attachment 399324 [details] [diff] [review]
The second cut at the patch, with Bryan's suggestions.

>+++ b/mail/themes/pinstripe/mail/accountCreation.css
> .errordescription {
>   -moz-padding-start: 4px;
>-  padding-top: 4px;
>-  color: rgb(244,50,50);
>+  margin-top: 3px;
>+  padding-left: 20px;

That'll get you 20px of padding-left in LTR, and 20px of padding-left and 4px of padding-right in RTL. I doubt that's what you want - here and in quite a few other places, every time you type -left or -right you should think about whether you really mean -start and -end, and if you really do mean -left instead of -start, add a comment explaining why it's intentional, because otherwise someone will mass-change it when they realize we've gone back to letting -left and -right slip in.


>+label.autoconfigLabel {
>+  width: 80px;
>+  text-align: right;

I know in 1.9.2, that gets you "r-, please use text-align: end instead" but what does it get you in 1.9.1, where end didn't exist? Did text-align automatically flip itself in RTL, or will that be backward for RTL, and does it want to be backward for RTL, or do we want to just drop the text-align until 3.1 when we'll be able to do it right?

Comment 42

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

Attachment: The autoconfig labels in RTL mode.

(In reply to comment #41)
> (From update of attachment 399324 [details] [diff] [review])
> >+++ b/mail/themes/pinstripe/mail/accountCreation.css
> >+label.autoconfigLabel {
> >+  width: 80px;
> >+  text-align: right;
> 
> I know in 1.9.2, that gets you "r-, please use text-align: end instead" but
> what does it get you in 1.9.1, where end didn't exist? Did text-align
> automatically flip itself in RTL, or will that be backward for RTL, and does it
> want to be backward for RTL, or do we want to just drop the text-align until
> 3.1 when we'll be able to do it right?

It seems to magically work.  :)  Are we going to go through everywhere we do this sort of thing for 3.1, or should we file a bug specific to this case, to upgrade the css when we jump to 1.9.2+?

(I'll go through the other cases, and post a new bug tonight.)

Thanks,
Blake.

Comment 43

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

Attachment: Third time's the charm?

(In reply to comment #41)
> (From update of attachment 399324 [details] [diff] [review])
> >+++ b/mail/themes/pinstripe/mail/accountCreation.css
> > .errordescription {
> >   -moz-padding-start: 4px;
> >-  padding-top: 4px;
> >-  color: rgb(244,50,50);
> >+  margin-top: 3px;
> >+  padding-left: 20px;
> 
> That'll get you 20px of padding-left in LTR, and 20px of padding-left and 4px
> of padding-right in RTL. I doubt that's what you want - here and in quite a few
> other places, every time you type -left or -right you should think about
> whether you really mean -start and -end, and if you really do mean -left
> instead of -start, add a comment explaining why it's intentional, because
> otherwise someone will mass-change it when they realize we've gone back to
> letting -left and -right slip in.

So, I fixed both of the locations I was using -left, and then checked out how the error icon looked in RTL, and it was showing up at the end of the label (uh, that is on the left hand side).  So I started to fix that, and then switched it to live in an hbox of its own, and then finally ended up just using an image, cause it is simple, and it works.

Thanks,
Blake.

Comment 44

[Phil Ringnalda (:philor)]

Comment on attachment 399383 [details] [diff] [review]
Third time's the charm?

>+<!ENTITY startOver.label                 "Start over">
>+<!ENTITY startOver.accesskey             "O">

One or the other of those has the wrong case, whether it's "Start Over" or "o".

>+# LOCALIZATION NOTE: %1$S will be the brandShortName.
>+looking_up_settings=%1$S is looking up the settings for your email account.
> manually_edit_config=Editing Config
>-finished_with_success=Finished
>-finished_with_error=Error
>+found_settings=%1$S has found the settings for your email account.
>+failed_to_find_settings=%1$S failed to find the settings for your email account.

Sadly, what does and doesn't work with l10n tools is pretty anecdotal, but the anecdotes say that you can't do that, that you have to assume that the tool is only showing "LOCALIZATION NOTE (foopy)" and "foopy=" to the user, not showing the whole file, so you have to do three separate "LOCALIZATION NOTE (looking_up_settings): %1$S will be the brandShortName.", "LOCALIZATION NOTE (found_settings): %1$S will be the brandShortName.", "LOCALIZATION NOTE (failed_to_find_settings): %1$S will be the brandShortName."

>+#back_button {
>+  border: medium none;

Doesn't "border: none;" do exactly the same thing, since border-width is optional, and defaults to medium, without making me say "medium none? wtf?"?

>+        <label value="&email.label;" class="autoconfigLabel"
>+               control="email" accesskey="&email.accesskey;"/>
>+        <textbox id="email"
>+                 class="padded uri-element"
>+                 emptytext="&email.emptytext;"
>+                 oninput="gEmailConfigWizard.onEmailInput()"
>+                 onblur="gEmailConfigWizard.validateEmail();"/>

As long as I'm sending you back into the trenches to pick nits, and as long as you're touching most of the XUL already, this would be a good time for this file to pick a single wrapping style and order for attributes, and stick with it. I'm pretty indifferent to which style and order, but since Mnyromyr went to the trouble of typing https://wiki.mozilla.org/SeaMonkey:MailNews:CodingStyle#Sort_XUL_attributes that's probably a good choice.

Comment 45

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

Attachment: [checked in] The previous patch, with philor's nits fixed.

Those all sound good.  I asked about the localization stuff in #l10n, and they said yeah, split them up, and split up the one at the top of the file too.

Thanks,
Blake.

Comment 46

[David :Bienvenu]

Comment on attachment 399462 [details] [diff] [review]
[checked in] The previous patch, with philor's nits fixed.

In general, this looks fine. But this part worries me:

+    if (inServer.password)
+      verifyLogon(config, inServer, alter, msgWindow,
+                  successCallback, errorCallback);
+    else {
+      // Avoid pref pollution, clear out server prefs.
+      accountManager.removeIncomingServer(inServer, true);
+      successCallback();
+    }

Does this mean if there's no password, you won't try to verify logon? If I have a password-less logon, e.g., GSSAPI, which does a cert-based logon, essentially, I won't be able to create the an account via the auto config wizard?

Comment 47

[David :Bienvenu]

Comment on attachment 399462 [details] [diff] [review]
[checked in] The previous patch, with philor's nits fixed.

I'll file a follow up bug on making sure password-less account setup works well, and does a logon verification.

Comment 48

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

(In reply to comment #46)
> In general, this looks fine. But this part worries me:
> +      // Avoid pref pollution, clear out server prefs.
> +      accountManager.removeIncomingServer(inServer, true);
> Does this mean if there's no password, you won't try to verify logon? If I
> have a password-less logon, e.g., GSSAPI, which does a cert-based logon,
> essentially, I won't be able to create the an account via the auto config
> wizard?

(Explained on IRC, but I thought I'ld comment here for posterity.)

Yes, partially.  It won't try to verify login, but it will create the account.  (The call to removeIncomingServer there is to remove the server that's used to test the username/password.  The actual server will be created by a call to createAccountInBackend on line 810 of emailWizard.js.)

Thanks,
Blake.

Comment 49

[David :Bienvenu]

 Bug 515371  filed for verifying password-less logon.

Comment 50

[Mark Banner (:standard8)]

Comment on attachment 399462 [details] [diff] [review]
[checked in] The previous patch, with philor's nits fixed.

Checked in: http://hg.mozilla.org/comm-central/rev/977abd1818d2

Comment 51

[Andreas Nilsson (:andreasn)]

Filed Bug 515390 for some polish issues.

Comment 52

[Peter Weilbacher]

(In reply to comment #31) and (In reply to comment #30)
[... snip (about insecureCleartext.description) ...]

Sorry, I was too slow. I'll shortly open another bug about that.

Comment 53

[Thomas D. (:thomas8)]

This bug should be reopened, because it appears to be morphing into a META / tracker bug for a lot of other bugs that have been added as blocking this bug in the "Depends on" field. "Meta / tracker" should be added to the summary, too.
rsx11m, thanks for tracking my autoconfig bugs.

Comment 54

[Mark Banner (:standard8)]

(In reply to comment #53)
> This bug should be reopened, because it appears to be morphing into a META /
> tracker bug for a lot of other bugs that have been added as blocking this bug
> in the "Depends on" field. "Meta / tracker" should be added to the summary,
> too.

No it shouldn't it has been fixed according to its initial requirements. If you really want a meta/tracker bug then a new one should be opened, however I don't see the need for it in this case really.

Comment 55

[rsx11m]

It's common practice that when you file a regression or follow-up to a bug to make your bug block the original one. This allows everybody participating in that bug (i.e., here) to get notified about some follow-up having been filed (and also receiving a notification once it is resolved). This doesn't make that bug a true meta bug. If you think that I have created dependencies on issues which are unrelated to the autoconfig redesign, they should probably block the original bug 422814 instead.

Comment 56

[Thomas D. (:thomas8)]

(In reply to comment #54)
> If you really want a meta/tracker bug then a new one should be opened, however
> I don't see the need for it in this case really.

Just wait till I'm through with dissecting autoconfig *eg* ;)

(In reply to comment #55)
> It's common practice that when you file a regression or follow-up to a bug to
> make your bug block the original one. This allows everybody participating in
> that bug (i.e., here) to get notified [...]

Thanks rsx11m, that was helpful. Common practices aren't easy to discover sometimes for those who aren't in on it :/ I didn't know you can have a closed bug with open blocking dependants.