Revamp the UI of the account wizard and implement new UX paradigms.

UI mock-ups for various platforms: https://presentator.ura.design/en/XP9IsbuQ?v=0&s=1&m=preview

Here's the first WIP patch to gather feedback and suggestions.

I created a dedicated accountHub.xul to handle the first screen and not affect any other dialog.
The styling has been tested on Linux and macOS for both light and dark mode.

I temporarily added a Open Account Hub link in the main account page (the one you see when you click on your email on the sidebar).

I'm gonna start applying this style to the accountWizard.

Tested on Windows 10. It looks good like your mock-up. :-)

On dark theme, the hover effect of the white button isn't very obvious.

Here's another WIP update.

I started working on the emailWizard.xul form and focusing on the UX of the tooltip fields.
Those elements should behave in this way:

• Normal state: appear on mouse over
• Error state: change icon and persistent visibility unless the user focus/blur on the parent input field

It seems that the moz-appearance tag doesn't adapt to a dark theme, is that correct? If so, yay for more CSS!

(In reply to Alessandro Castellani (:aleca) from comment #3)

It seems that the moz-appearance tag doesn't adapt to a dark theme, is that correct? If so, yay for more CSS!

That's correct. This is why we still have so much light places with dark theme. :-(
And you need -moz-appearance: none; with styling everything in CSS.

Only Linux adapts the colours when using a dark Linux theme.

It seems that the moz-appearance tag doesn't adapt to a dark theme, is that correct? If so, yay for more CSS!

That's correct. This is why we still have so much light places with dark theme. :-(

[slightly offtopic]
Instead of changing all the CSS, wouldn't it make more sense to adapt moz-appearance? In C++, system style sheets, or wherever it's defined?

(In reply to Ben Bucksch (:BenB) from comment #5)

[slightly offtopic]
Instead of changing all the CSS, wouldn't it make more sense to adapt moz-appearance? In C++, system style sheets, or wherever it's defined?

If I'm correct, it's defined here: https://searchfox.org/comm-central/search?q=lookandfeel&case=false&regexp=false&path=

I'll push forward ignoring the dark mode for now.
I'll then do a full CSS review and force dark-mode style per-platform.

If you guys have time, it would be helpful to have some screenshots from Windows and other Linux distros.
You can upload them here or send them to me via email.

Cheers

Screenshot from Windows.

Why have feed accounts been removed and marginalized? Is there data showing more calendar or chat or filelink accounts than feed accounts? Or has this been decided in some other way.

Itty bitty progress here.
Here's a recap of what you can expect if you want to take this patch for a spin.

• On TB startup, if you don't have any account setup, the new Account Hub will be launched.
• The Setup email and Create new email buttons are now opening the related modals.
• Loading messages and Warning messages styled
• A bit of initial work on the manual config section.

The manual config still needs to be rewritten completely as it's based on HTML tables instead of XUL elements, which makes the whole thing less flexible.
So, when that appears the UI breaks.

I should be able to complete that tomorrow alongside the various warning messages. Once that's done, the email wizard model will be completed with the new strings and the new style.

If you decide to test it and you see some quirky UI behaviour, please attach a screenshot.

This took me way longer than expected.
The email setup wizard has been updated with the latest and greatest UI.

Since we're at the end of the week, we need to decide what to do with this patch.
Due to the limited time, I don't think I will be able to complete any other dialog workflow (calendar, chat, filelink, etc), but I think it would be a waste to not land the new email account wizard since it brings a much needed UI and UX refresh to the whole process.

Here's my proposal.
We temporarily hide the new Account Hub modal, but we leave the code in, and we keep using the current emailWizard on first startup, but with the new UI.

This will allow us to ship all the strings for future work, and also land something that is visually drastically different that for sure will catch the attention of the users.
We could use it as a "Preview" of what's to come.

Also, I'd like to propose the temporary removal of the "Get a new email address" button since that modal needs to be nuked and rewritten completely. We're also only offering 1 provider, which isn't that great for a feature that prominent on first start.

Thoughts?
Feel free to review the patch, simulate some common actions, and share some screenshots if something weird comes up.

If we decide to land, I will be focusing on updating the tests and upload it for a full review.

The first dialog looks good. But when it shows the found configuration, it has scrollbars and doesn't expand to the right size.

The darker background in the results has also almost the same colour as the menulists, not so optimal.

I also added a screenshot of a not found configuration with the notification in it, so you know how it looks.

(In reply to Richard Marti (:Paenglab) from comment #12)

Thanks for the screenshots and feedback.

The first dialog looks good. But when it shows the found configuration, it has scrollbars and doesn't expand to the right size.

The dialog should update it's height to accommodate for the extra content. I see it does that but not entirely.
I'll fix it.

The darker background in the results has also almost the same colour as the menulists, not so optimal.

Yeah, it blends a bit too much

I think this is ready for a full review.

Regarding the menulist elements blending with the background, that's unfortunately the default style of the -moz-appereance attribute.

(In reply to Richard Marti (:Paenglab) from comment #16)

When it found a config, the scrollbar is still visible.

Thanks for the feedback, this should be fixed.

(In reply to Magnus Melin (:mkmelin) from comment #15)

While these looks nice as a filler in the footer of the dialog, not sure it makes sense to have donation in this dialog.

We can remove it from the Account Hub modal if we want, once we decide to activate that dialog, but I'd like to keep this as an available string in case we decide to use it.
What do you think?

(In reply to Alessandro Castellani (:aleca) from comment #17)

Created attachment 9065882 [details] [diff] [review]
We can remove it from the Account Hub modal if we want, once we decide to
activate that dialog, but I'd like to keep this as an available string in
case we decide to use it.
What do you think?

I think we should remove it. Adding it is adding work for 63 localizers, and I just don't see us including it in there. It is a place where where we could potentially have commercial partners' offers featured, and if we add donations in there it would be quite a mixup.

(In reply to Magnus Melin [:mkmelin] from comment #20)

I like the looks quite a bit, it's much more modern than what we use to have.

Yay :D

Not sure why the dialog is so narrow though. It's like it would be for
mobile.

It's made on purpose to keep the user's focus on a small and tight area.
This is mostly recommended when dealing with forms, instead of letting the user's eye bounce right to left on a wider dialog.

• the buttons got scrolled out of view once it finds the configs (on linux
  at least)

Argh, I thought I fixed that as it doesn't happen for me (elementary OS) and Richard confirmed it doesn't happen on Linux either. Can you upload me a screenshot so I can see if maybe the Ubuntu stylesheet is doing something strange?

• the "no encryption" warning page is not at all fitting in the dialog, and
  parts of the text is out of view (try setting the incoming server to
  imap.netti.fi)

I never got this warning, can you upload a screenshot?

• changing from pop to imap doesn't update the server name (might be an
  existing bug?)

It might be as I barely touched the JS only for those parts interacting with the UI.

When the settings are found, it shows like

Incoming: IMAP imap.netti.fi No Encryption
Outgoing: SMTP smtp.netti.fi No Encryption

I think this ^^ is a bit odd. For outgoing it's never anything else than
SMTP. For incoming, I'd also not show the protocol there.

Sounds good to me. Those were always like that, but I'll remove them as it's a bit redundant indeed.

For the encryption, it's not quite clear what it is when it's pulled
together like that.

I've no idea, sorry, I never got that error message and I didn't touch the methods returning those strings.

Thanks for the detailed feedback and screenshots.
I implemented a more strict size check that reverts back to the default size once the scrollable area shrinks back.
This should take care of all the necessary size changes when the content is to big to fit.

+<!ENTITY feeds.label "Feeds">
+<!ENTITY feeds.newButton "Subscribe to Feeds">

This should be "Create a New Feeds Account".

I think for completeness we should instead of Other add Address book, Feed and Newsgroup setup options. (No movemail though)

The latest does not seem to address Magnus' comment 20 nor did you reply to comment 9 (I'm the feeds maintainer/peer - maybe you didn't know that - and will appreciate your courtesy is letting me know about changes to things having to do with the module, thanks).

"Subscribe to Feeds"
This should be "Create a New Feeds Account".

"Subscribe to Feeds" is better.

Users don't think of this as an "account". If RSS needs an account created first, and then to subscribe, then this should happen in the background and automatically. The user action is just "I want to read this newspaper in Thunderbird", not "I create a Feeds account". Even I as Thunderbird developer wouldn't know what a "feeds account" is, much less a normal user. Please think from a user perspective.

(In reply to Ben Bucksch (:BenB) from comment #30)

"Subscribe to Feeds"
This should be "Create a New Feeds Account".

"Subscribe to Feeds" is better.

Users don't think of this as an "account". If RSS needs an account created first, and then to subscribe, then this should happen in the background and automatically. The user action is just "I want to read this newspaper in Thunderbird", not "I create a Feeds account". Even I as Thunderbird developer wouldn't know what a "feeds account" is, much less a normal user. Please think from a user perspective.

No. It doesn't subscribe to any feed/feeds, so that is a misleading and false description. If there is no account existing, certain dnd operations of a feed link onto Tb would autocreate an account with a subscription. But not in this flow. Each feed subscription in an account is equivalent to a mail server/inbox. As far as not knowing how a Feed account works, perhaps the guide will help: https://support.mozilla.org/en-US/kb/how-subscribe-news-feeds-and-blogs.

Further, the exact equivalent is a newsgroup account, which itself only contains subscriptions.

OK, understood, but a user won't understand that. So, what we need to do here is to make this dialog really do a subscription, not create an account. If no account exists yet, it should be created transparently and automatically in the background. You cannot explain a user the concept of a feeds account. It makes no sense to them. It's a purely technical thing in Thunderbird, and other feed readers don't have it, either.

Given that this is a new dialog, we can make it what we want, and I think Alessandro is right to see this from a user perspective and make the software do what the user wants.

I don't think you understand at all. There is no such thing in this flow of knowing what specific feed url the user wants to subscribe to, just as there isn't for newsgroups when that account is created.

The claim that a user won't understand that is based on what? I have never seen, in any bug or support forum, any confusion at all. Please stop creating a false issue and advocating for flat out incorrect wording for what happens in the dialog.

I think the misunderstanding is about "this flow" and "this dialog". This is not the "Create an RSS account" dialog. This dialog here with "Subscribe to Feed" button is a completely new dialog. From what I understood, Alessandro is creating a new flow for RSS here. The dialog can easily show an input field for an RSS/Atom URL, and then do the same thing that you do for Drag&Drop ("dnd operations of a feed link onto Tb would autocreate an account with a subscription"). And Alessandro is right, that is what should happen.

Anyway, this bug is not about RSS. It's just one button of many. Let's break this discussion about RSS into a new bug.

(In reply to Magnus Melin [:mkmelin] from comment #28)

Re the "no encryption warning": we should really change the looks of that,
potentially in another bug though. But, if I click open the details there,
the dialog resizes. I'd expect the dialog never to resize, but be properly
sized to start with and the internal elements would get scrollbars as
appropriate (that is, scrollbars on the actual warning content, not a
resized dialog)

Yes, I totally agree. The original idea is to never resize the dialog.
We can definitely improve this in a follow up bug as the UI of these warnings needs to be rethought and improved to fit in the dialog.

We might want to change these to <html:input type="text" and <html:input type="email" while we're here.
They will be within weeks anyway

I'm having issues in making the <html:input> grow 100% of the width of its container. It doesn't seem to respect the flex attribute as the other XUL elements. Is this a known issue with a workaround?

which would also let you just use title="&name.text;" etc to get the tooltip

Mh, using title="&name.text;" on the input field doesn't trigger any tooltip for me. I'd recommend anyway to keep the tooltip to be triggered by the right icon as it's more discoverable and commonly used.

But, not sure we need this? The window can be closed, but that's not advisable for someone coming here for the first time at least (can be difficult finding one's way back, we may even want to prevent it).

I think we should keep this. There was a long discussion in tb-planning related to this as to show the user an easy path to close this dialog.
The issue is mostly with the current UI of TB, which as you say doesn't offer an easy way back to this dialog, something we need to fix in the near future.
Putting the "Set Up Later" link also helps us communicate to the user that this is not a one time only dialog and it can be reopened at a later time. Right now, it's unclear how to close it other than the X or ESC and it doesn't give any info regarding its future availability.

(In reply to alta88 from comment #34)
Hi alta88,
As Ben correctly wrote, this bug is not in any shape or form changing the current Feeds dialog.
This bug is updating the UI and UX of the Email Wizard dialog, plus, is adding some translatable strings for a future integration of a new Account Hub, a unified dialog that will guide the user and help them to choose what they need to set up.

I'm not sure why your dialog is so narrow.
It should be triggered with a 440px width with plenty of space.

Here's an updated patch with the latest changes, except the replacement of the <textbox> with the <html:input> element as I'm trying to figure out how to force its width to grow at 100%.

Hey Alessandro,

thanks for the nice patch!

I've looked over it only quickly, not in depth. But generally, this looks like good work and reasonable changes.

I could not review the XUL, because there are too many changes to read the diff. I will just trust you that you did not make substantial changes, just moved the elements around.

A few small code comments:

1. window size: You should not set both width and height to an absolute value in pixels.
   a) Setting absolutes sizes in pixels is this problematic on different platforms and with different font size settings of the user, e.g. because he's old and does not see well (accessibility etc.). I try to use em where possible.

b) Setting both width and height leaves the layout no chance at all to fit all content, and will force scrollbars when there's too much content. We should never ever have scrollbars in windows, even if the user changed font settings. Set either width or height, but not both.

This is the reason why you have problems with the scrollbars, as some of the screenshots show. Fix this (both the px and remove the absolute height), and the scrollbars should sort themselves out.

2. You add a special case to call stopSpinner with no actionStrName argument. But then it calls showStatusTitle(actionStrName), which will fail. It think the case if (!actionStrName) { should be removed. Alternatively, if it's really needed, then you need to make the showStatusTitle() call conditional, too. And please test this case.

let type = gStringsBundle.getString(sanitize.translate(server.type,

Please keep the type in the result display. It may be implicit in the hostnames that you use, and you also have a IMAP/POP3 toggle, so in such cases, it would be duplicated. But that's not always the case. Not all providers offer both choices - in fact most providers so not provide 2 choices -, and not all of them use "imap" in the hostname. So, the protocol should stay in the display. It's a key part of the config.

That also means that line
hostnameE.querySelector(".protocolType").hidden = true; // already have a nicer label
needs to stay.

If the duplication with the radio selector and the label is really disturbing you, you could make the display of the protocol label conditional on whether there are multiple configs. But it would complicate the code further.

4. Placeholder: "Your full name"
   <user>What is a "full name"? Is that ben\MYDOMAIN or ben@mydomain.com or ? </user>
   This is not sufficiently clear that we mean the real name, with first name and last name, in this order.
   This is particularly important as some countries write "BUCKSCH Ben" or "Bucksch Ben" or "Bucksch, Ben", which will be confusing in an international context. That's particularly true for other languages, where I cannot even tell which is the first or last name. We should make clear that the user should write "Ben Bucksch", i.e. first name, then last name.
   Key words would be "real name" and "first name" and "last name", and it should be visible before they enter anything, not just in the help text.

5. Likewise, email address placeholder was "you@example.com" and you change it to "Your email address". The placeholder text should be an example of what you would write, not a description. An example is always more direct and more approachable than a description. The label is already the description.

6. It makes sense to move the Advanced Config button into the manual edit section. But please, could you make it less prominent? I really want to discourage people clicking on it. Can you make it a button, much smaller, and all the way on the bottom right? Still in the dark grey section, but a little lower, and right above the Cancel button. I want it really out of the eye focus of the users, so that nobody is tempted to click on it who doesn't know exactly what they are doing.

7. Nice idea with the "show password" toggle. I like that, also that it's an icon inside the field.

8. Nice creating the uppercase label with CSS text-transform: uppercase; instead of the string. This allows us to change it later without re-translation. Good work!

replacement of the <textbox> with the <html:input> element as I'm trying to figure out how to force its width to grow at 100%.

Not sure which <textbox> you're talking about. I don't see any <html:input> in the patch. And that's good. If you're talking about the emailWizard.xul, please do not change the <textbox>, because its behavior is different from <html:input>, and we have a lot of events and behavior and CSS attached to them, and changing it will likely break stuff without careful testing.

Typo: all the way on the bottom right
Correction: all the way on the bottom left
(and smaller than it is now)

I think another reason where the scrollbars come from might be the innerWidth = scrollWidth. It doesn't include margins, nor margin or padding of the <window>, and other stuff that might take space that cause the size to be off by a few pixels. There were also strange interactions with the scrollbar being calculated, even though it's no longer necessary after settings the size, or odd stuff like that. I don't remember the details, but I remember also fighting with that "off by a few pixels" bug.

I would simply set the width (which you seem to care most about about) to a value (relative to the font, e.g. em), and then set a minimum height, to avoid too many changes. Then, try whether sizeToContent() does the right thing in this case - I don't know whether it respects the width you set or not. If that doesn't work, find another way to set the height (leave the width alone), but keep margins and other elements in the calculation.

If this is too difficult, maybe leave this window size stuff out of the patch, land the strings, and fix the window size after.

(In reply to Alessandro Castellani (:aleca) from comment #37)

(In reply to alta88 from comment #34)
Hi alta88,
As Ben correctly wrote, this bug is not in any shape or form changing the current Feeds dialog.
This bug is updating the UI and UX of the Email Wizard dialog, plus, is adding some translatable strings for a future integration of a new Account Hub, a unified dialog that will guide the user and help them to choose what they need to set up.

Hi aleca,
It's apparent that this patch is a nonfunctional wip, with a dialog variation of the Account Central new account section[1]. For other than email, it's impossible to tell whether the string is correct for any other account type, and more impossible given some future thing that "could" be implemented that's different from what is implemented.

Although some seem to believe a new flow has already been designed, I haven't seen any such mockups or discussion in either the tb-planning thread or in topicbox relating to anything other than email. If they exist, please inform us where; it would be unhelpful to do this in a closed and opaque manner.

Also, what is the point of all this slightly modified new UI if it's not being done in html? See Bug 1540278 for what's coming in core. And here's a mockup: https://bugzilla.mozilla.org/attachment.cgi?id=8985554

[1] Bug 529131 unified account type creation again, after the email new account/get new email implementation fractured the original unified account wizard.

Hi alta88,
Apologies for the late answer and for accidentally "ignoring" comment #9, sometimes it's hard to keep up with the daily amount of emails and messages, and some topics slip away. I wasn't ignoring you.

It's apparent that this patch is a nonfunctional wip, with a dialog variation of the Account Central new account section[1]. For other than email, it's impossible to tell whether the string is correct for any other account type, and more impossible given some future thing that "could" be implemented that's different from what is implemented.

This patch is primarily a UI/UX enhancement and a first step towards a unified Account Hub to better guide users in setting up what they need.
We discussed a lot about the pros and cons, and went through multiple iterations with hundreds of emails in tb-planning for the past 2 months.

This initial patch is focused on improving the Email Wizard dialog, which everyone agrees that in the current form, is not the most appealing part of TB, and from a purely UX point of view, it doesn't communicate to the user a message of modernity, polish, and stability, like we want for TB.

Alongside, due to the string freeze for TB 68, we're using the occasion to push some translatable strings to accommodate the future implementation of the Account Hub dialog, which is currently inactive and it doesn't change or affect the current workflow of any account creation dialogs, including the Feeds.

Although some seem to believe a new flow has already been designed, I haven't seen any such mockups or discussion in either the tb-planning thread or in topicbox relating to anything other than email. If they exist, please inform us where; it would be unhelpful to do this in a closed and opaque manner.

No one believes that a new flow has been designed. As I said, this was discussed thoroughly in tb=planning, and the initial step to implement an Account Hub has been approved by the module owners, and it currently being carefully evaluated and reviewed by Magnus and Richard, both module owners and in charge of Theme and UX decisions.

I don't understand your concerns and negative feedback, since all of this is not news and we've been working on it for months, publicly sharing everything. I'd like also to ask you to please stop using phrases like do this in a closed and opaque manner. It is extremely wrong accusing such behaviour and it seems like it is the first reaction everyone has as soon as a change they don't like gets introduced.

I'm always happy to receive feedback and constructive criticism, and I think TB can only benefit from all of us working together without constantly going down the path of accusing each other of some mischievous behaviour or hidden agenda.

Also, what is the point of all this slightly modified new UI if it's not being done in html?

I'm sure Magnus can speak for this better than I can, but I can say that it's mostly a matter of resources and time.
We're slowly replacing discontinued XBL and XUL components with custom elements and HTML elements, following along the updates and patches that lands in FF. We're a small team with limited resources and we have a lot to do. I think fixing where it's needed and following along what happens in FF is less of a burden compared to rebuilding everything from scratch for something that will happen in the near future.

It's a bit disappointing knowing that you consider this just a slightly modified new UI, considering I've been working on this for months and I'm trying to bring my experience and knowledge to TB in order to improve it and make it a better and more usable software.
If you think this is just a superfluous update, it means I didn't do a good job of communicating the scope and the reasons behind it.

I'd like for Ryan, Magnus, and Richard to chime in, especially regarding your attachment https://bugzilla.mozilla.org/attachment.cgi?id=8985554, which I wasn't aware of work being done in that area.

Cheers

TB can only benefit from all of us working together

+1

https://bugzilla.mozilla.org/attachment.cgi?id=8985554

It's great to put the accounts list into the main preference dialog. I'm all in favor of that.

However, the account creation dialog should stay a dialog, in exactly the form that Alex is working on. It could be simply a [+] button on that account manager, which then opens the account creation dialog/hub.

The controversy is mostly the wording of a not yet implemented functionality. The biggest problem can be that the string that needs to land for 68 is not the correct one. When this is the case, we need to look then how to go.

So I think, Alessandro go further with your great work here and then, when the feed account work will be done, the discussion can go into the corresponding bug.

https://bugzilla.mozilla.org/attachment.cgi?id=8985554

This looks very good, also when the list on the left could look unclear with many accounts. And like Ben wrote, the account creation should stay as a dialog to have the focus on it and not let the user click on other items in the prefs tab during this process. When opened from the prefs tab, the dialog can be opened as a in-content dialog like we do actually for the dialogs in the prefs.

(In reply to Alessandro Castellani (:aleca) from comment #45)

Hi alta88,
Apologies for the late answer and for accidentally "ignoring" comment #9, sometimes it's hard to keep up with the daily amount of emails and messages, and some topics slip away. I wasn't ignoring you.

It's apparent that this patch is a nonfunctional wip, with a dialog variation of the Account Central new account section[1]. For other than email, it's impossible to tell whether the string is correct for any other account type, and more impossible given some future thing that "could" be implemented that's different from what is implemented.

This patch is primarily a UI/UX enhancement and a first step towards a unified Account Hub to better guide users in setting up what they need.
We discussed a lot about the pros and cons, and went through multiple iterations with hundreds of emails in tb-planning for the past 2 months.

This initial patch is focused on improving the Email Wizard dialog, which everyone agrees that in the current form, is not the most appealing part of TB, and from a purely UX point of view, it doesn't communicate to the user a message of modernity, polish, and stability, like we want for TB.

Alongside, due to the string freeze for TB 68, we're using the occasion to push some translatable strings to accommodate the future implementation of the Account Hub dialog, which is currently inactive and it doesn't change or affect the current workflow of any account creation dialogs, including the Feeds.

Although some seem to believe a new flow has already been designed, I haven't seen any such mockups or discussion in either the tb-planning thread or in topicbox relating to anything other than email. If they exist, please inform us where; it would be unhelpful to do this in a closed and opaque manner.

No one believes that a new flow has been designed. As I said, this was discussed thoroughly in tb=planning, and the initial step to implement an Account Hub has been approved by the module owners, and it currently being carefully evaluated and reviewed by Magnus and Richard, both module owners and in charge of Theme and UX decisions.

I'm well aware of the tb-planning discussion. I said above, the comments are for "other than email". Comment 35 implies a new flow which requires new strings, which is at the moment, wrong.

I don't understand your concerns and negative feedback, since all of this is not news and we've been working on it for months, publicly sharing everything. I'd like also to ask you to please stop using phrases like do this in a closed and opaque manner. It is extremely wrong accusing such behaviour and it seems like it is the first reaction everyone has as soon as a change they don't like gets introduced.

I'm always happy to receive feedback and constructive criticism, and I think TB can only benefit from all of us working together without constantly going down the path of accusing each other of some mischievous behaviour or hidden agenda.

Please don't take unnecessary offense, or inflate or selectively quote my comments, which are very mild. A new flow has clearly been implied, for non email, starting in comment 30 and confirmed in comment 37, and there hasn't been any discussion I've seen, thus leading to a request to be open. That's all.

Also, what is the point of all this slightly modified new UI if it's not being done in html?

I'm sure Magnus can speak for this better than I can, but I can say that it's mostly a matter of resources and time.
We're slowly replacing discontinued XBL and XUL components with custom elements and HTML elements, following along the updates and patches that lands in FF. We're a small team with limited resources and we have a lot to do. I think fixing where it's needed and following along what happens in FF is less of a burden compared to rebuilding everything from scratch for something thaat will happen in the near future.

Now this I very much disagree with. I believe it will, in fact, result in a duplication of effort requiring more resources in the end and is an inefficient use of donor funds. Any revamp of Account Manager should be html from the start. The not very complex "hub" dialog is a great place to start (<dialog>) getting our feet wet. I have not made any comment regarding the email setup functional changes or flow other than html.

I understand your reasoning, alta88, for not wanting to merge something only to have it need to be changed in the near future. How can we move forward in a positive way?

I know that everyone on this thread has a deep passion for getting quality work into Thunderbird. Let's brainstorm some good ways forward that we can all live with. I would encourage Paenglab, Alessandro, and alta88 (and Magnus and Ben if they can) to hop in a chat on IRC and hash this stuff out so that we have a shared vision for what we want, as we're all working to make the best open source email client on the planet. :)

Let's bring new life to this bug that has been left behind for too long.
In this new updated WIP patch, I'm using em values for the dialog size.
I reworked a bit the style of the warningbox, and refreshed other UI elements.

The dialog shouldn't trigger any scrollbar anymore, and the warningbox will have its own scrollbars if the content is too big.

Question for Ben.
I noticed there are a bunch of panels at the beginning of the emailWizard.xul file, like the <panel id="insecureserver-cleartext-panel" class="popup-panel">. Are these used anywhere?

Regarding the string changes for the placeholders
Magnus requested those changes, and I personally don't mind them since they're now consistent with each other. I will let Ben and Magnus discuss and find an agreement for this matter.

Regarding <html:input>
I converted those textbox elements to HTML input and everything works as expected. All the events and actions are properly working, and the relative tests are passing.
Since those XUL elements are getting deprecated in a few weeks, we should use the occasion to follow along.

Regarding the Advanced config button
I converted that one into a link button to make it less prominent. I prefer to leave it right aligned at the bottom of the manual config area as it's more balanced with the other elements. It really looks odd left aligned.

What's next

• Right now, the dialog changes its height to accommodate changes in the content. Overall is not too jarring and sometimes it's hardly noticeable, but I will keep working to implement the suggestion of Magnus, which is to let internal containers have optional scrollbars in case the content is too big.
• Cleaning up all the outdated CSS in the various accountCreations.css files
• Fixing 1 failing mozmill test
• Something else I'm sure I'm missing right now

I noticed there are a bunch of panels at the beginning of the emailWizard.xul file, like the <panel id="insecureserver-cleartext-panel" class="popup-panel">. Are these used anywhere?

Yes, if there are there, I assume they are also used :-).

Have you tested servers without SSL? Drop into manual config, and enter non-SSL servers. Check for any warnings already in the dialog. If you want to use the config, you should get another red warning page inside the dialog that explains the impact of non-SSL servers to end users.

Also test the flow with an ISP DB config that comes back with non-SSL servers. Try foo@versatel.de as email address.

https://autoconfig.thunderbird.net/v1.1/versatel.de

Regarding <html:input> ... I converted those textbox elements to HTML input and everything works as expected.

I know that Magnus asked for that, but like I said at the very end of comment 40: Engineering-wise, that's a very bad idea. Given that the events in this dialog are fragile (witness the subtle breakage when we changed the editable dropdowns), the change from <textbox> to <html:input> is regression prone. It has nothing to do with this bug, and good engineering practices demand that they are not combined. I think this is a mistake to land it both at once in the same patch. It makes it much harder to find the cause of any possible regressions later on.

I've emailed maildev about this general problem. See http://lists.thunderbird.net/pipermail/maildev_lists.thunderbird.net/2019-June/001710.html , msgid 96d6e79c-6c69-9172-702e-943012c4a28c@beonex.com , subject "Atomic changes".

You're just trying to follow reviewers, so I won't block the patch due to this change. But please make triple sure that nothing broke, with all cases and all edge cases, and by manually testing all possible DOM events that you find the the code.

I converted that one into a link button to make it less prominent. I prefer to leave it right aligned at the bottom of the manual config area as it's more balanced with the other elements. It really looks odd left aligned.

I see what you mean, but the impact of a curious user clicking on it is bad. On the bottom right, it's close to the Done/Continue button, so in the user eye focus, so the likeliness of the user noticing it and clicking it out of curiosity or a "I'm feeling brave" notion is very high. That is not the case with the current dialog, where the button is on the left and out of eye focus.

With tens of thousands of users every day (!) going through this dialog, even a small percentage of "I'm feeling brave" users cause a lot of trouble. So, this is important to get right.

Could you please find another arrangement of visual style so that you can place the button on the left, or otherwise out of eye focus of a normal user who is just trying to finish the dialog?

Dialog size:

There should be no scrollbars in a UI dialog, ever, anywhere. That is always a bug. (Only exception is when we display a large body of text.) Do not make internal sections scroll, that would be a bug. It would also be pointless, given that a desktop computer has enough screen real estate for all widgets here, even on 1366x768 displays.

Personally, I think it would be OK to change the height of the dialog once when we change into manual edit mode. It could then stay at this larger size. You be the judge of that.

The default size should be so that the height switching normally does not happen and happens only in exceptional cases (e.g. language translation with huge labels, or user changed font size).

This should be ready for a full review.
I tested various scenarios, including expired SSL certificate warnings, long strings, and various settings.
Everything should be covered.
All the textbox elements have been replaced with html:input elements, which have been tested to be sure all the methods and behaviours remained the same.
The mozmill tests have been fixed as well.

Alessandro: There are several SSL problems that need to be tested:

• Invalid SSL certificate, due to e.g. expired certs, or wrong domain
• The user configured a non-SSL server, and we warn him.
• The user configured a non-SSL server and plaintext passwords. IIRC, we have a specific warning for that.

Exchange section cut off

Solution options:

1. Make the window wider, at the moment we start this mode. (But do not resize the window smaller again.)
2. Change the arrangement of this section to have less width.

Once fixed, mark this screenshot attachment obsolete.

We have 2 cases here:

• The addon is not yet installed and needs to be installed (that's what's on the screenshot)
• The addon is already installed and we just show a result of an Exchange server and the [Done] button

This should be a simple fix. We might simply make the width of the text smaller, resulting in 1 additional line each.
We might fix this ourselves and attach a new patch.
But please do not land as-is.

Beautiful!!!

Hey Ben, thanks for the review and thorough walk-through.
I'm updating the patch to deal with the exchange warnings.

Here's a screenshot with SSL and No Encryption warning states.

It was even an easier fix than we thought.
Before we were using row elements to generate the add-on section. Converting those into a regular hbox, with a flex="1" attribute to its container, allows the label to go on another line when the text is too long.

Updated patch with the exchange fixes ready for review.

(In reply to neil@parkwaycc.co.uk from comment #65)

Out of interest, I was able to fix the display by adding flex="1" to the
middle column. (But I think <grid> is deprecated anyway?)

Hey Patrick, thanks for the heads up.
I don't know if the grid element is getting deprecated, but I removed that section because it wasn't used at all.
All those columns with IDs were not used at all, as all the elements are generated in JS and appended in the section.
Now everything works as expected with a single vbox and hbox.

<grid> is indeed going away (I think in some months)

Updated, thanks for checking.

(In reply to alta88 from comment #48)

Also, what is the point of all this slightly modified new UI if it's not being done in html?

Part of this could be done in html, and perhaps the account hub should use that from start.
The xul -> html migration will come gradually for most windows I think, and there are plans for tools to help that transition. For existing UI, we can just take it one step at a time.

For existing UI, we can just take it one step at a time.

Yes, please: https://en.wikipedia.org/wiki/Scope_creep

Let's do this in steps, one aspect per step / patch / commit. HTML rewrite can be done outside the scope of this ticket, and should be. Otherwise, we have too many things changed at once, this will never finish, and if it does and there are bugs, they will be hard to find and fix the more changes are in the patch.

Thanks for the review.
So, based on the current situation and your feedback, here's the plan of action for the next few days in order to land this.

• Remove the Account Hub first screen and related files from the patch, make it smaller and only tackle the email wizard workflow.
• Leverage the built-in HTML5 form validation, when possible, and thoroughly test this layout with accessibility tools.
• Don't add any more dtd or properties files and handle new translations via Fluent.

Does that sound good?
When updating strings in pre-existing dtd or properties files, should I keep using those or create a new Fluent file?

For the info tooltip thingy, a standard tooltip would be accessible (title="foo"). I think those fields (name, email)
aren't really too difficult to understand so perhaps we can just remove that part.

I'd recommend to leave them to maintain visual consistency with the errors icons and use them to better specify the necessity of writing your full name and an existing email account.

<tooltip> is also a bit special. I'm not sure what XUL removal will make of that.... At any rate, seems problematic.

What should I do then? Not use this element entirely? I can potentially drop this, use the title attribute to handle mouse hover tooltips, and print the error messages underneath the related fields. It'll visually change the form a little bit, but we should have enough real estate to handle these changes.

I'll post some screenshots once I'm closer to uploading the revisited patch.

When updating strings in pre-existing dtd or properties files, should I keep using those or create a new Fluent file?

I think Magnus only referred to new DTD files, not existing ones. Even a few new strings in an existing dialog should be added to the existing files, not in a completely new file with a new method. A migration of existing dialogs to Fluent should happen in a different bug.

Re <tooltip>s, I think what you have looks great. It think it should be enough for Thunderbird 68.

I would recommend against putting errors underneath the fields, because it will either make all the below fields move back and forth (horrible user experience - move the field while you're trying to interact with the form - can't be worse), or you need to leave more 1 line plus margins between the fields, which will look odd.

I like the design that you have right now. We can remove <tooltip> when it comes to that, in Thunderbird 69, but it won't be next week anyway. Let's concentrate on shipping 68.

(In reply to Alessandro Castellani (:aleca) from comment #73)

Does that sound good?

Sounds good.

When updating strings in pre-existing dtd or properties files, should I keep
using those or create a new Fluent file?

Probably better to keep those as old style.

What should I do then? Not use this element entirely?

Probably not no. Something that makes sense semantically should also work with a11y. (You don't usually put error messages in tooltips.)

After spending some time in trying to adapt this to what we talked about, I'm more and more convinced this implementation won't happen any time soon.

The main problem is related the first screen with the input fields.
The whole new look and experience was heavily relying on tooltips in order to declutter the interface and prevent size changes.
Tooltips are not accessible, therefore shouldn't be used for important messages.
Using built-in html5 validation is not a perfect solution either, as it behaves differently from a webpage.

Here's the list of problems I stumbled upon:

• The title tag doesn't show any label or tooltip while hovering over an image, so the info icons are basically useless.
• I tried to adapt the fields and use built-in html5 validation, with required, pattern, etc, and tried to leverage the checkValidity() but other than applying a red outline to the form, no other visible benefits are noticeable.
• I also tried to use the reportValidity() method, by wrapping the fields around a form a simulating a submit to trigger the built-in error messages. That doesn't work at all.
• Since we're not using a real form submit, all these html5 validations need to be triggered manually in JS because adding the required attribute to the fields makes them invalid right away, without even typing.

All these changes are moving the final product further away from the original mockup, which at this stage doesn't work anymore.

I'm going back to drawing board trying to find a solution without any tooltips, floating elements, or any other shenanigan that can cause accessibility issues.

Here's a slightly more positive update.
It seems like FF is planning to implement checkValidity and reportValidity for ElementInterals.
https://bugzilla.mozilla.org/show_bug.cgi?id=1556367
https://bugzilla.mozilla.org/show_bug.cgi?id=1556370

Which means that in a not so far future we will be able to do form validation like the gentlemen we are.

For now, I think I found an acceptable solution.
Using the tooltiptext attribute makes the info and warning images accessible, and the built-in tooltip appears on mouse over (as you can see from the screenshot).

This solution allows us to not change anything in the account UI and keep the new paradigm.
I can then file a follow up bug to implement built-in form error messages once those 2 patches from FF have landed.

What do you think?

Here's the patch with the Account Hub sections stripped away, and the accessible tooltiptext implementation.

I left in the accountHub.css files as that style is entirely used by the email wizard, and will be shared across all the different account creation dialogs.

Removed a leftover icon and rolled back a couple of IDs that didn't need to be updated.

In response to comment 76:

I'm more and more convinced this implementation won't happen any time soon.

Sorry, but I have to protect Alessandro and the project goal here:

Guys, this whole project is completely de-railing due to excessive reviewer demands.

The purpose of this project is not to rewrite the dialog with modern or current coding styles. In fact, it was an explicit non-goal. The only purpose here is to adapt the style. Nothing else. The XUL should be adapted only minimally, and the JS not touched at all, as far as possible. That was my condition from the onset for this project.

By adding HTML re-write and other aspects into this same patch,
a) the patch becomes larger and much harder to read, not only for the reviewer, but also in the code history
b) the risk of regressions becomes much higher, and debugging such a large patch will be much harder
c) It drags out, risking whether it can make it for TB 68
d) it becomes flat out impossible to do, as Alessandro now concluded as well.

The problems we have are a predictable result of a failed policy. Please see https://en.wikipedia.org/wiki/Scope_creep and https://en.wikipedia.org/wiki/Atomic_commit#Atomic_commit_convention . Such rules are there for a reason. They come from decades of engineering experience of failed projects. Ignoring them will lead to ... surprise ... failed projects.

That's why I opposed additional changes. The patch as it is works fine and is a massive, very visible improvement over status quo. It should ship in TB 68. It must not have further changes, for the above reasons.

Please land the patch as-is. Do not modify it further with more HTML rewrite. That would be a separate project. That project has its own difficulties. One problem at a time, please!

I think you will find that it's possible to do your intended UI with HTML. It's just another task. It has nothing to do with this project.

In response to comment 77:

For now, I think I found an acceptable solution.
Using the tooltiptext attribute makes the info and warning images accessible, and the built-in tooltip appears on mouse over (as you can see from the screenshot).
This solution allows us to not change anything in the account UI and keep the new paradigm.
I can then file a follow up bug to implement built-in form error messages once those 2 patches from FF have landed.
What do you think?

Yes, I concur. If the patch works well as-is, let's land it and ship it in TB 68.

Then, do a followup task to do any code rewrite using HTML or whatever, and do that for TB 70+. That project would be concentrated on the HTML rewrite only. That allows to better manage the work, project and regression bug risks.

I think that's a wise suggestion, yes.

(In reply to Magnus Melin [:mkmelin] from comment #83)

Looks good to me, just some formatting nits and such. r=mkmelin

Yay!

move to first. shouldn't it be type="password"?

Yes, it should. That's a leftover from the original dialog where for some reason it was done to prevent the placeholder to show as ***.

the accounthub part should be removed

The accountHub.css files are used to style the form and provide shared styles.
Since these same styles will be used for all the other account creation wizards (feeds, calendar, etc) I'd like to keep these files.
it's better than defining everything inside the accountCreation.css file and then duplicate them for all the other wizards, or include the accountCreation.css in all the other files.

If you don't like this approach I can change it, and then we will deal with other wizards styles when the time comes.

(In reply to Alessandro Castellani (:aleca) from comment #77

What do you think?

In that case, maybe name it something more neutral, and use it from both dialogs. Well, just the one to start with.

I can't detect a try run here. How do we know that the various account creation tests still pass?

Mhhh, I remember I did a try run for this but a while ago. Maybe I'm wrong and I just run those tests locally.
I'll push a try run now, sorry about that.

mozmake SOLO_TEST=newmailaccount/test-newmailaccount.js mozmill-one
fails.

BTW, your try run (which would be good to paste into a comment for recording purposes) at
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=09b8f74740e6e85f2d619a89c96d419c99332af6
is busted due to new duplicates you have created:

[task 2019-07-24T19:20:46.077Z] 19:20:46 INFO - package> WARNING: Found 43 duplicated files taking 221033 bytes (uncompressed)
[task 2019-07-24T19:20:46.077Z] 19:20:46 INFO - package> ERROR: The following duplicated files are not allowed:
[task 2019-07-24T19:20:46.078Z] 19:20:46 INFO - package> Thunderbird Daily.app/Contents/Resources/chrome/toolkit/skin/classic/mozapps/profile/information.svg
[task 2019-07-24T19:20:46.078Z] 19:20:46 INFO - package> Thunderbird Daily.app/Contents/Resources/chrome/classic/skin/classic/messenger/icons/information.svg

In summary: Landing this patch would have been quite fatal :-(

Thanks for the help and all the important recommendations.
I'll take care of this right away.

Here's the try push, which it looks good if those Z2 are expected: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=bb0df65eb9e22f142d26ca13928a58bb387667db

I'm asking a further quick review to have the OK on the name of the files, and the fact that I had to put back the "Get a new email address..." button, styled as a link, to fix some test failures.

Hmm, those Z2 failures have been seen without your patch here:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=011167f1ac2a0d861f9514df9ef437347f0815f7

I did that try run since I had nothing to push to cover the M-C merge of
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5585edba8fdb83267722e62241952272c8e24a8f&tochange=6598e37c88d2816deed4fdaedbddf9c9dade7987

As you can see, I reran some tests, and the Z2's turned green. Also this push
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=e2dad5423b3ba2aa4742d4f0e74e0a06ebe92291
only has one of them.

Sorry, too many words, conclusion is that the status of those Z2 failures is not known, but they're not related to your patch.

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/047998237781
Implement new UI for account creation dialog. r=mkmelin

Why did you add information.svg if it wasn't referenced anywhere?

Honestly, I don't know.
After pulling the latest changes I had a merging issue while applying this patch and that's where I got the inability to add information.svg to the jar file.
That was a stupid distraction mistake.