440377 - (tb-pills) Redesign recipients address fields (To, Cc, Bcc) as single-line input fields for multiple comma separated addresses - not one line/row per address

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, enhancement, P2)

Description

[Olaf Fraczyk]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.14) Gecko/20080404 Firefox/2.0.0.14
Build Identifier: version 2.0.0.14 (20080421)

Thunderbird expects that every address goes in separate (To, CC etc.) field.
It is OK for 1-3 addresses, but with more it is a real pain.
Almost every other software supports putting addresses in comma separated list.

Today it is possible to write such a list in Thunderbird but it is far from perfect:
1. If you hit "Enter" then the list is split in numerous "To" fields
2. As I reported in bug #440370 - LDAP autocompletion works for first address only
3. As I reported in bug #440376 - If you drag addresses from "Contacts" pane and you drop them in the same "To" field then no comma is added and mail is sent to the last recipient only - the rest is treated as recipients name.



Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Actual Results:  
Pain :)

Expected Results:  
To have it working like in Outlook or in Evolution or ... ;)

Comment 1

[Wayne Mery (:wsmwk)]

Olaf, I'm not defending the current implementation, but what problem do you encounter regarding one line/address?  Or are we talking strictly aesthetics?

Comment 2

[Olaf Fraczyk]

1. Habits - if people use other e-mail clients they expect that things work in similar way - please don't classify this simply as aesthetics.
For now I see the disadvantages below (the most severe for me are 2 and 5):

2. If you have more than 4 recipients you need to scroll - this is what makes me personally unhappy, in outlook you can enter more than 15 addresses (depending on address length:) per one recipient type (To, Cc, Bcc) before you need to scroll (the widget expands up to 4 rows before adding scrollbar).

Even if it didn't expand (as in Evolution (at least in 2.8) and in Thunderbird) you have plenty of space - it will fit at least 5-6 e-mail addresses. 

It is __much__ easier if you can see all the recipients at a glance. You won't miss any person to send, and what is more important - you won't send to a person that shouldn't get the mail :)

3. If you put several entries in "To" and then you change your mind and want to put some of them in "Cc" or "Bcc" it is easier to copy and paste than to change every "To,CC,Bcc" combo.

4. If you want to remove some addresses you need to select every one separately and delete. You can't select 5 addresses and delete them all. BTW. there is no way to delete unused fields, it is possible to delete only addresses.

5. If you send several messages to the same group of people (or a large portion of addresses is the same - eg. all addresses in CC). With "multi address" fields (like in Evo, Outlook) you can just select and do a copy/paste. In Thunderbird it is impossible.

To make this even harder, in Thunderbird the text in message header pane cannot be selected, so you can't select addresses from a message and put in address fields. I'll fill separate bug for it anyway.
Creating a list is not an option because they are often ad hoc lists - used for 5-6 messages. Doing Ctrl-C and Ctrl-V is much easier and quicker.

This is what hit me in 2 days.

Comment 3

[Wayne Mery (:wsmwk)]

>Almost every other software supports putting addresses in comma separated list.

Jeff, Does eudora support this?


(In reply to comment #2)
 For now I see the disadvantages below (the most severe for me are 2 and 5):
> 
> 2. If you have more than 4 recipients you need to scroll ...
> It is __much__ easier if you can see all the recipients at a glance. You won't
> miss any person to send, and what is more important - you won't send to a
> person that shouldn't get the mail :)

agree

> 3. If you put several entries in "To" and then you change your mind and want to
> put some of them in "Cc" or "Bcc" it is easier to copy and paste than to change
> every "To,CC,Bcc" combo.

yup
 
> 4. If you want to remove some addresses you need to select every one separately
> and delete. You can't select 5 addresses and delete them all. BTW. there is no
> way to delete unused fields, it is possible to delete only addresses.

sort of bug 252665.  I thought there was another bug but I'm not finding it.
 
> 5. If you send several messages to the same group of people (or a large portion
> of addresses is the same - eg. all addresses in CC). With "multi address"
> fields (like in Evo, Outlook) you can just select and do a copy/paste. In
> Thunderbird it is impossible.

Or save as template. Or view source + copy/paste.  Or edit as new.  Lots of workarounds

moding summary to better fit description

Comment 4

[Jeff Beckley]

(In reply to comment #3)
> >Almost every other software supports putting addresses in comma separated list.
> 
> Jeff, Does eudora support this?

Yes, it does.  This is very standard email practice since in transit the addresses are comma-separated lists.  People are used to seeing them this way when viewing a message, so it becomes natural to allow this in composition as well.

Comment 5

[Olaf Fraczyk]

(In reply to comment #3)
> Or save as template. Or view source + copy/paste.  Or edit as new.  Lots of
> workarounds
1. I saved as template. But where is it saved? How do I create new message from template? - OK - another bug - I assume that Templates should be a folder in "Local Folders". Unfortunately I don't have Templates there. And attempt to creating one tells me that such folder already exists :)
2. View source - the fields are quoted-printable or base64 encoded - no way
3. Edit as new - thanks - this works.

Comment 6

[Magnus Melin [:mkmelin]]

xref bug 392932

Comment 8

[Wayne Mery (:wsmwk)]

I could swear this is a dupe but I'm not finding it, so my recollection may be bad or perhaps I misinterpreted a related bug.

to add to Magnus's xref  Bug 392932 -  autocomplete multiple addresses on the same address line
 Bug 416711 -  no possibility to change delivery mode (to/cc/bcc) for multiple addresses at once 
 Bug 440410 -  When using commas to separate multiple addresses, there is no check if all of these are valid. 
 Bug 457872 -  Splitting of multiple mail addresses in the TO field is only done if Enter is pressed
 Bug 351863 -  thunderbird -remote mailto doesn't allow multiple addresses

Comment 12

[Karsten Düsterloh]

On a drive-by note: commas may appear in email addresses, so using comma as a separator isn't wise...

Comment 13

[Olaf Fraczyk]

Please read the RFC. They have to be quoted. Just like semicolons, etc. If you don't agree that they have to be quoted when typed into input field, then you are out of luck as any ASCII character is permitted.
The comma is explicite shown in RFC as address separator and all other e-mail clients I have ever seen, support using commas as address separator. Please keep this consistent and don't reinvent the wheel.

Comment 15

[mad.engineer]

I agree with Olaf. The current Message compose design of To:, Cc: fields is absolutely poor design IMHO. Every other client that I've used in the past allow multiple addresses via comma separate fields. If majority of the clients work the way it should be then they must be doing it right. That's why I dumped TB in favor of my previous client for now till this gets fixed.

Comment 16

[Bluelightav.org]

The space usage for the address field is not efficient the way TB3 uses it. I am getting some user feedback in office environments about the way TB
displays the recipients in the compose window. TB filling in each line only one
address leads to a long list vertically which is out of the viewable area and
needs quite some scrolling to manually check if one has entered all recipients.
That applies also when one wants to have a quick check if all people are
entered via a mailing list. The customer prefer the horizontal display of other
email clients more where email addresses are displayed in a row separated by a
comma. That way mailing list can be checked and easily temporarily modified not
touching the original mailing list.

Better space usage on small screens and better overview is what I wish to see :-)

Comment 17

[Vliegendehuiskat]

I agree on how Thunderbird 3 is pretty non-efficient with screen space.

It tries to be efficient because when you enter a lot of addresses you'll get to scroll or to drag the field down, which makes more lines appear.

However, when you enter only one address it still shows 4 address lines.

It would be nice if the default number of address lines becomes 1, so that you can also hide the unused address lines.

Comment 18

[Vliegendehuiskat]

I've discoverd a plugin called mailtweak that does exactly this.

http://mailtweak.mozdev.org/

It would be nice if some of its functionality would be built into Thunderbird.

Comment 19

[chrizilla]

#17 and #18 is a separate issue and should be filed as a separate request (in case the idea has not been brought up yet).
PS: I agree that it should be built into TB.

Comment 20

[Jason Daly]

I'm really quite surprised that this issue has never been addressed.  In my opinion this is Thunderbird's major weakness with regards to targeting a corporate user-base.  Corporate users will not adopt Thunderbird until users can:

- Enter all addresses horizontally on one line
- Click one of To:, Cc:, or Bcc: to open the default address book.

Comment 22

[Geert Poels]

The recipients UI works ok for a couple of recipients but basically it is flawed from an intuitive point of view.
Intuitively, you want to place each email address in a category (to, cc, bcc) and quickly see how is to be placed where. To also move or remove them when wrong.
Thunderbird UI approach fails on each of these.
Moving means cut/paste, even clicking it workes bizarre (I'd expect a single click to fully select, a second click to edit) and looking who's is which status is to be completely forgotten.
For those who'd really keep the current setup, keep the current look-and-feel as an option.

Comment 23

[Frank Breitling]

Terrible bug!

Comment 25

[Thomas D. (:thomas8)]

I wonder how hard this would actually be for someone with better coding skills & experience. We're already able to parse comma-delimited addresses. The UI needs radical change, but it will actually be simpler after that. I'd imagine that if we can autocomplete for single addresses, there must be a way to offer just what's being typed between two commas to autocomplete?

Comment 26

[KeithP]

Wow, can't believe this is still an issue. Thunderbird is on it's own with this one. This is the main reason I don't use it. I download every update to check to see if it's been implemented yet and every time I'm let down. Would love to use Thunderbird for my work email but can't due to this one very big annoying issue.

Come on, get it together guys. I see Postbox can do it, which is built on Thunderbird, so why can't you?

Comment 27

[Thomas D. (:thomas8)]

(In reply to KeithP from comment #26)
> Wow, can't believe this is still an issue. Thunderbird is on it's own with
> this one. This is the main reason I don't use it. I download every update to
> check to see if it's been implemented yet and every time I'm let down. Would
> love to use Thunderbird for my work email but can't due to this one very big
> annoying issue.

yes, it's weird (sigh...)

> Come on, get it together guys. I see Postbox can do it, which is built on
> Thunderbird, so why can't you?

Not enough manpower (now practically maintained by volunteers only), complex codebase, and too many bugs and RFEs to choose from that are equally or more annoying...

If you can come yrself or bring somebody with some coding skills in JavaScript/HTML to fix this, most welcome... :)

fwiw, are you aware that you can type or dump comma-separated addresses into one of TB's single fields, and TB will automatically resolve them into single addresses of same type?

Comment 28

[M. Grau]

@KeithP: Coming from Outlook, I had the same problem until I found this add-on: https://addons.mozilla.org/de/thunderbird/addon/mrc-compose (separate to/CC/BCC fields with comma-separated addresses and address book suggestions while typing). I have not had a long test phase with this add-on yet, but it seems to work okay so far. Should be Thunderbird default though, IMHO.

Comment 29

[Thomas D. (:thomas8)]

(In reply to M. Grau from comment #28)
> @KeithP: Coming from Outlook, I had the same problem until I found this
> add-on: https://addons.mozilla.org/de/thunderbird/addon/mrc-compose
> (separate to/CC/BCC fields with comma-separated addresses and address book
> suggestions while typing). I have not had a long test phase with this add-on
> yet, but it seems to work okay so far. Should be Thunderbird default though,
> IMHO.

Thanks M. Grau, that's helpful information. In fact, looking at the addon source code might be a good starting point for fixing this.

Comment 30

[KeithP]

@M. Grau: Great. Thanks for letting me know about the add on. It seems to do the trick. Still would like to see it as Thunderbird default though.

I have no coding skills and don't know anyone with any but hopefully someone can use the add on source code to get this implemented ASAP.

Thanks for your quick replies guys.

Comment 31

[michelr]

Hi,

I forgot to inform you that I created an addon that seems to fulfill your request :
https://addons.mozilla.org/fr/thunderbird/addon/mrc-compose/

Thanks to Andreas Wagner who implemented the LDAP support.
Since TB24, we have some small regressions, most annoying is the default focus that doesn't follow rules.
Hope it helps.
Michel

Comment 32

[Josiah Bruner [:jsbruner]]

FYI, I'm starting work on this now.

Comment 33

[Thomas D. (:thomas8)]

(In reply to Josiah Bruner [:JosiahOne] from comment #32)
> FYI, I'm starting work on this now.

Josiah, that's great news, and I think the majority of users will be truly greatful to you for normalizing and simplifying the composition header. So thanks a lot!

Btw, this has huge potential to build upon and push things somewhat further and make address handling more efficient wrt selecting, deleting, copying, changing recipient type etc. of individual addresses in composition header.

Comment 34

[David T. Macknet]

Thank you for working on this.

Two other usability issues with the scrolling: it's not always presented (no idea why - it's sometimes there, sometimes not, and I can't force it to appear when I want to add more addresses); and I've hit "reply-all" and subsequently changed my mind, after composing the reply, and found that it's much easier to cut / paste my reply than it is to go through deleting addresses.

Comment 35

[Dave Aton]

(In reply to Josiah Bruner [:JosiahOne] from comment #32)
> FYI, I'm starting work on this now.

Whew! Six years of begging and pleading, someone's finally taking it on. Thank you, Josiah, and DietySpeed!

Comment 36

[David E. Ross]

(In reply to Olaf Fraczyk from comment #2)
> 3. If you put several entries in "To" and then you change your mind and want
> to put some of them in "Cc" or "Bcc" it is easier to copy and paste than to
> change every "To,CC,Bcc" combo.

I strongly disagree.  It is easier for me to change "To:" to "CC:" than to cut-and paste.  

 
> 4. If you want to remove some addresses you need to select every one
> separately and delete. You can't select 5 addresses and delete them all.
> BTW. there is no way to delete unused fields, it is possible to delete only
> addresses.

This could instead be accomplished if MailNews allowed users to select multiple lines within the address area of the Compose window.  


> 5. If you send several messages to the same group of people (or a large
> portion of addresses is the same - eg. all addresses in CC). With "multi
> address" fields (like in Evo, Outlook) you can just select and do a
> copy/paste. In Thunderbird it is impossible.

Why not use a mailing list.  

I happen to like the current capability and thus request any implementation of this RFE be optional with a UI to enable or disable the option.  In that case, I would not be upset if this RFE were the default option.  

Also, if bug #119977 is not implemented prior to or concurrently with this bug, please do not implement in a way that breaks the PopMailListRecipients extension.  And if bug #933472 is not implemented prior to or concurrently with this bug, please do not implement in a way that breaks the "Reply to All as Cc" extension.

Comment 37

[:aceman]

OK, are there any mockups on how do you propose to solve this? Will there be a line for each of the recipient types (like in Outlook), of which there are about 6 now?

Comment 38

[Josiah Bruner [:jsbruner]]

Not yet, mockups will be available at: http://apps.josiahbruner.com/SingleLine.html as I work on them.

(It has very basic design currently), and I update it live, so feel free to follow the design.

Comment 39

[Richard Leger]

For reference as per my related comment in another bug reported:

https://bugzilla.mozilla.org/show_bug.cgi?id=1042780#c48
"...
- Replace the current To/Bcc/Cc separate fields for each email address set by a comma (or semi-colon) list of addresses instead (as usually used in alternative email client software). With a trailing X button for each address to allow deletion/removal or a drop down menu to: delete, copy email address, edit contact, add contact, etc...
..."

Comment 40

[Richard Leger]

In addition of proposed mockup for improvement in the Compose Window layout (see attachment 8507216 [details] and bug 1042780 comment 48) as possible suggestion, if dedicated fields are created for To,Cc,Bcc (with comma or semi-colon list of recipients), the Cc and Bcc fields could be hidden by default (with default setting manageable via end-user in options possibly) with two buttons appearing below the To field to make them appear if required, something like:

   From: [           ]
     To: [           ]
         [           ]
         (Cc) (Bcc)
   [                 ] 
   [ Body text       ]
   [                 ]

If you click on (Cc) button, field would deploy (same for Bcc) and looks something like:

   From: [           ]
     To: [           ]
         [           ]
     Cc: [           ]
         [           ]
         (Bcc)
   [                 ] 
   [ Body text       ]
   [                 ]

Re-clicking on Cc: (or Bcc:) would hide it again (and disable it/clear field content?)... with application remembering the last settings... to keep it the same for all following compose windows...

Easy and intuitive to use... but that may not be easy to program and implement in Thunderbird, would option be validated and accepted... :)

Comment 41

[Richard Leger]

(In reply to David E. Ross from comment #36)
> (In reply to Olaf Fraczyk from comment #2)
> > 3. If you put several entries in "To" and then you change your mind and want
> > to put some of them in "Cc" or "Bcc" it is easier to copy and paste than to
> > change every "To,CC,Bcc" combo.
> 
> I strongly disagree.  It is easier for me to change "To:" to "CC:" than to
> cut-and paste.  

If entered typed addresses and/or entries were to become some sort of contact objects (after text parsing) in the (To,Cc,Bcc) fields on which action can be applied via a trailing drop down menu for example (as suggested in comment 39) it may be possible as one can imagine to add the following actions:
- move to Bcc
- move to Cc

and/or also allow drag and drop(s) of one or more contacts (multiple contact move would require ability to select one or more contacts for actions: delete, add to contacts, etc... (among possible others...)

Comment 42

[:aceman]

I still see no mentioning of the "Reply-To", "Newsgroup" and "Followup-To" recipient types.

Comment 43

[Josiah Bruner [:jsbruner]]

Those will be included too, but will only be visible if relevant. Reply-To may be part of the To field somehow, but I'm not sure yet.

Comment 45

[Thomas D. (:thomas8)]

Josiah, are you still working on this?

This looks like a very important feature to improve/normalize the UX of TB composition in terms of ux-consistency, ux-efficiency, etc...

We should try to land this for the next big release after TB 38.

Comment 46

[Josiah Bruner [:jsbruner]]

(In reply to Thomas D. from comment #45)
> Josiah, are you still working on this?
> 
> This looks like a very important feature to improve/normalize the UX of TB
> composition in terms of ux-consistency, ux-efficiency, etc...
> 
> We should try to land this for the next big release after TB 38.

I am extremely busy between work and school now, and my main priority these days is reviews, so I find it unlikely I will get to this any time soon.

I would like it in 38 as well, but I don't know if that's possible at this point. Someone else can take this in the meantime if they so desire. Otherwise, it'll have to wait until I have some free time. I will continue working on it whenever I get a chance.

Comment 47

[:aceman]

Thomas correctly meant NEXT release after TB 38, so TB 45. I think nobody expects to get this into 38 as there would be very short testing. And this is a critical part of TB.

Comment 48

[Josiah Bruner [:jsbruner]]

(In reply to :aceman from comment #47)
> Thomas correctly meant NEXT release after TB 38, so TB 45. I think nobody
> expects to get this into 38 as there would be very short testing. And this
> is a critical part of TB.

Ah, missed that. Regardless, everything I said still stands.

Comment 49

[David E. Ross]

Will the implementation include a user-option option to keep the current design?  Will the implementation break the PopMailListRecipients or "Reply to All as Cc" extensions?

Comment 51

[michelr]

Hi,
FYI, I already wrote an addon that does what you requested :
https://addons.mozilla.org/fr/thunderbird/addon/mrc-compose/

With the upcoming 1.8 release, there will be full LDAP support, so it'll be able to replace existing UI.
I expect the 1.8 release to be announced in few days/weeks.
Until that, you can see code :
https://github.com/michelRenon/mrc_compose

I would be happy if that addon can be integrated (fully or partly) in Thunderbird.
(Jonathan Protzenko already contacted me nearly two years ago about integrating it in TB : I just had not enough time to dig into global TB build process)

Comment 53

[Hardi]

(In reply to michelr from comment #51)
> FYI, I already wrote an addon that does what you requested :
> https://addons.mozilla.org/fr/thunderbird/addon/mrc-compose/

Great addon thank you for this michelr!

Currently drag & drop copies (instead of moving) the selected address.
if the drag & drop can work like Outlook or Gmail it will be perfect.

Comment 54

[Ludovic Hirlimann [:Usul]]

Removing myslef on all the bugs I'm cced on. Please NI me if you need something on MailNews Core bugs from me.

Comment 55

[Dan]

This extension is UNSIGNED and therefore Thunderbird warns me NOT to install it.  How do I know the compiled code is not changed from github source and maybe it reads and phones home my email contents?

Comment 56

[Kent James (:rkent)]

Thunderbird should not be warning you to not install unsigned extensions, because we decided not to require signed extensions. If you are getting an error such as you report "Thunderbird warns me NOT to install it" could we have more details?

In any case, extension signing has caused lots of issues in Firefox, so we are unlikely to support it any time soon (if ever). Also, it does not really prove "the compiled code is not changed from github source" either.

Comment 57

[Dan]

(In reply to Kent James (:rkent) from comment #56)
> Thunderbird should not be warning you to not install unsigned extensions,
> because we decided not to require signed extensions. If you are getting an
> error such as you report "Thunderbird warns me NOT to install it" could we
> have more details?
> 
> In any case, extension signing has caused lots of issues in Firefox, so we
> are unlikely to support it any time soon (if ever). Also, it does not really
> prove "the compiled code is not changed from github source" either.

OK, I installed it.

Comment 58

[mad.engineer]

(In reply to Kent James (:rkent) from comment #56)
> Thunderbird should not be warning you to not install unsigned extensions,
> because we decided not to require signed extensions. If you are getting an
> error such as you report "Thunderbird warns me NOT to install it" could we
> have more details?
> 
> In any case, extension signing has caused lots of issues in Firefox, so we
> are unlikely to support it any time soon (if ever). Also, it does not really
> prove "the compiled code is not changed from github source" either.

Long shot, but I'll ask anyway. Any chance that this MRC Compose add-on can be included in the Core TB itself? Thanks

Comment 59

[Kent James (:rkent)]

(In reply to mad.engineer from comment #58)

> Long shot, but I'll ask anyway. Any chance that this MRC Compose add-on can
> be included in the Core TB itself? Thanks

I looked at the extension code, it is quite intrusive, so it is not as simple as just incorporating it. But I don't see anyone here objecting to the concept, so the addon might be used by someone as a basis to prepare a core patch. Patches welcome, in other words.

Comment 60

[michelr]

(In reply to Dan from comment #55)
> This extension is UNSIGNED and therefore Thunderbird warns me NOT to install
> it.  How do I know the compiled code is not changed from github source and
> maybe it reads and phones home my email contents?

Hi Dan,
Just FYI, that extension has no compiled code, so you can check yourself what it does.
From the .xpi downloaded :
- change the '.xpi' to '.zip'
- extract it
- then you can see all .xml and .js files and see that it does not steal data.
It also allows you to check that what you downloaded is exactly what is in github.

Cheers

Comment 61

[michelr]

(In reply to Kent James (:rkent) from comment #59)
> (In reply to mad.engineer from comment #58)
> 
> > Long shot, but I'll ask anyway. Any chance that this MRC Compose add-on can
> > be included in the Core TB itself? Thanks
> 
> I looked at the extension code, it is quite intrusive, so it is not as
> simple as just incorporating it. But I don't see anyone here objecting to
> the concept, so the addon might be used by someone as a basis to prepare a
> core patch. Patches welcome, in other words.

Hi, I'm the developer of MRC Compose.
I'm quite honored that you think to include it in TB.
But I have to warn you that it would break compatibility with some other addons, those that work on addressing widgets. Here are some addons that incompatible : flexible-identity, account colors, BorderColors GT

The last request is to add some 'objects' or 'tokens' in the fields. But it would require a great rewrite, and trying to embed jQuery+TokenInput. So not easy at all...

But currently, I have no time anymore to work on it. The best I can do is to provide some help to people wanting to work on it.
Cheers, Michel

Comment 62

[David E. Ross]

(In reply to Kent James (:rkent) from comment #59)
> I looked at the extension code, it is quite intrusive, so it is not as
> simple as just incorporating it. But I don't see anyone here objecting to
> the concept, so the addon might be used by someone as a basis to prepare a
> core patch. Patches welcome, in other words.  

I object!  See my comment #36 and comment #49.  I prefer the current line-per-addressee design.  As I indicated before, if this RFE is implemented, it should be an option and not a replacement of the current design.

Comment 63

[Jim Porter (:squib)]

(In reply to David E. Ross from comment #62)
> I object!  See my comment #36 and comment #49.  I prefer the current
> line-per-addressee design.  As I indicated before, if this RFE is
> implemented, it should be an option and not a replacement of the current
> design.

I doubt this will be an option, since TB needs fewer options, not more. But that doesn't preclude someone (even the TB team themselves!) from producing an add-on to support this. However, I think we can address your concerns even with the new UI:

(In reply to David E. Ross from comment #36)
> I strongly disagree.  It is easier for me to change "To:" to "CC:" than to
> cut-and paste.  

A good implementation of this would be just as easy to change To->CC as before. The design I have in mind is that each address is an atomic unit (basically...*). Just drag and drop the address in question.

> > 5. If you send several messages to the same group of people (or a large
> > portion of addresses is the same - eg. all addresses in CC). With "multi
> > address" fields (like in Evo, Outlook) you can just select and do a
> > copy/paste. In Thunderbird it is impossible.
> 
> Why not use a mailing list.  

There are times where you don't do this enough to warrant making a mailing list.

* I thought I had a comment in this bug that described my plan, but it seems I do not.

Comment 64

[Jim Porter (:squib)]

The design I have in mind for this is that each address is "bubbled" like in the message reader, so you can operate on it as though it's a single element, rather than a string of characters. This would make it easy to move addresses around (individually or in bunches), which should help to maintain the benefit of the current UI while also adding new capabilities.

I could probably take a look at bug 626532 sometime soon, which would make implementing such a UX a lot easier.

Comment 65

[Dan]

I especially like how this extension shows email list recipients after adding the email list.

Comment 66

[M Lopez-Ibanez]

A very annoying issue with the current UI is that when replying to multiple recipients, the UI does not show all the recipients (one needs to scroll). Moreover, if you remove one recipient, the list does not scroll up, it just leaves an empty space. The result is that I often end up not trimming enough the recipient list. With long lists, the process of removing recipients becomes very slow (there should be a one-click check-mark like in Gmail!).

Do I need to open a new PR or is this a duplicate of this one?

Comment 67

[M Lopez-Ibanez]

(In reply to Jim Porter (:squib) from comment #64)
> The design I have in mind for this is that each address is "bubbled" like in
> the message reader, so you can operate on it as though it's a single
> element, rather than a string of characters. This would make it easy to move
> addresses around (individually or in bunches), which should help to maintain
> the benefit of the current UI while also adding new capabilities.

The UI of Thunderbird Conversations for quick replies looks like this, but it does not allow drag&drop or editing the bubbles.
https://addons.cdn.mozilla.net/user-media/previews/thumbs/51/51379.png?modified=1331218902
https://addons.mozilla.org/en-US/thunderbird/addon/gmail-conversation-view/

Comment 68

[M Lopez-Ibanez]

(In reply to M Lopez-Ibanez from comment #66)
> A very annoying issue with the current UI is that when replying to multiple
> recipients, the UI does not show all the recipients (one needs to scroll).
> Moreover, if you remove one recipient, the list does not scroll up, it just
> leaves an empty space. The result is that I often end up not trimming enough
> the recipient list. 

I just installed: https://addons.mozilla.org/fr/thunderbird/addon/mrc-compose/ and it solves this problem wonderfully.

Comment 69

[Marco A.G.Pinto [:marcoagpinto]]

(In reply to M Lopez-Ibanez from comment #68)
> (In reply to M Lopez-Ibanez from comment #66)
> > A very annoying issue with the current UI is that when replying to multiple
> > recipients, the UI does not show all the recipients (one needs to scroll).
> > Moreover, if you remove one recipient, the list does not scroll up, it just
> > leaves an empty space. The result is that I often end up not trimming enough
> > the recipient list. 
> 
> I just installed:
> https://addons.mozilla.org/fr/thunderbird/addon/mrc-compose/ and it solves
> this problem wonderfully.

I installed it too... but it doesn't work well with Enigmail.

For several months that I have been waiting for a fix without success.

With the fix, it simply can't be used together.

Comment 70

[Marco A.G.Pinto [:marcoagpinto]]

With=without*

sorry for the typo.

Comment 71

[Annie Wellington]

Please provide the promised functionality of multiple addresses in one To: field
The current idea of separated fields is a pain and very strange

Comment 72

[Cigar Guy]

This is perhaps an implementation feature, but is really a pain. I would like to see the existing functionality also enhanced by allowing multiple emails in TO , CC , BCC fields 
Example
[e-mail], [e-mail],[e-mail],[e-mail],[e-mail]..... So on, so forth.... Almost all email software offers this feature of delimitting email fields by a COMMA and SemiColon. Why is this composition window any different and making UX difficult and frustrating

Comment 74

[M Lopez-Ibanez]

(In reply to M Lopez-Ibanez from comment #68)
> I just installed:
> https://addons.mozilla.org/fr/thunderbird/addon/mrc-compose/ and it solves
> this problem wonderfully.

Thunderbird 60 broke this feature of MRC Compose. See bug 1502391.

Comment 76

[M Lopez-Ibanez]

The github of MRC Compose is here: https://github.com/michelRenon/mrc_compose

It seems TB 60 broke two things in MRC Compose:
* The TO field cannot expand to more than one line anymore. The CC and BCC fields do expand.
* When a tooltip/warning is shown, the TO field becomes invisible. It works for CC and BCC fields.

The problem seems to be some recent change to the TO field in thunderbird.

Comment 77

[M Lopez-Ibanez]

Attachment: Screenshot_20181027_223333.png

Screenshot showing the problem. The CC and BCC fields work as described in this bug. The TO field doesn't. It was working before TB 60.

Comment 78

[Bob]

Second the observation from Lopez.
Actually, MRC compose should be mainlined into Thunderbird, at least an option. That's how the interface should have been. That's how most other email editors do it. It is easier to bulk edit emails (deleting, moving them between to:/cc:, or merging different branches of the same thread). 
Thunderbird should work well for the power users as well as those who only need to enter 1 or 2 email addresses.
Best!

Comment 79

[Dan]

Second the observation from Bob. Is MRC development dead?

Comment 80

[Richard Leger]

I agree with Bob and support his comment, as a daily user of Thunderbird I also think MRC shall be make mainline into Thunderbird especially for power users... but it would need to be well integrated and intuitive for end-users... I am happy to test and provide feedback via beta channel would it be implemented...

Taking into account suggestions from this bug report to maximise flexibility such as Comment 39 or Comment 40... would be nice as well... to improve the potential feature...

Copy/Past shall also be made possible... (of one or more contacts from within TB or outside sources) as well as drag and drop between TO,CC,BCC... as user see fit best... ideally...

Thanks for listening :-)

Comment 81

[M Lopez-Ibanez]

Although it is true the MRC Compose is a massive improvement over the current compose window of Thunderbird, modern email applications are much more advanced.

For an example, you just have to compose an email in GMail/Outlook or see this unmaintained add-on (developed by Mozilla staff but never merged into Thunderbird):  https://addons.thunderbird.net/en-US/thunderbird/addon/compose-for-thunderbird/

Or see the quick-reply compose window of Conversations (sadly not available for composing new emails): https://addons.thunderbird.net/user-media/previews/full/51/51379.png?modified=1530208682
https://addons.thunderbird.net/en-GB/thunderbird/addon/gmail-conversation-view/

(In reply to Dan from comment #79)
> Second the observation from Bob. Is MRC development dead?

https://github.com/michelRenon/mrc_compose/issues/35#issuecomment-385939943

Comment 82

[michelr]

Hi, I'm the author of MRC Compose.
First, thanks to all users of my addon.

The development is not dead, I don't have a lot of time to work on it.
But I'll look at the "TO" bug asap.
And there are some improvements that are planned (most important is CardBook compatibility).


About the inclusion in TB, it has already been discussed (comment #61)
Since that time (dec 2015 !), some things have changed  :
it would be possible to create new API to have MRC Compose and other addons (incompatible or not) work together :
https://mail.mozilla.org/pipermail/tb-planning/2018-December/006324.html
I think it is a huge positive step to allow inclusion in TB.
I you have any suggestion, please use the support email from addon page (https://addons.thunderbird.net/en-GB/thunderbird/addon/mrc-compose/)

Cheers,
Michel

Comment 83

[Miguel]

Following with interest. I was a user of MRC, but had to drop it as it is currently incompatible with Cardbook (which provides two way integration with could address books, like google's).

MRC with the fix to allow these address books (https://github.com/michelRenon/mrc_compose/issues/35 ) would be a massive improvement to Thunderbird.

Comment 85

[Alessandro Castellani [:aleca]]

This bug is long overdue.
Here's a mockup I did for the redesign of the address area in the compose dialog.
attachment 9052074 [details]

Comment 86

[Magnus Melin [:mkmelin]]

(adding back the old summary for findability)

Comment 88

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

All right, here's a first and super broken patch as a proof of concept for this new implementation.
I'm asking early feedback before going to deep.

This is just a test patch with some early work, almost everything is broken and not functional at all, so, be careful. These are the things I did and looking for feedback.

• Only 1 autocomplete input, no duplication of rows (the CC, BCC, etc, will have their own fields).
• On enter or autocomplete select, the value gets converted into a custom element.
• The new CE is a blatant copy of the email address field in the Message Header. For now is super broken, but the objective is to have the same features of the Message Header, like adding an address to the Address Book, the same context menu, etc.
• Invalid emails or not matching the address book will be highlighted in red.
• The field can handle multiple addresses at once pasted in, separated via comma or semicolon.
• After every enter, the focus stays on the input field to allow a continuous typing experience.
• I also did a bit of dark mode styling, but nothing too crazy.

That's is so far, pretty barebone initial patch.
The UI is pretty ugly right now and nothing really works, but as I said, this is just for an early evaluation.

What do you guys think about the approach from a code POV?
Does it look decent so far?
Is this a good direction?

Comment 89

[Alessandro Castellani [:aleca]]

Attachment: input-pills.webm

Uploading a quick screencap to showcase it.

Comment 90

[nicorac]

As far as I can see from the attached video sample, this is really awesome!
The interface gives a good experience, expecially to unexperienced users.
Good job 👍

Please keep us updated on your progress; I'm ready to test it when you have a testable version.

Comment 91

[Richard Leger]

Result and video are great... well done! Very promising...

Perhaps I would suggest to set the recipient name in bold perhaps so to create some sort of visual "separation" or "distinction" (both between name and address and between Pills) without adding any extra spaces... this would help to review entries visually... especially when numerous... if that make any sense...

Comment 92

[Frank Breitling]

Would it now also be possible to copy and paste multiple addresses from a draft email to a new one at once?

Comment 93

[Richard Leger]

Mid-air collision detected!
Someone else has made changes to bug 440377 at the same time you were trying to. The changes made were:

No changes have been made to this bug yet.

Added the comment(s):

User Story

Comment 92User image Frank Breitling 2019-10-11 00:56:49 PDT
Would it now also be possible to copy and paste multiple addresses from a draft email to a new one at once?
Your comment was:
Other ideas for later... if you would consider...

What about coloring Pills other than grey to see them better at least?

One color per Addressbook perhaps? Or at leat to distinguish those coming from Addressbook, Auto-Collected and New (maybe not red as this color in associated with error)?

When typing recipient not selected from auto-completion (e.g [e-mail]), one could imagine that pressing space bar any time after @ would act as comma, semi-colon or enter and convert entered data into a Pills (if valid or tolerable mistake) which may be even easier and more natural way to separate recipients perhaps... in addition of other options available...

Also only converted and valid "name <address>" shall be converted in Pills and saved in auto-completion (upon sending? or conversion into Pills?) for new recipient... faulty once shall not be converted, saved or usable (or at own risk?)... a warning would raise upon
conversion/validation... but still allowing end-user to correct or continue at his own "risk" ;-)

You have the following choices:

This will cause all of the above changes to be overwritten, except for the added comment(s).
Throw away my changes, and revisit bug 440377

Comment 94

[Richard Leger]

Other ideas for later... if you would consider...

What about coloring Pills other than grey to see them better at least?

One color per Addressbook perhaps? Or at leat to distinguish those coming from Addressbook, Auto-Collected and New (maybe not red as this color in associated with error)?

When typing recipient not selected from auto-completion (e.g [e-mail]), one could imagine that pressing space bar any time after @ would act as comma, semi-colon or enter and convert entered data into a Pills (if valid or tolerable mistake) which may be even easier and more natural way to separate recipients perhaps... in addition of other options available...

Also only converted and valid "name <address>" shall be converted in Pills and saved in auto-completion (upon sending? or conversion into Pills?) for new recipient... faulty once shall not be converted, saved or usable (or at own risk?)... a warning would raise upon
conversion/validation... but still allowing end-user to correct or continue at his own "risk" ;-)

Comment 95

[Richard Marti (:Paenglab)]

Comment on attachment 9100374 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9100374 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for doing this. This looks very promising and I think this is the way to go. Maybe it's only me but for me it looks a bit strange when the pills aren't in the textbox and the textbox moves to the right. 
What I found is, that tabbing doesn't focus the pills. Also do Mailing lists get the warning class.

::: mail/themes/shared/mail/messengercompose.css
@@ +501,5 @@
> +}
> +
> +.address-pill {
> +  border-radius: 9999px;
> +  padding: 0px 2px;

Please use zero without a unit.

@@ +508,5 @@
> +  background-color: rgba(0,0,0,0.1);
> +}
> +
> +:root[lwt-default-theme-in-dark-mode] .address-pill,
> +.address-pill:-moz-lwtheme {

Don't use :-moz-lwtheme here as this will apply to the pill also with a normal LW-theme which doesn't style the header, only the toolbars above.

@@ +518,5 @@
> +  background-color: rgba(255,0,0,0.1);
> +}
> +
> +:root[lwt-default-theme-in-dark-mode] .address-pill.warning,
> +.address-pill.warning:-moz-lwtheme {

Here too.

@@ +525,5 @@
> +}
> +
> +.address-pill:hover,
> +.address-pill:focus,
> +.address-pill:hover:focus,

:hover:focus isn't needed as both single selectors apply too.

Comment 96

[Paul Morris [:pmorris]]

This is looking good overall, a nice and promising start on a valuable feature! I have a few thoughts.

Glancing over the code I noticed two places where the focus shifts to the subject line. Ideally there would be only one place where that happens. Should be do-able.

As a user I share the concern expressed above about not being able to copy a group of addresses that have been entered. (It's one of my frustrations with the current one-address-per-row UI.) It looks like pasting a group of addresses in would work (right?), but copying them back out looks like a more difficult problem -- if we want to be able to do that and have the pill functionality.

If we can't support selecting and copying (and deleting?) groups of pills, then we might consider offering other options, like maybe a button to "copy all to clipboard" or maybe a toggle that switches the view between "pill view" and "plain text view". Some users might prefer the plain text option. If a plain text view is something we want to offer, then it might make sense by starting with it (simpler) and then adding the pill view.

I suppose you could let backspace delete the previous pill when there's no text in the input field.
I assume it will be possible to edit an address after it has become a pill.

Finally, I have some thoughts about our use of custom elements that are a broader / another topic. I'll save that for an email or something else that's not this bug. (We're about to hit triple digit comments on this bug, so might be worth making it a meta bug at some point.)

Comment 97

[Paul Morris [:pmorris]]

Comment on attachment 9100374 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9100374 [details] [diff] [review]:
-----------------------------------------------------------------

I wrote an email to maildev with some thoughts about custom elements (subject: "custom elements, models, and views").  But that's a larger question/conversation.  Although it might be interesting to think about what more separation between model and view would look like in cases like this.

Comment 98

[Alessandro Castellani [:aleca]]

Perhaps I would suggest to set the recipient name in bold perhaps so to create some sort of visual "separation" or "distinction"

That's a good point, and really easy to implement. I'll see how it looks at a later stage, when thorough UI polish is needed.

Would it now also be possible to copy and paste multiple addresses from a draft email to a new one at once?

Yes. It will be possible to select all the pills to copy and paste them into another draft email.

What about coloring Pills other than grey to see them better at least?
One color per Addressbook perhaps? Or at leat to distinguish those coming from Addressbook, Auto-Collected and New (maybe not red as this color in associated with error)?

I'll explore better colouring at a later stage, but I'm not excluding these suggestions, thanks.

Also only converted and valid "name <address>" shall be converted in Pills and saved in auto-completion (upon sending? or conversion into Pills?) for new recipient... faulty once shall not be converted, saved or usable (or at own risk?)... a warning would raise upon
conversion/validation... but still allowing end-user to correct or continue at his own "risk" ;-)

The pill won't be created unless there's a valid email address in the string (I'm coding this right now).
Fixing a mistaken "saved in auto-completion" action is something we should tackle in a separated bug.

Maybe it's only me but for me it looks a bit strange when the pills aren't in the textbox and the textbox moves to the right.

Yeah, that's just temporary as proof of concept. The final implementation won't show a separated textbox but rather the entire row will be clickable to add more addresses.

Tabbing doesn't focus the pills.

Yes, that's something I'll fix once tackling the keyboard navigation of these elements.

Also do Mailing lists get the warning class.

Yup, I still working on that :D

As a user I share the concern expressed above about not being able to copy a group of addresses that have been entered. (It's one of my frustrations with the current one-address-per-row UI.) It looks like pasting a group of addresses in would work (right?), but copying them back out looks like a more difficult problem -- if we want to be able to do that and have the pill functionality.

Copy and pasting multiple pills at once will be possible.

If we can't support selecting and copying (and deleting?) groups of pills, then we might consider offering other options...

All these things will be available and will emulate the same behaviour of dealing with a simple text string.

I suppose you could let backspace delete the previous pill when there's no text in the input field.

Yes

I assume it will be possible to edit an address after it has become a pill.

And yes

Thank you all for the feedback, I'll keep working and upload another WIP patche once I reach a good state.

Comment 99

[Magnus Melin [:mkmelin]]

Comment on attachment 9100374 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9100374 [details] [diff] [review]:
-----------------------------------------------------------------

Quite a bit of work still to do, but looks like a promising start.

::: mail/base/content/mailWidgets.js
@@ +1726,5 @@
>    customElements.define("attachment-list", MozAttachmentlist, {
>      extends: "richlistbox",
>    });
> +
> +  class MozMailEmailRecipient extends MozXULElement {

MozMailAddress perhaps?
It could be good to follow the naming of parts, see https://tools.ietf.org/html/rfc5322#section-3.4 - down the line we want to support the group syntax too

@@ +1746,5 @@
> +      this.classList.add("address-pill");
> +      this.setAttribute("context", "emailAddressPopup");
> +      this.setAttribute("popup", "emailAddressPopup");
> +
> +      const label = document.createXULElement("label");

would use let for things like these. const is not commonly used for things like this in mozilla code

@@ +1823,5 @@
> +        "presenceTooltip"
> +      );
> +    }
> +
> +    _updateNodeAttributes(attrNode, attr, mappedAttr) {

Hmm, should be using inheritedAttributes functionality instetad for all of this I think.

::: mail/components/compose/content/MsgComposeCommands.js
@@ +6458,5 @@
> +  try {
> +    if (Services.prefs.getBoolPref("mail.autoComplete.highlightNonMatches")) {
> +      toAddrInput.highlightNonMatches = true;
> +    }
> +  } catch (ex) {}

shouldn't be in try/catch

::: mail/components/compose/content/addressingWidgetOverlay.js
@@ +1346,5 @@
>    return window.docShell.QueryInterface(Ci.nsILoadContext);
>  }
> +
> +function isEmptyOrSpaces(str) {
> +  return str === null || str.match(/^ *$/) !== null;

should be .test() instead of match() when you don't need the resulting match

@@ +1357,5 @@
> + * @param event       The DOM keypress event
> + * @param element     The element that triggered the keypress event
> + * @param parentID    The ID of the element's container
> + */
> +function recipientKeyPress(event, element, parentID) {

Need to handle comma as well. not sure if it's here or elsewhere

@@ +1363,5 @@
> +    case "Enter":
> +    case "Tab":
> +      // If the recipient input text is empty or contains only blank spaces
> +      // stop the execution of the script.
> +      if (isEmptyOrSpaces(element.value)) {

no need for this. just check if (!element.value.trim())

::: mail/locales/en-US/chrome/messenger/messengercompose/messengercompose.dtd
@@ +260,5 @@
>  <!ENTITY headersSpace.style "width: 9em;">
>  <!ENTITY fromAddr.label "From:">
>  <!ENTITY fromAddr.accesskey "r">
>  <!ENTITY toAddr.label "To:">
> +<!ENTITY toAddr.accesskey "T">

related to this, not sure you should use the recipient type selector.

Comment 100

[Eyal Rozenberg]

Strongly disagree with this initiative as now stated; and - this is a key aspect of Thunderbird UI which merits a wider surveying of users IMHO.

The opening comment says that one-row-per-address is

OK for 1-3 addresses, but with more it is a real pain.

Not it isn't - in itself. It has issues which may need to be addressed - but going many-address-per-line is trading one set of problems for another. I will elaborate below, based on Olaf's list of arguments. But the main point is that it is difficult and inconvenient to search through multiple lines with variable number of recipients each of varying length and possibly different languages. The fact that Thunderbird does not make you do that (when writing email at least) - is a benefit of Thunderbird over, say, MS Outlook and Evolution. Don't take it away please

Almost every other software supports putting addresses in comma separated list.

And so does Thunderbird. You can type that in or copy-paste it and it'll get adjusted to the one-address-per-line view. If you want to see all the addresses narrowly-spaced together - that could be a feature I suppose, but not replacing what we have now.

Olaf presented several arguments against the current UI approach; let me address them.

1. Habits - if people use other e-mail clients they expect that things work in similar way - please don't classify this simply as aesthetics.

The important habits are for Thunderbird users, not for MS Outlook users; and immitating MS Outlook is not a principle to go by. Also note that when writing formal letters, we typically have one recipient per line - that is the actual habit to uphold.

2. If you have more than 4 recipients you need to scroll - this is what makes me personally unhappy, in outlook you can enter more than 15 addresses (depending on address length:) per one recipient type (To, Cc, Bcc) before you need to scroll (the widget expands up to 4 rows before adding scrollbar).

When you have more than 20 or 30 recipients, you would need to scroll with multiple addresses per line. Plus, the non-scrolling search becomes difficult faster.

It is much easier if you can see all the recipients at a glance.

No. It would be easier if we could see them all aligned. But on a single line - we don't really see them at a glance. Unless there are few of them, in which case we usually see them at a glance as things stand already.

You won't miss any person to send, and what is more important - you won't send to a person that shouldn't get the mail :)

You very well might - since it's easier to miss people in non-aligned layout. Also, with many recipients - you need to scroll anyway, and scrolling is much easier when there's alignment.

3. If you put several entries in "To" and then you change your mind and want to put some of them in "Cc" or "Bcc" it is easier to copy and paste than to change every "To,CC,Bcc" combo.

But if you move a person from "To" to "CC" it is more difficult, since instead of just two clicks - or even a single click, see: https://bugzilla.mozilla.org/show_bug.cgi?id=251917

4. If you want to remove some addresses you need to select every one separately and delete.

That is not inherent to a one-address-per-line layout. This can, and should, be changed. I'm pretty sure there's a bug page about that.

BTW. there is no way to delete unused fields, it is possible to delete only addresses.

You don't need to "delete unused fields"; there are infinitely many unused rows at the bottom of the recipient list.

5. If you send several messages to the same group of people (or a large portion of addresses is the same - eg. all addresses in CC). With "multi address" fields (like in Evo, Outlook) you can just select and do a copy/paste. In Thunderbird it is impossible.

This is just the combination of the previous issue and the next issue.

To make this even harder, in Thunderbird the text in message header pane cannot be selected, so you can't select addresses from a message and put in address fields. I'll fill separate bug for it anyway.

Indeed, that should be addressed - we should be able to copy multiple recipients when reading messages as well. For now it's possible by viewing the source and copying the text from there... :-(

A few final points:

• One line/field per recipient means you can order your recipients either by category (To/CC/BCC), but also in other ways (e.g. alphabetically), while one line per category forces an ordering.
• I would have liked to see better real-estate use by making it possible for users to indicate they want to add recipients in another column, splitting the recipient list section of composition window vertically (but not right from the get-go so it doesn't look like a bingo card).
• A one-line-per-category preview of the recipients would also be nice, but not as the only way to enter them.
• aleca - putting aside the fact that I don't like the basic notion, it seems nicely implemented. I don't mean to disparage your great work!

Comment 101

[Alessandro Castellani [:aleca]]

100 comments....yay...
Thanks for your feedback Eyal, but it's missing completely the scope of this patch.

Strongly disagree with this initiative as now stated; and - this is a key aspect of Thunderbird UI which merits a wider surveying of users IMHO.

The proposed UI with a thorough explanation of the various usability features was released here: https://thunderbird.topicbox.com/groups/ux/T37151b06e9361fac/work-started-on-redesign-recipients-address-fields-to-cc-bcc-etc

It was reviewed and approved by the core team and the technical director, and the exploration of this solution was initially introduced 7 months ago in bug 1535503.

All your points and issues are mostly related to the way you're personally used to use the compose message, how you feel comfortable, and the workarounds you learned with time to overcome some issues.
Those are not an indication of good usability and intuitive UI.

The proposed solution introduces many usability improvements, and helps streamlining the UI for multiple scenarios, while the current implementation comes with too many "IFs" and "BUTs", and it doesn't scale gracefully.

I'm sure you'll have many further points, so please keep writing in the topicbox thread and let's limit this bug to code related feedback.
Thanks.

Comment 102

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

All right, this is starting to get into a pretty good shape.
Here's the full list of what's working:

• Pasting multiple addresses will be converted into pills.
• Pills have focus.
• Double Click or hit Enter on a Pill turns it back into an editable autocomplete input.
• Hitting Enter after editing a pill will update the values and switch back to a pill.
• Hitting Escape while editing a pill will revert to the previous value.
• Backspace from the main autocomplete input selects the last pill.
• Delete or Backspace while a pill is focused deletes it.
• Navigate all the pills with Tab and Shift+Tab.
• Warning colour if an address is valid but not in the address book.
• Error colour if an address is not valid.

What's missing

• Add the other rows through links.
• Links will be populated based on user settings (Reply-To, Followup-To, etc).
• Ability to collapse/expand long list of pills.
• Select multiple pills at once with the usual various shortcuts.
• Drag&Drop one or multiple pills within the same container to reorder.
• Drag&Drop one or multiple pills between various containers.

More things to code

• Update all the tests
• Generate pills when forwarding/editing an email
• Enable context menus for the pills like in the email header.

I'm sure I'm missing a million of other things, but slowly, step by step, this is getting pretty.

Comment 103

[Richard Leger]

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

• Backspace from the main autocomplete input selects the last pill.

Hitting Backspace in such situation shall delete the last pill (1) not select it... pressing left arrow (or shift+tab) instead would select it...

That would seems a more natural and expected behaviour from Backspace key...

Alternatively (or in addition) if it was a pill just converted, hitting Backspace shall move back the pill in edit mode (2)... with the pointer set after last character... it would act like erasing last space comma or semi-colon that triggered pills conversion... but only if it was just last edited/converted... otherwise behaviour (1) may prevail...

Just a thought...

Comment 104

[Magnus Melin [:mkmelin]]

Comment on attachment 9101029 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9101029 [details] [diff] [review]:
-----------------------------------------------------------------

Looking good. One think I notice is I can't really enter addresses not in the address book, they too need to become a pill. 
I think pills should be separated by commas, and typing in a comma should make whatever was typed in a pill too (tying into the previous sentence.)
A [x] icon at the end of each pill to remove it could be nice. Later on, maybe a nice tooltip too, but that would be for a followup.

I think the address type selector(s) should stay like they were so you can select To/CC/Bcc/Newsgroup/Followup/Reply-To, and others. Or be somehow else accessible. In the patch I can only add To.

Comment 105

[Richard Marti (:Paenglab)]

Comment on attachment 9101029 [details] [diff] [review]
440377-recipients-pills.patch

Looks good at this stage. The tabbing works now. When entering contacts and the autocomplete list is shown and I click on a contact in it all is okay but when I use ENTER the pill is created but email address of the contact is also shown behind the pill in the text box.

Comment 106

[Paul Morris [:pmorris]]

Comment on attachment 9101029 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9101029 [details] [diff] [review]:
-----------------------------------------------------------------

Looking good!  Everything works as described.  (I was able to make pills from non-address-book addresses just fine.)  Navigating pills with left and right arrow keys (in addition to tab and shift-tab) might be worth adding to the to-do list.  I'm still thinking about how to best do model/view separation with custom elements, but I'll post to that email thread with some thoughts on that.  So good to see this updated UI making its way into TB!

::: mail/base/content/mailWidgets.js
@@ +1890,5 @@
> +        this.classList.remove("error");
> +      }
> +
> +      if (
> +        this._isValidAddress(this.email) &&

No need to call _isValidAddress twice here, just call once and assign result to a local variable.  (Noticed this so thought I might as well comment, though I realize this is still WIP.)

Comment 107

[M Lopez-Ibanez]

It is great to see improvement in this bug. I'd strongly recommend installing MRC Compose for comparison. Features of MRC Compose that help a lot in "more than one address per line" mode:

• Default text that informs users of how to input addresses.
• Ability to show/hide CC, BCC, Reply-to and other fields.
• Counter of recipients on the left of the corresponding field (helps to see if one has double paste a very long list of recipients)
• Ability to configure which address books are searched (LDAP is too slow for me, but sometimes I need to enable it)
• Sorting criteria (Popularity!)

Comment 108

[M Lopez-Ibanez]

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

• Select multiple pills at once with the usual various shortcuts.

It will be useful if keyboard shortcuts will work exactly as if the list of pills were words separated by ",":

At start of a pill:

• backspace: convert the previous pill to text, pressing backspace again will start deleting text
• ctrl-backspace: delete previous pill
• ctrl-del: delete next pill
• left-arrow: convert the previous pill to text and place the cursor under the last letter (in case I did a typo while writing the address)
• ctrl-left-arrow: convert the previous pill to text and place the cursor at the start of the pill
• ctrl-shift-left-arrow: select the previous pill (one should be able to do this several times to select several pills, like selecting several words)

In the middle of the text of a pill:

• home-end: move to start/end of pill
• ctrl-left/right-arrow: move between words (as usual)
• "," close the pill, start a new one

At any point:

• ctrl-home: convert the first pill to text and move the cursor under the first letter.
• ctrl-end: convert the last pill to text and move the cursor under the last letter:
• shift-ctrl-home: select all pills from current position to the first one (thus, end -> shift-home selects all pills, the Ctrl+C/X copies/cuts all addresses, separated by commas)
• shift-ctrl-end: select all pills from the current position to the last one
• Ctrl-Z undo last edit (undo delete pill/s, undo typed pill/s)

I'm sure you get the idea. This is the standard navigation/editing shortcuts, just that pills are treated as whole words unless they are being edited. Otherwise, navigating/editing many addresses will become a pain and all the benefits of having addresses in a single line (like MCR Compose does) will be lost.

(It would be nice to add to the whish-list to have X and contact photos in the pills like these chips here: https://material-ui.com/components/chips/ )

Comment 109

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

More improvements to the recipient pills.

This updated patch brings lots of accessibility improvements and various fixes.

• You can navigate through the various pills with the arrow keys other than TAB and SHIFT+TAB.
• Added the X icon to delete pills.
• Fixed the email address re-appearing after a pill was created.
• Fixed a couple of conflicts with the autocomplete while editing pills.
• Improved UI and colours between the different states.

I have another WIP patch that adds the various CC, BCC, etc, almost ready to be uploaded, but I need a bit more time to fix a couple of quirks.

Comment 110

[Richard Marti (:Paenglab)]

Comment on attachment 9106455 [details] [diff] [review]
440377-recipients-pills.patch

Looks very good. Only two things:
- the delete button inside the pill has a uneven border around it. On the right it is wider than top/bottom.
- when I edit a pill (not easy to find without a context menu :) ) it expands and shifts the following pills to the right. What about letting it the same width and only expand when the text inside will be wider when editing?

Comment 111

[Richard Marti (:Paenglab)]

Attachment: Uneven border around the delete button

Comment 112

[Richard Marti (:Paenglab)]

Attachment: Edit the pill makes it very wide

Comment 113

[Richard Marti (:Paenglab)]

And I forgot to write that the text box is taller than the From menulist and the subject text box. Maybe reduce the space above/below the pill to make it less tall.

Comment 114

[Paul Morris [:pmorris]]

Comment on attachment 9106455 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9106455 [details] [diff] [review]:
-----------------------------------------------------------------

Looking good Alex!  It's great to see the progress on this.  I scanned through the code and made a few comments.  It worked well when I tried it out.  Here's a few little things I noticed and some other thoughts:

- Double-click to edit a pill, then click to put the cursor elsewhere, the pill stays in 'editing mode'.  In this way you can have multiple pills in editing mode at the same time.  Seems like it should exit editing mode when it loses focus?  (Probably just still to do.)

- When adding addresses, the cursor goes to the next line before you expect it to.  And when resizing the window the height of the "TO" field moves from one to two lines where you don't expect it.  Would it work to shrink the width of the text input element to avoid these things?

- Might not be worth it, at least in the first release of this, but users might want to be able to enter new addresses between existing pills.  So click between or use keyboard to navigate between and then start typing.  Something to keep in mind, if you're not already.

- Would be a good idea to develop this behind a temporary pref, so we could go ahead and land code incrementally as we go rather than all at once?  Then when it's ready, turn it on?

::: mail/base/content/mailWidgets.js
@@ +1773,5 @@
> +      this.setAttribute("originalInput", val);
> +    }
> +
> +    get originalInput() {
> +      return document.getElementById(this.getAttribute("originalInput"));

getElementById?  Seems odd here.

@@ +1781,5 @@
> +      return this.getAttribute("emailAddress");
> +    }
> +
> +    set email(val) {
> +      this.setAttribute("emailAddress", val);

Probably better for these identifiers to be the same like the others (here email != emailAddress).  Maybe consider "address" since it's shorter, with less room for confusion?  With "email" it might not be clear if it's the address or the whole message (e.g. "delete email").

@@ +1806,5 @@
> +    }
> +
> +    set displayName(val) {
> +      this.setAttribute("displayName", val);
> +    }

I wonder about how much we gain from these getters and setters that are just doing this.getAttribute/this.setAttribute... but fine to use them if you find them helpful.

@@ +1838,5 @@
> +    _setupEmailInput() {
> +      let pillCount = document.querySelectorAll("mail-address").length;
> +      this.appendChild(this.fragment);
> +      this.emailInput = this.querySelector(`input[is="autocomplete-input"]`);
> +      this.emailInput.setAttribute("id", `toAddrInput#${pillCount}`);

I'd avoid the "#" in an id.  I ran across this recently in the calendar code and it didn't work to do say `.querySelector("#toAddrInput#1")`.

@@ +1900,5 @@
> +      });
> +
> +      this.emailInput.popup.addEventListener("click", () => {
> +        this.emailInput.dispatchEvent(
> +          new KeyboardEvent("keypress", { key: "Enter" })

This seems a little odd to have a click event fire off a keypress event.  Wouldn't it be better to have both events calling the same function, if they are to do the same thing?  If there's a reason to do it this way, I'd suggest adding a comment.

@@ +1924,5 @@
> +      // });
> +    }
> +
> +    _isValidAddress(address) {
> +      return address.includes("@", 1) && !address.endsWith("@");

Hm, I wonder if we already have some code for validating addresses?

::: mail/components/compose/content/MsgComposeCommands.js
@@ +6478,5 @@
>  
>  function setupAutocomplete() {
> +  let toAddrInput = document.getElementById("toAddrInput");
> +  toAddrInput.popup.addEventListener("click", event => {
> +    toAddrInput.dispatchEvent(new KeyboardEvent("keypress", { key: "Enter" }));

Same question as above on click -> keypress events.

::: mail/components/compose/content/messengercompose.xul
@@ +2021,5 @@
> +                            minresultsforpopup="2"
> +                            ignoreblurwhilesearching="true"
> +                            onfocus="highlightAddressContainer(this);"
> +                            onblur="resetAddressContainer(this);"
> +                            onkeypress="recipientKeyPress(event, this, 'toAddrContainer');"/>

Hm, unfortunate that this is repeated in the CE and here in the XUL.  Not a big deal, but I wonder if it would be worth dynamically inserting it instead.

Comment 115

[Alessandro Castellani [:aleca]]

Thanks all for the positive feedback.
Here are some notes regarding what your wrote.

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

the delete button inside the pill has a uneven border around it. On the right it is wider than top/bottom.

This should be fixed in the upcoming patch, but anyway I'm not super focused on pixel perfection across OSs, so something will definitely look not aligned until the final patch.

when I edit a pill (not easy to find without a context menu :) ) it expands and shifts the following pills to the right.

Fixed in the upcoming patch, the width of the editing area should respect the length of the pill.
I'm also adding a context menu with the "Edit" item. I was also thinking, would be worth considering showing a pencil icon on hover? So the user can click that to edit the address? More discoverable than a double or right click.

(In reply to Paul Morris [:pmorris] from comment #114)

Double-click to edit a pill, then click to put the cursor elsewhere, the pill stays in 'editing mode'. ..Seems like it should exit editing mode when
it loses focus? (Probably just still to do.)

I'm not sure about this as it might be super annoying for the user. Think about the scenario where you're writing an address, and you need to copy it from another window, or maybe you click somewhere else to view another email and confirm the address, and the pill reverts back losing your temporary editing.
Maybe I can implement a condition in which if the email address is valid while losing focus, the pill reverts back, but if it's not, it remains in edit mode.

When adding addresses, the cursor goes to the next line before you expect it to. And when resizing the window the height of the "TO" field moves from one to two lines where you don't expect it. Would it work to shrink the width of the text input element to avoid these things?

This is an annoying limitation I'm not sure how to overcome as a wider input is necessary for the autocomplete panel. That panel is as wide as the input field, so we can't shrink it down too much otherwise the autocomplete list can get unreadable.

Might not be worth it, at least in the first release of this, but users might want to be able to enter new addresses between existing pills.

That'd be nice, but I'm not sure it's possible with the current autocomplete field. Maybe for the future.

Would be a good idea to develop this behind a temporary pref, so we could go ahead and land code incrementally as we go rather than all at once? Then when it's ready, turn it on?

I'm not against this, I'll let Magnus decide for this matter.

Regarding code reviews

Indeed, there are a lot of quirky decisions and repeated code, but that's normal for now.
For now, I'm more interested in making this work and have a working patch for the entire email sending workflow.
Lots of things are probably useless and redundant, so apologies if I'm giving you stomachache :P

Stay tuned for another patch.

Comment 116

[Paul Morris [:pmorris]]

Think about the scenario where you're writing an address, and you need to copy it from another window, or maybe you click somewhere else to view another email and confirm the address, and the pill reverts back losing your temporary editing.

Ah, good point. Suggestion withdrawn. That is too bad about the width of the autocomplete panel.

Comment 117

[Thomas D. (:thomas8)]

Exciting stuff happening here, thanks everyone, and special thanks to Alessandro for taking on the beast!

1.) Could we fine-tune the click pattern for "edit pill" & friends a bit?

Single-click on pill: Select pill (select more pills with Ctrl+click/shift-click)
Single-click on selected pill: Edit pill ***
Double-click on pill: open associated contact (create new contact if not yet associated)

Rationale: Exploit users' habitual click-patterns from other, similar tasks, e.g. selecting and renaming files in Explorer (ux-consistency).

*** "Edit pill" probably refers to a direct, one-time edit, right? As opposed to changing a possibly associated contact in the address book.

2.) I'm not so sure yet about the best behaviour for outside clicks while editing pill (leaning towards exiting edit mode), but definitely we can't have a situation where multiple pills are in edit mode, as was pointed out above.

3.) Thunderbird users have been with neat, vertically aligned addresses forever; I'd think that for many users this may have become more than habit formation, they might actually appreciate this as a distinctive feature. The UX is poor not mainly because of the vertical alignment, but because of technical design and behaviour shortcomings, e.g. that we can't batch-change multiple addresses from To to BCC (in fact, we can't even select and copy multiple addresses). Also note that some of the behavioural shortcomings have been addressed in the current release version, e.g. recipients can now be navigated with cursor up/down, click to select single recipient has been implemented (pill-style), DEL and Backspace now allow controlled and predictable sequential deletion of multiple recipients with 2 keystrokes each.

I recommend:

• allowing some way of vertically aligning/sorting/etc. recipients in the new pill scheme of things (not doing so may significantly irritate existing users). This might be as "simple" as having a "legacy view" which only allows one pill per line. As others have suggested, we should try to separate views from content/roles, and explore creative variants like multi-column views of pills.
• to ease the transition for existing users, allow a two-click (or single-click) way of changing the recipient type of each pill (contextual action, hover menu etc.).
• addressing the existing UX shortcomings in the pill scheme (provide multi-select and allow multi-selection-actions like mass-changing recipient type, mass-deletion, mass-copy etc.

I'm aware that some of these might require an incremental approach, but we should ensure now that the new design allows for such.

Comment 118

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

All right, here's another big improvement.

Keyboard navigation

• Now you can navigate between pills with the arrow keys and tab.
• If you hold CTRL, you can select multiple pills both with the click and the arrow navigation.
• CTRL+A will select all the pills if one is already selected (I need to add this to the input field as well).
• You can delete multiple selected pills.

Context Menu

• Pills have context menu which allow to edit, delete, and copy the email address.

Cc, Bcc, etc.

• The other fields are there and can be revealed by clicking on the label.
• You can hide the field back, and get prompted with a confirm dialog if the field has addresses in it.
• The header area grows to accommodate all the pills and fields, but as soon as you resize it, the area turns scrollable.

What's missing

• Making this thing actually work and send emails.
• Populate the fields when forwarding an email.
• Add Drag&Drop for the pills.

As usual, the overall code is very dirty, with lots of repetitions and some nasty solutions. I'm doing it this way for the sake of having a fully working prototype which we can later iterate to optimize.

The next patch I'll upload should come with everything currently missing, and we can start the review process.

Thank you again everyone for the insightful feedback and your patience on waiting on this monolith.

Comment 119

[michelr]

Hi, I'm the developer of MRC Compose.

Alessandro, from what I saw with screenshots, you're doing a great job ! Thank you to take care of such an important point in Thunderbird.
Are you ok if I try to integrate some parts of your prototype (ideas/code) in MRC Compose ? It's something I wanted to do from the beginning, but it requires so much low-level handling that I always postponed it.
Would you like to be notified of any progress I may make ?

Cheers,
Michel

PS: MRC Compose is now compatible with TB68 and can search in Cardbook !

Comment 120

[Paul Morris [:pmorris]]

Comment on attachment 9107655 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9107655 [details] [diff] [review]:
-----------------------------------------------------------------

Looking good!  I tried all the things you listed and everything worked well.  I noticed some minor glitches and potential refinements that could be addressed here or in a follow-up.  

- How to resize the header area to make it scrollable?  I wasn't able to get this to happen.

- I was able to tab to the Cc, Bcc, etc. labels, but hitting enter when they are selected doesn't do anything yet.

- Do we want to alert the user that they are deleting addresses by closing Cc, Bcc, etc. or allow them to undo it after the fact?

- Backspace or delete to remove all pills.  If one is in editing mode, you get stuck on it.  Backspace (or delete) stops having any effect.

- Start editing a pill, delete all text in it, hit enter, nothing happens. I'd expect the pill to be deleted.

- Click a pill to select it.  Hit tab to move to the next pill.  The first pill stays selected in addition to the next pill.

- If a pill that's not the first or last pill is in editing mode and you use the arrow keys to try and navigate to it and past it, it doesn't work like with tab and shift+tab. When you get to that pill you can't arrow key away from it, only tab into it to edit it some more.  Arrow keys should work like tab and shift+tab for pills in editing mode.

- When a pill is in editing mode, maybe show a "checkmark" button (or similiar, in the place where the "X" button is), that would do the same as hitting "enter" on keyboard?  Then if I switch away from an editing pill, I can get it out of editing mode directly by using the mouse.

- If the last pill is in editing mode, you can't move the cursor to the text input to add another address without taking the pill out of editing mode first.

- It seems a little odd to have more than one pill in editing mode, but maybe it's fine and just a matter of getting used to it.  But if more than one pill can be in editing mode, it would be good to differentiate a selected editing pill from a non-selected editing pill, so you can tell where you are when tabbing through them.

Comment 121

[Richard Marti (:Paenglab)]

Comment on attachment 9107655 [details] [diff] [review]
440377-recipients-pills.patch

Looking good.
- When editing a pill and the text is longer than the pill, the text goes outside of the pill.
- I'm not so happy with the Cc: Bcc: line. It looks for me a bit like text that is there by accident. What about a second textfield with the menulist in front to choose the desired sending option, like the ones we use in our actual composer? Defaulting to Cc:?
- In the line above I have Newsgroups: and Followup-To: too but they don't work.
- After pressing F9 you have the Contacts sidebar with the buttons "Add to To:", "Add to Cc:" and "Add to Bcc:" but they don't work.

Comment 122

[Richard Leger]

(In reply to Paul Morris [:pmorris] from comment #120)

If I may suggest as an end-user...

• Do we want to alert the user that they are deleting addresses by closing Cc, Bcc, etc. or allow them to undo it after the fact?

Hiding Cc,Bcc,etc... shall not remove content in the first place... it should just hide it... addresses shall only be deleted if delete action is triggeref by end-user.

• Backspace or delete to remove all pills. If one is in editing mode, you get stuck on it. Backspace (or delete) stops having any effect.

For me a pill in edit mode is just a pill converted back to simple editable text (no pill anymore), as it was before it became a pill. It would be much simpler behaviour in concept. We should try to keep To,Cc,Bcc as close as possible to editable text fields, the pill system being just a visual and management aid.

Hitting backspace would simply erase text... if text become empty and you keep backspacing, I would expect previous pill (if exist) to convert back to text, and characters to start being deleted again...

Similar behaviour with the delete key.

• Start editing a pill, delete all text in it, hit enter, nothing happens. I'd expect the pill to be deleted.

If pill in edit mode become just text, if empty text I would expect the pill to no longer exists (effectively deleted). You would just end up with curser between two pills. Therefore hitting enter having no effect an empty text being not a valid email address to convert back into pill.

In a similar way, if I place cursor between two pills and hit Backspace (or Delete), it would convert previous (or next) Pill into editable text that you can further delete (per character) or amend... in a very similar way to editing a simple text...

• Click a pill to select it. Hit tab to move to the next pill. The first pill stays selected in addition to the next pill.

A bug it seems... hitting tab shall shall move selection to next tab. Previous selected pill shall remain selected only if shift+tab wss pressed together.

• If a pill that's not the first or last pill is in editing mode and you use the arrow keys to try and navigate to it and past it, it doesn't work like with tab and shift+tab. When you get to that pill you can't arrow key away from it, only tab into it to edit it some more. Arrow keys should work like tab and shift+tab for pills in editing mode.

If pill is convert back to text (edit mode) I would expect arrows to only allow navigation within the editable text.

Though one could imagine that if you reach start (or end of text) and keep pressing left arrrow (or right arrow):

• if edited text email address is valid, it would convert the text into a pill, and set the previous (or next pill) into edit mode
  OR
• if not valid, do nothing (just raise alert that address is invalid and must be valid)

• When a pill is in editing mode, maybe show a "checkmark" button (or
  similiar, in the place where the "X" button is), that would do the same as
  hitting "enter" on keyboard? Then if I switch away from an editing pill, I
  can get it out of editing mode directly by using the mouse.

For me editing mode should just be text mode... no longer a pill... therefore there should be no need for such checkmark... just nee to hit Enter to convert text back into pill (or via click outside editided text).

• If the last pill is in editing mode, you can't move the cursor to the text input to add another address without taking the pill out of editing mode first.

Again editing mode shall just be text mode.

• It seems a little odd to have more than one pill in editing mode, but maybe it's fine and just a matter of getting used to it.

As edit mode is just text mode it allow to enter (or edit) multiple addresses at once... especially handy when copy/pasting multiple addresses from wherr else...

Though I would expect to only be able to edit one pills at a time after convertion not multiple unless they are next to next...

Comment 123

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

Another patch, another iteration.

What's new

• Now you can CTRL+C and CTRL+X on the pills to copy or cut them, it works for single or multiple selected pills at once.
• The various selection interactions should have been fixed.
• Add addresses from Address Book has been fixed.
• Hitting Enter on and an empty pill while editing will delete that pill.

Pills interactions

• If you're editing a pill and you keep hitting backspace or delete the pill WILL NOT be deleted. This is to prevent accidental and unwanted deletion while clearing out wrong addresses.
• While in editing mode, the arrow keys won't allow switching from a pill to another. This is to prevent accidentally leaving the pill which it can be extremely annoying. Also, hijacking the arrow keys inside an input field is not great.

Alerting the user when removing a row with pills in it
Yes, we should absolutely prompt the user with a confirm, and we should clear that row if the user decides to remove it.
Imagine the scenario where the user is forwarding an email with 100+ addresses inside the CC container. The user deletes that container because it doesn't need it and starts editing the content. Then, he realizes he needs to CC that email to another contact and reopens the CC container, finding once again the 100+ addresses he though he deleted.
The very own X button, which implies the deletion of a row, can't trick the user by not actually deleting the content within.
It's linear, makes more sense, and it avoids us developers doing a check on submit like "the CC field has 100+ addresses, but it's hidden, so don't use them".

Since this patch is getting pretty big, I won't implement the Drag&Drop feature in here, but I will do that as a follow up bug after this first iteration lands on daily.

Comment 124

[Richard Marti (:Paenglab)]

Comment on attachment 9109194 [details] [diff] [review]
440377-recipients-pills.patch

Looks good. Looks almost complete.
The .address-container doesn't work well with dark theme, can be fixed later.
Mailing lists pills are coloured red like invalid addresses.

Comment 125

[Paul Morris [:pmorris]]

Looks good. Here are a few new things I noticed, but nothing that should block this from landing, just potential follow-up polishing stuff.

• Maybe add "cut" to pill context menu.
• After cutting and pasting a grey background, in-address-book pill the background was orange on the second instance, so not recognized as in the address book.
• Should you be able to tab to the "X" button for removing Cc/Bcc/Reply-to row? Currently you can't tab to them.

Comment 126

[Magnus Melin [:mkmelin]]

Comment on attachment 9109194 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9109194 [details] [diff] [review]:
-----------------------------------------------------------------

I don't particularly like how the Cc Bcc Reply-t Newsgroup Followup-to field names are lined up under the To line. 
Many webmails at least seem to favour putting them rightmost on or above the To/From. Not sure where we should put them, especially as we have more fields than typical. Also we'd not want to have the Newsgroup and Followup-To ones there showing for all mails.

For the To row, what's not working atm is at least entering addresses that aren't in your ab already. And commas between addresses should work.

The Subject "box" seem to have lost its top border

Comment 127

[Alessandro Castellani [:aleca]]

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

The .address-container doesn't work well with dark theme, can be fixed later.

Ah yes, I will definitely need your expertise to be sure everything looks top notch in dark mode.

Mailing lists pills are coloured red like invalid addresses.

This should have been fixed in the upcoming patch.

(In reply to Paul Morris [:pmorris] from comment #125)

Maybe add "cut" to pill context menu.

Done.

After cutting and pasting a grey background, in-address-book pill the background was orange on the second instance, so not recognized as in the address book.

Fixed.

Should you be able to tab to the "X" button for removing Cc/Bcc/Reply-to row? Currently you can't tab to them.

Good call, done.

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

I don't particularly like how the Cc Bcc Reply-to Newsgroup Followup-to field names are lined up under the To line.

Indeed, that's not an optimal placement. I'm experimenting with other locations, one of which is inline the To field, right aligned.
Also, I'm thinking about "hiding" the less common labels behind a dropdown menu. I'm not sure yet, still trying to find the best solution.

Also we'd not want to have the Newsgroup and Followup-To ones there showing for all mails.

That shouldn't be the case as I've updated the code in the hideIrrelevantAddressingOptions() to not show those fields based on the account type.

For the To row, what's not working atm is at least entering addresses that aren't in your ab already.

That works for me, can you elaborate on that? If you write a new address and you hit Enter, what happens?

And commas between addresses should work.

That works for me as well. If I write multiple addresses separated by a comma and then hit Enter, all those addresses are converted into pills.

The Subject "box" seem to have lost its top border.

It never had a top border. It has always been the bottom border of the field above.
Moving the Cc, Bcc, etc, labels to the right should fix that visual "issue".

I'm still struggling a bit with the email submission and the automatic fill-up of the fields on replied or forwarded emails.
Those methods are tightly coupled with so much stuff that it's giving me a hard time.

Sorry for the delay. A new patch ready for review will be uploaded ASAP.

Comment 128

[Magnus Melin [:mkmelin]]

Try this: add an address with autocomplete, then try to add an address that's not in the addressbook.
You shouldn't have to press enter either. Once you add a comma it's the next address.

Comment 129

[Alessandro Castellani [:aleca]]

Ah, I see, that's a UX issue.

In order to solve this I've implemented the creation of the pill when the comma character is pressed, or onblur of the input field with a valid email address.
You're right, users shouldn't be force to hit Enter or Tab to create a pill.

Thanks for the detailed explanation.

Comment 130

[Richard Leger]

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

Ah, I see, that's a UX issue.

In order to solve this I've implemented the creation of the pill when the comma character is pressed...

Would semi-colon ';' be usable as email address separator? Basically having same effect as comma ','? This may bring some flexibility especially for copy/paste, if not directly by end-user possible habits...

When To, Cc, Bcc are empty would there be any hint text in the field to indicate to 'Enter email addresses comma or semi-colon separated' as a tip? May be handy for new or accustomed users ;-)

Comment 131

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

All right, I think it's time to start the first of (I suspect) many reviews.

The whole workflow works now.
Sending emails work.
The various forward, reply, etc, work.
Writing to an entire list work.

Old methods
I started removing the unused methods of duplicating rows and checking for dummy_rows, but I think I'll stop for now as some of those methods are tightly coupled with the Address Book methods.
I will take care of those in a follow up clean-up patch.

UI and UX
Currently not at a great state, but if it's OK for you Magnus, I'd like to focus on landing a working and usable patch, with all the tests green, to then follow up with a UI-centric patch on a different bug.
Having the working prototype will help me to define better what we need and how it should be improved.

Tests
All the tests are failing, obviously, but I will hold off on those until this patch gets an r+.

What's missing
Dropping email addresses on a field is broken, but it should come in the next patch.

Thanks everyone for reviewing this beast.

Would semi-colon ';' be usable as email address separator?

Done :D

Comment 132

[Magnus Melin [:mkmelin]]

Re semi-colon: I think you shouldn't. Pasting will do the right thing nowadays. Eventually we want to support group syntax and I wouldn't want to have users wrongly trained to use it.

Comment 133

[Richard Leger]

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

Pasting will do the right thing nowadays.

You mean it would convert semi-colon into commas and convert addresses into pills on the fly?

Eventually we want to support group syntax

How using semi-colon as separator prevent group syntax?

Is there an RFC or other reference that indicate/recommand semi-colon as email separator for group/distribution list?

Is your idea to be able to create an auto-completion group by just typing email addresses separated by semi-colon?

and I wouldn't want to have users wrongly trained to use it.

You may be right on this, maybe then accept it as a valid separator but don't encourage it...?

This would bring flexibility - some email client use semi-colon as separator or accept it as alternative to coma...

Unless of course you have other plan/idea...

Comment 134

[Magnus Melin [:mkmelin]]

Just try copy-pasting something with semi-colon and you'll see we make that right (already on release).

Group syntax is RFC 5322, see "Group Addresses". Like "Family: Foo <[e-mail]>, Bar <[e-mail]>"; "Friends: Joe <[e-mail]";

Comment 135

[Thomas D. (:thomas8)]

Could someone kindly offer a try build? TIA.

Comment 136

[Alessandro Castellani [:aleca]]

(In reply to Thomas D. from comment #135)

Could someone kindly offer a try build? TIA.

I'll do that later once the majority of the tests are fixed, and a couple of missing features are added to the patch.
Probably before the end of this week.

Comment 137

[Paul Morris [:pmorris]]

Comment on attachment 9110172 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9110172 [details] [diff] [review]:
-----------------------------------------------------------------

Nice work!  It's looking good to me.  I haven't read through the code yet, just tried the UI.  Sent a message, checked reply/forward, etc.  All seems in working order.   

One tiny nit: usually cut comes before copy, but currently copy comes before cut in the drop down menu.

I'm guessing the request for a try run was so that people can download the built binary and try it out, without having to build it locally.

Comment 138

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

Here's another patch with more improvements:

• Address Book lists are now recognized and properly styled.
• Drag and Drop of addresses from the Address Book sidebar to the field works, as well as dropping them on top of the labels.
• All the Address Book methods have been moved to the dedicated JS file. The addressingWidgetOverlay.js now handles only the compose dialog methods.

Here's a try run with all the builds:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=2bb73e8838882784652dd6e5d91640d5474bb9c2

Be aware that:

• All the tests will fail.
• The UI is not final and many improvements will come in the next days.

Please, test it and report issues!
I'm sure this patch will introduce many many many regressions, so, ignoring the fact that this bug is gonna probably reach 200+ comments, I'd like to ask anyone interested to grab a build, test it, and report any issue or bug you stumble upon.

Comment 139

[Ben Bucksch (:BenB)]

I'm testing the patch, and it works really well! I like the UI design, too.

A few minor glitches that I noticed and that should be easy to fix:

• It's too happy to make entries without email address. E.g. "where.com" results in "where.com <>" in a red pill (where.com <> [x]). I don't think such entries with syntactically invalid email address should be created at all. As pill, they look valid, even if red. And as pill, they are hard or impossible to fix. I think they should just stay text until they are fixed.
• It's a little to quick to end the entry and create a pill. E.g. on my keyboard, the comma , (which creates a new pill) is right next to . (which I need for an email address), and I've repeatedly accidentally create a new pill when I didn't want to. Once it's a pill, it's hard to fix. Not sure how I'd solve that.
• Backspacing into a pill should not delete the pill, but re-enter edit mode and delete only the last characters. Probably similar for other key strokes like arrow keys and others.

All these are minor "editor rules" issues. Technically, this works really well, from what I'm seen in my quick test. Good job!

Comment 140

[Ben Bucksch (:BenB)]

Style (sorry for the bike-shedding):

• I don't like the colors, esp. the yellow. I'd stick to a more conventional grey. Color doesn't fit here.
• The contrast between foreground color and background is too low.
• If you use red for error (I'd rather not create a pill at all in this case, see above), make the red much more flashy.

Comment 141

[Thomas D. (:thomas8)]

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

• Backspacing into a pill should not delete the pill, but re-enter edit mode and delete only the last characters.

Please don't. Backspacing onto a pill should delete the pill, and consecutive backspaces should delete pills backwards one by one. Backspacing to delete character-wise only makes sense for a single pill in edit mode. It's highly unlikely that user needs to edit more than one or two odd pills in a delete-per-character fashion, whereas it's much more likely that recipients need to be removed. We have just introduced the distinction in bug 1527547 to improve the UX-efficiency of removing recipients, and it works well: For editing a single recipient, backspace deletes characterwise until that recipient (display name and email address text) is completely deleted. Pressing backspace again on an empty recipient row will remove the row and start removing recipients one by one. I haven't tried pills yet, so I'll comment again after that, but I believe wrt keyboard deletion pills should behave similar to the current release version behaviour, where my patch of bug 1527547 has created a precursory pill-style experience.

Comment 142

[Richard Marti (:Paenglab)]

Comment on attachment 9110415 [details] [diff] [review]
440377-recipients-pills.patch

Looks almost ready.
- The fields are no more vertically aligned in columns because the text fields begins directly after the To: label.
- When I DnD a valid address from sidebar the address is shown red first until I press the ENTER button or click in a other text field. A valid address should never be red and when dropping a address from the sidebar it should change immediately convert to a pill.

Comment 143

[Alessandro Castellani [:aleca]]

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

It's too happy to make entries without email address. E.g. "where.com" results in "where.com <>" in a red pill (where.com <> [x]).

Good call. Text won't be converted into pills if the address is not valid.

It's a little to quick to end the entry and create a pill. E.g. on my keyboard, the comma , (which creates a new pill) is right next to . (which I need for an email address), and I've repeatedly accidentally create a new pill when I didn't want to. Once it's a pill, it's hard to fix. Not sure how I'd solve that.

If this happens, you can easily edit the pill by using the left arrow key to select the pill, then press Enter to enable edit mode, and now you can edit the pill address.
Anyway, this problem should be solved because even if you hit , accidentally, the pill won't be created because the string doesn't contain a valid email address.

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

• The fields are no more vertically aligned in columns because the text fields begins directly after the To: label.

The UI is not polished at all and I'll need to iterate on it a bit more to find a better overall solution.

When I DnD a valid address from sidebar the address is shown red first until I press the ENTER button or click in a other text field. A valid address should never be red and when dropping a address from the sidebar it should change immediately convert to a pill.

We found out that the patch was outdated, so these issues have been fixed in the most recent patch.

---

I have a little problem with character encoding. When the pill gets created from an address with special characters (eg. ö) those characters turn into a question mark.
That's pretty odd as I'm simply storing those values into a label inside a custom element.
Does anyone know if custom elements need an encoding attribute, or the UTF-8 charset needs to be specified somewhere?

I'm attaching a screenshot of the issue right after this message.

Thanks everyone for the feedback and the help.

Comment 144

[Alessandro Castellani [:aleca]]

Attachment: wrong-charset.png

Here's the outcome of an address saved into a label value.

Comment 145

[Óvári]

Please also check with these 4 characters: ŐőŰű

https://www.compart.com/en/unicode/U+0150
https://www.compart.com/en/unicode/U+0151
https://www.compart.com/en/unicode/U+0170
https://www.compart.com/en/unicode/U+0171

For an test email address, please try:
ÁáÉéÍíÓóÖöŐőÚúŰű, Testing <ÁáÉéÍíÓóÖöŐőÚúŰű@example.com>

What do you think?

Thank you

Comment 146

[Magnus Melin [:mkmelin]]

Comment on attachment 9110415 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9110415 [details] [diff] [review]:
-----------------------------------------------------------------

Skimmed through this, some comments below:

::: mail/base/content/mailWidgets.js
@@ +1789,5 @@
> +    }
> +
> +    get originalInput() {
> +      return document.getElementById(this.getAttribute("originalInput"));
> +    }

The originalInput is really confusing. Maybe it just needs a better name and documentation. It's just the base name?

@@ +1882,5 @@
> +      this.addEventListener("dblclick", event => {
> +        this.startEditing(event);
> +      });
> +
> +      this.addEventListener("keypress", event => {

a lot of cases under this. a switch would be more readable

@@ +1948,5 @@
> +
> +      this.emailInput.addEventListener("keypress", event => {
> +        setTimeout(() => {
> +          this._finishEditing(event.key);
> +        }, 1);

you can leave out the 1ms

@@ +2123,5 @@
> +      this.focus();
> +    }
> +  }
> +
> +  customElements.define("mail-address", MozMailAddress);

maybe MailAddressPill + mail-address-pill would be better

::: mail/components/compose/content/MsgComposeCommands.js
@@ +3486,5 @@
>  /**
> + * Create the recipient label to add in the messenger compose dialog.
> + *
> + * @param  labelID     The unique identifier of the email header
> + * @return label       The newly create XUL label

Please for all the documentation use the proper JSDoc style, so:
``` 
 * @param {string} labelID - The unique identifier of the email header.
 * @return {string} The newly create XUL label.
```

@@ +3492,5 @@
> +function createRecipientLabel(labelID) {
> +  let label = document.createXULElement("label");
> +  label.setAttribute("id", labelID);
> +  label.setAttribute("value", `${labelID}:`);
> +  label.setAttribute("onclick", `showAddressRow(this, 'addressRow${labelID}')`);

add an event listener instaed

@@ +3497,5 @@
> +  label.setAttribute(
> +    "onkeypress",
> +    `showAddressRowKeyPress(event, this, 'addressRow${labelID}')`
> +  );
> +  label.setAttribute("inputelement", `${labelID}AddrInput`);

We should avoid putting random attributes on standard elements like this. Is there a better way to handle this?

@@ +6312,5 @@
>    // Collapsing the menuitem only prevents it getting chosen, it does not
>    // affect the menulist widget display when Newsgroup is already selected.
>    for (let item of newsTypes) {
>      item.collapsed = hideNews;
>    }

please remove addrWidget and change this all to

for (let item of document.querySelectorAll("#addr_newsgroups,#addr_followup") {
  item.collapsed = hideNews;
}

@@ +6341,5 @@
> +      "#bccAddrInput",
> +      "#replyAddrInput",
> +      "#newsgroupsAddrInput",
> +      "#followupAddrInput",
> +    ]);

looks wrong, querySelectorAll takes a string not an array.

I'd inline the inputs variable

@@ +6545,5 @@
> +    "#newsgroupsAddrInput",
> +    "#followupAddrInput",
> +  ]);
> +
> +  for (let input of inputs) {

same

::: mail/components/compose/content/addressingWidgetOverlay.js
@@ +189,4 @@
>    }
> +
> +  let container = element.parentNode.parentNode;
> +  let pills = container.querySelectorAll("mail-address");

inline

::: mail/locales/en-US/chrome/messenger/messengercompose/composeMsgs.properties
@@ +480,5 @@
>  ## Identity matching warning notification bar.
>  ## LOCALIZATION NOTE(identityWarning): %S will be replaced with the identity name.
>  identityWarning=A unique identity matching the From address was not found. The message will be sent using the current From field and settings from identity %S.
> +
> +confirmRemoveRecipientRowTitle=Confirm Row Removal

Remove <fieldname>

@@ +481,5 @@
>  ## LOCALIZATION NOTE(identityWarning): %S will be replaced with the identity name.
>  identityWarning=A unique identity matching the From address was not found. The message will be sent using the current From field and settings from identity %S.
> +
> +confirmRemoveRecipientRowTitle=Confirm Row Removal
> +confirmRemoveRecipientRowBody=All the addresses written in this field will be removed. Do you want to proceed?

Should say which field

Comment 147

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

All right, here's an updated patch with everything that was highlighted by Magnus.
I also fixed the wrong charset when dealing with special characters.

The originalInput is really confusing. Maybe it just needs a better name and documentation. It's just the base name?

Yeah, I'm not happy with the name either.
That is the ID of the autocomplete html:input where the pill was generated from. I need that to ensure keyboard navigation and focusing on it if all the pills are delete.
Which name would make more sense? I added a comment but I'm not sure it's clear.

We should avoid putting random attributes on standard elements like this. Is there a better way to handle this?

True, sorry about that. I ended up using the control attribute to reference the html:input. I didn't need a custom attribute at all.

Onto fixing all the failing tests.

Comment 148

[Jorg K (CEST = GMT+2)]

Had to interrupt my holiday to check this out. Looking good :-)

Comment 149

[Jorg K (CEST = GMT+2)]

Oops, spoke to early. Try this: Create a Cc field. Summon the addressing side bar or open an address book. Try dragging an address to the Cc field :-( - I don't want to mention that this malfunction could be quite fatal when a Bcc address is added to the To field.

Comment 150

[Alessandro Castellani [:aleca]]

(In reply to Jorg K (GMT+1) (PTO to 1st Dec 2019, not reading bugmail) from comment #149)

Oops, spoke to early. Try this: Create a Cc field. Summon the addressing side bar or open an address book. Try dragging an address to the Cc field :-( - I don't want to mention that this malfunction could be quite fatal when a Bcc address is added to the To field.

Ah, I see why this is happening.
Most likely you didn't drop the address exactly on the label, which it doesn't grow 100% of the column's width, and by default if a drop area is not recognized it falls back to the To field.

So, it's a matter of CSS and ensuring the the entire address row is accepted as a correct drop area, and not just the label or the input field.
Thanks for testing it and reporting this.

Comment 151

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

I started tackling the majority of the failing tests and I'm moving forward towards a green try run.

While updating tests, I was able to nail down some quirks and unexpected wrong behaviours, hopefully with the remaining tests to fix, I will lower the number of potential regressions to a manageable amount.

Here's a fresh try-build for anyone interested in testing it: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=16ba5865ee16850d0f618d7e49c900a83ac6b126

Comment 152

[Jorg K (CEST = GMT+2)]

I was able to nail down some quirks and unexpected wrong behaviours

Including fixing comment #149? I was dragging onto the field, not the label, BTW.

Comment 153

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

This is ready for a full review as all the tests on Linux have been fixed.
I've also been using daily with this patch applied all day to send emails and I didn't stumble upon any issue...so far.

Here's the try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=37715c9cc02dcb65deeb5c40b1f40a728a9c6a94

There are a couple of tests failing on macos, which I will fix tomorrow since I have to update the UI and dark theme variation for that platform.

Comment 154

[Alessandro Castellani [:aleca]]

Attachment: 440377-ui-solution1.patch

Here's a patch to apply on top of the original one to implement a slicker and more refined UI.
This is good to test only on Linux for now as I will be tackling macos issues tomorrow.

Comment 155

[Jorg K (CEST = GMT+2)]

Repeating: Have you addressed comment #149? (see comment #152).

Comment 156

[Alessandro Castellani [:aleca]]

(In reply to Jorg K (GMT+1) (PTO to 1st Dec 2019, not reading bugmail) from comment #155)

Repeating: Have you addressed comment #149? (see comment #152).

Yes. Sorry, I meant to answer your question in my previous comment but I forgot.
I tested it on Linux and macos and it works. Now, it shouldn't matter where you drop the address on the entire row, the field should always be the expected one.

Comment 157

[Richard Marti (:Paenglab)]

Comment on attachment 9111839 [details] [diff] [review]
440377-ui-solution1.patch

Only f+ because it's Linux only yet. Looks good, thanks.
I write more in the next comment with a screenshot.

Comment 158

[Richard Marti (:Paenglab)]

Attachment: linux-UI-27.11.png

Issues I found, not fatal and some could be done in follow-up patches/bugs.

• I think, the drop-shadow of the boxes is too dark. First when I saw it, I thought there must be a popup below that doesn't open correctly ( I was in the "From" menulist). With dark theme the drop-shadow is very hardly visible.
• It seems depending on the recipient pill's length it calculates if a second pill will match on the same line. If not it expands already the text box to a second line (see screenshot Bcc field) which looks a bit weird when there is still enough space to enter an address. I think, it should only expand when the cursor touches the box on the end.
• It would be great when we have a feedback, like the mouse hover feedback, when I try to drop an address on the "Add Cc", "Add Bcc" etc. buttons.
• The text boxes grow by 1px when the first address converts to a pill (no pill in box, 29px, with pill 30.33px).
• Dragging the pills to re-order/move to an other box isn't possible. For me okay for a follow-up bug.
• Also for a follow-up bug: maybe add a "Edit Contact" menu item like we have on the message header address.

Comment 159

[Paul Morris [:pmorris]]

Comment on attachment 9111839 [details] [diff] [review]
440377-ui-solution1.patch

Review of attachment 9111839 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, and I like the new UI refinements.  I'm in agreement with Richard's points and also found the drop down shadows to be too prominent.  Otherwise it appears to work well.

However, I noticed one problem that I think should be addressed before landing this.  I cut and pasted a large number of comma separated addresses, say 50-100, into the To field and then hit enter to turn them into pills.  The field expanded vertically for all the pills to fit, and that pushed the rest of the UI off the bottom of the window, and there was no way to scroll to bring it back into view.

Comment 160

[Alessandro Castellani [:aleca]]

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

Thanks for the feedback, I'll fix what you reported.
Here some thoughts on what you suggested.

It seems depending on the recipient pill's length it calculates if a second pill will match on the same line. If not it expands already the text box to a second line (see screenshot Bcc field) which looks a bit weird when there is still enough space to enter an address. I think, it should only expand when the cursor touches the box on the end.

That's an issue Paul pointed out before, and I agree that it looks awful. The problem is that I need a min-width for the input field in order to have a decently sized autocomplete popup. If I let the main input field shrink to fit inline with previous pills, the autocomplete popup will be too small since it inherits the width from the input field.
I'll see if I can force the popup to use the entire row container (which is styled to look like an input field but is not) as a width reference.

It would be great when we have a feedback, like the mouse hover feedback, when I try to drop an address on the "Add Cc", "Add Bcc" etc. buttons.
Dragging the pills to re-order/move to an other box isn't possible. For me okay for a follow-up bug.

Yeah. Both these edits will come in a follow up bug where I'll focus only on implementing a proper Drag& Drop experience.

(In reply to Paul Morris [:pmorris] from comment #159)

I cut and pasted a large number of comma separated addresses, say 50-100, into the To field and then hit enter to turn them into pills.
The field expanded vertically for all the pills to fit, and that pushed the rest of the UI off the bottom of the window, and there was no way to scroll to bring it back into view.

Uh oh, that's not supposed to happen, and it's definitely a bug.
I have a check in place to see if the header area grows taller than 2/3 of the window height whenever a pill is created, and if it does it should stop growing and add overflow scroll.
I'll fix that.

Comment 161

[Richard Marti (:Paenglab)]

Comment on attachment 9111838 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9111838 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/themes/linux/mail/compose/messengercompose.css
@@ +310,5 @@
>  
> +#identityLabel-box,
> +#subjectLabel-box,
> +.address-label-container {
> +  width: 8em;

Instead of hard coding the width here, you should probably do it like before with style="&headersSpace.style;" to make the width localizable.

Comment 162

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

Here's another patch with a fix for the issue reported by Paul.
The try run is looking pretty green, except for a couple of failures I'm not sure how to deal with: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=d13eaf801f2df3b4d15e51bd0e1b3aaffe0a7055

X3 failure: security/manager/ssl/tests/unit/test_intermediate_preloads.js
How can I run this locally and debug it? It's part of m-c.

Z4 failure: That's a whole crash of the app happening on macos only. But when I test it locally I can't seem to recreate the issue and all the tests pass. Any advice?

Comment 163

[Magnus Melin [:mkmelin]]

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

X3 failure: security/manager/ssl/tests/unit/test_intermediate_preloads.js
How can I run this locally and debug it? It's part of m-c.>

./mach test security/manager/ssl/tests/unit/test_intermediate_preloads.js

Probably not related though

Z4 failure: That's a whole crash of the app happening on macos only. But when I test it locally I can't seem to recreate the issue and all the tests pass. Any advice?

Probably not from your patch.

Comment 164

[Jorg K (CEST = GMT+2)]

X3 failure: security/manager/ssl/tests/unit/test_intermediate_preloads.js
Ignore. The person keeping the tree green should have filed a bug and fixed it. Started this morning, Thu, Nov 28, 06:06:07.

Comment 165

[Alessandro Castellani [:aleca]]

I see both X3 and Z4 happened in other try runs without my patch applied.
Thanks both for the info.

Comment 166

[Alessandro Castellani [:aleca]]

Attachment: 440377-ui-solution1.patch

And this is the updated UI patch with the following changes:

• Light and Dark theme styled for Linux and macos (Windows currently missing).
• Added shortcuts for macos.
• Prevent input field from showing an empty blank row.
• The autocomplete popup is attached to the row container and not the small input field.

I'm currently working on the other UI solution, which will come on a patch to apply on top of this.

Since the other UI variation only changes the location of the other recipient labels, I will not upload it until this one gets reviewed and approved in terms of colours and sizing.

Comment 167

[Richard Marti (:Paenglab)]

Comment on attachment 9112393 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9112393 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/addrbook/content/abMailListDialog.js
@@ +781,5 @@
> + * Delete recipient row (addressingWidgetItem) from UI.
> + *
> + * @param {<html:input>} inputElement  the recipient input element
> + *                                     (textbox-addressingWidget) whose parent
> + *                                     row (addressingWidgetItem) will be deleted.

There is no textbox-addressingWidget anymore. Is this parameter still valid?

Comment 168

[Richard Marti (:Paenglab)]

Comment on attachment 9112429 [details] [diff] [review]
440377-ui-solution1.patch

Yes, this is the way I think we should go.

Would it be somehow possible to give the "Add Cc:" etc. labels a "drop-on" attribute when I'm over them with a address? Then we could give the user a feedback if the address is on the right label to add.

Comment 169

[Richard Marti (:Paenglab)]

Attachment: 440377-ui-solution1.1.patch

Added the Windows styles. Also removed a lot of no more used code. I also tried to align vertically the labels to the text inside the fields.

I had to move some code from shared to the platform files to not to have to revert styles we don't need?

On Mac I added the box-shadow: var(--focus-ring-box-shadow); to the boxes to make it look more like the default focus on Mac.

Comment 170

[Magnus Melin [:mkmelin]]

Comment on attachment 9112393 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9112393 [details] [diff] [review]:
-----------------------------------------------------------------

Didn't test extensively yet. Below some code comments.
Autocomplete by adding comma + entering the next person doesn't work unless it's the first comma. I think we should also be consistent with commas and maybe add them between each recipient.

Leaving a pill in editing mode when you click outside works oddly. I know there were some use cases listed in comments above, but it's unexpected. And if you, say, move on to body - has it taken affect or not...?

::: mail/base/content/mailWidgets.js
@@ +1756,5 @@
> +
> +  class MailAddressPill extends MozXULElement {
> +    static get inheritedAttributes() {
> +      return {
> +        ".emaillabel": "crop,value=label",

since this is a a new element, instead of mapping value to label, we could just use label. I think that's clearer too

@@ +1783,5 @@
> +      this._setupEventListeners();
> +      this.initializeAttributeInheritance();
> +    }
> +
> +    // Set the ID of the autocomplete input field of the recipient row.

JSDoc style documentation for the function comments please

@@ +1830,5 @@
> +      return !this.emailInput.hasAttribute("hidden");
> +    }
> +
> +    get pills() {
> +      return this.parentNode.querySelectorAll("mail-address-pill");

I don't think this belongs here. An element is self-contained and shouldn't be allowed to look into the parent.

@@ +1834,5 @@
> +      return this.parentNode.querySelectorAll("mail-address-pill");
> +    }
> +
> +    get allPills() {
> +      return document.querySelectorAll("mail-address-pill");

.... and not into the whole document scope either ;)

@@ +1838,5 @@
> +      return document.querySelectorAll("mail-address-pill");
> +    }
> +
> +    get allSelectedPills() {
> +      return document.querySelectorAll(`mail-address-pill[selected]`);

same here

@@ +1841,5 @@
> +    get allSelectedPills() {
> +      return document.querySelectorAll(`mail-address-pill[selected]`);
> +    }
> +
> +    get fragment() {

There are some more efficient ways for this, like https://searchfox.org/mozilla-central/rev/073b138dcba41cd3f858522e5f0a9ee73e39afa0/browser/base/content/tabbrowser-tab.js#72

@@ +1881,5 @@
> +      this.addEventListener("dblclick", event => {
> +        this.startEditing(event);
> +      });
> +
> +      this.addEventListener("keypress", this._handleKeyPress);

seems you should inline _handleKeyPress

@@ +1885,5 @@
> +      this.addEventListener("keypress", this._handleKeyPress);
> +
> +      this.emailInput.addEventListener("keypress", event => {
> +        // We need the setTimeout to wait for the autocomplete to run its
> +        // original methods before using the correct value.

That's unfortunate. There's probably some better way. Maybe as a followup

@@ +1941,5 @@
> +          this.checkKeyboardSelected(event, this.nextSibling);
> +          break;
> +
> +        case "Home":
> +          this.parentNode.firstChild.focus();

the parentNode should be in charge of this functionality

::: mail/components/compose/content/MsgComposeCommands.js
@@ +4033,5 @@
> +      );
> +      let isMailingList =
> +        listNames.length > 0 &&
> +        MailServices.ab.mailListNameExists(listNames[0].name);
> +      let isNewsgroup = address.originalInput.id == "newsgroupsAddrInput";

followup is also for newsgroups. 
I don't think we should be looking at ids here. Can you add some suitable class instead, like "nntp-input"?

@@ +6332,5 @@
>    }
>    // If there is no News (NNTP) account existing then
>    // hide the Newsgroup and Followup-To recipient type in all the menulists.
> +  for (let item of document.querySelectorAll(
> +    "#addr_newsgroups,#addr_followup"

such a class would be useful here too

@@ +6359,5 @@
>      }
>  
> +    for (let input of document.querySelectorAll(
> +      "#toAddrInput,#ccAddrInput,#bccAddrInput,#replyAddrInput,#newsgroupsAddrInput,#followupAddrInput"
> +    )) {

and here

@@ +6587,5 @@
> +
> +/**
> + * Select all the pills in the recipient container if they exist.
> + *
> + * @param input      The autocomplete input field

@paran {HTMLElement} input - The autocomplete input field.

But is there something off here? Should it be selectRecipientPill? (without s). If not, how can you focus()?

@@ +7058,5 @@
>      document.getElementById("appcontent")
>    );
>  }
>  
> +function GetMsgToRecipientElement() {

maybe we can just get rid of this... there's no need for caching and all it does is look up the element by id which is super fast

::: mail/components/compose/content/addressingWidgetOverlay.js
@@ +47,5 @@
> +        .map(fullValue =>
> +          headerParser.makeMimeAddress(fullValue.name, fullValue.email)
> +        )
> +        .join(", ");
> +    } catch (ex) {

what problem are we expecting?

@@ +172,5 @@
>      return;
>    }
>  
> +  let element;
> +  let label = document.getElementById(recipientType);

you can move this down to where it's first used

@@ +264,5 @@
>    if (dragSession.isDataFlavorSupported("text/x-moz-address")) {
>      validFlavor = true;
>    }
>  
>    if (validFlavor) {

just use 
if (dragSession.isDataFlavorSupported("text/x-moz-address")) 


... and get rid of the validFlavor var

@@ +283,4 @@
>      dragSession.getData(trans, i);
> +    let dataObj = {};
> +    let bestFlavor = {};
> +    let len = {};

please drop the len, see bug 1600606

@@ +367,5 @@
> + * in the Message Compose window.
> + *
> + * @param {Event} event - The DOM keypress event
> + * @param {HTMLElement} element - The element that triggered the keypress event
> + * @param {string} parentID - The ID of the element's container

nit: add periods to these at the end of the sentence

@@ +532,5 @@
> +/**
> + * Activate the pill's EditMode from the context menu.
> + *
> + * @param {HTMLElement} element - The element from which the context menu was
> + *                                opened.

I think for cases like this, the nicer row wrapping is to just indent by two on the next line, like

@param
  opened.

::: mail/locales/en-US/chrome/messenger/messengercompose/composeMsgs.properties
@@ +484,5 @@
> +## Recipient pills fileds.
> +## LOCALIZATION NOTE(confirmRemoveRecipientRowTitle): %S will be replaced with the field name.
> +confirmRemoveRecipientRowTitle=Remove %S
> +## LOCALIZATION NOTE(confirmRemoveRecipientRowTitle): %S will be replaced with the field name.
> +confirmRemoveRecipientRowBody=All the addresses written in the %S field will be removed. Do you want to proceed?

remove the "written"?

::: mail/themes/shared/mail/messengercompose.css
@@ +514,5 @@
> +}
> +
> +.address-row.hidden {
> +  visibility: hidden;
> +  display: none !important;

hmm, both visibility and display?
and is the important neeeded?

Comment 171

[Alessandro Castellani [:aleca]]

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

Autocomplete by adding comma + entering the next person doesn't work unless it's the first comma. I think we should also be consistent with commas and maybe add them between each recipient.

Can you elaborate on this? Sorry, but I don't think I understood what problem you encountered.
The comma is currently handled in 2 scenarios:

• While typing, if you press comma, the test before gets converted into a pill.
• When pasting multiple addresses separated by a comma, those get converted into pills on Enter or blur.

Leaving a pill in editing mode when you click outside works oddly. I know
there were some use cases listed in comments above, but it's unexpected. And
if you, say, move on to body - has it taken affect or not...?

All right, too many people gave the same feedback on this, so I guess we can see how the users react on a pill exiting edit mode on blur.

::: mail/base/content/mailWidgets.js

  ".emaillabel": "crop,value=label",

since this is a a new element, instead of mapping value to label, we could just use label. I think that's clearer too.

Could you point me to an example? I don't think I got what you mean, sorry.

I don't think this belongs here. An element is self-contained and shouldn't be allowed to look into the parent.
...
same here

I moved the get pills(), get allPills(), and get allSelectedPills() methods to the AddressingWidgetOverlay.js file

@paran {HTMLElement} input - The autocomplete input field.

But is there something off here? Should it be selectRecipientPill? (without
s). If not, how can you focus()?

When the focus is on the html:input and the user hits CTRL+A I'm moving the focus on the latest pill present in the container.
All the available pills in the same row get the selected attribute, but the focus should move on one pill in order to handle the mass cut, copy, and delete actions.

function GetMsgToRecipientElement() {

maybe we can just get rid of this... there's no need for caching and all it
does is look up the element by id which is super fast

The gMsgToRecipientElement variable is used a couple of times in the SwitchElementFocus() method to handle the focus via F6.
How would you suggest updating that method?

All the other points have been feedback.
I'm gonna merge the UI patch into this in order to have everything together into a single patch and avoid dealing with merging issues while updating the base patch.

Comment 172

[Alessandro Castellani [:aleca]]

Comment on attachment 9112673 [details] [diff] [review]
440377-ui-solution1.1.patch

Review of attachment 9112673 [details] [diff] [review]:
-----------------------------------------------------------------

Good stuff, thank you so much for taking care of this.
I'll merge this edits into the main patch so we have a solid and unique starting point to use for the UI solution 2.

Regarding your question:
"Would it be somehow possible to give the "Add Cc:" etc. labels a "drop-on" attribute when I'm over them with a address?"

For sure, I will try to implement something like that in the follow up patch to enable drag&drop of pills between fields.

Comment 173

[Magnus Melin [:mkmelin]]

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

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

Autocomplete by adding comma + entering the next person doesn't work unless it's the first comma. I think we should also be consistent with commas and maybe add them between each recipient.

Can you elaborate on this? Sorry, but I don't think I understood what problem you encountered.
The comma is currently handled in 2 scenarios:

• While typing, if you press comma, the test before gets converted into a pill.

It doesn't seem to work if you autocompleted, get the pill, and then add a comma (doesn't trigger autocomplete when you write then). Also for cases you come back to fill in more addresses.

All right, too many people gave the same feedback on this, so I guess we can see how the users react on a pill exiting edit mode on blur.

I think that would be preferable yes.

::: mail/base/content/mailWidgets.js

  ".emaillabel": "crop,value=label",

since this is a a new element, instead of mapping value to label, we could just use label. I think that's clearer too.

Could you point me to an example? I don't think I got what you mean, sorry.

For the above mapping it means you'd set pill.value="foo" to get the label to be foo. Seems more readable to just have it be pill.label="foo" to set the label. So the rule would be ".emaillabel": "crop,label",.

I don't think this belongs here. An element is self-contained and shouldn't be allowed to look into the parent.
...
same here

I moved the get pills(), get allPills(), and get allSelectedPills() methods to the AddressingWidgetOverlay.js file

@paran {HTMLElement} input - The autocomplete input field.

But is there something off here? Should it be selectRecipientPill? (without
s). If not, how can you focus()?

When the focus is on the html:input and the user hits CTRL+A I'm moving the focus on the latest pill present in the container.
All the available pills in the same row get the selected attribute, but the focus should move on one pill in order to handle the mass cut, copy, and delete actions.

function GetMsgToRecipientElement() {

maybe we can just get rid of this... there's no need for caching and all it
does is look up the element by id which is super fast

The gMsgToRecipientElement variable is used a couple of times in the SwitchElementFocus() method to handle the focus via F6.
How would you suggest updating that method?

Use document.getElementById to get it when needed.

On a more general note, the "Add Newsgroup" and "Add Followup-To" headers are now quite in your face, so to speak. They used to be much more hidden. I'm not sure what you have planned for them? The other UI alternatives you sent would have been nicer re that. As is, I think this will be confusing to many, who have no idea even what a newsgroup is.

For the pill styling, maybe they should be less rounded in the corners?

For newsgroups, the confirmation of removing the field does not show.

Comment 174

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

I implemented the full UI for solution 1 in this new patch, and fixed almost everything that was reported.
Here's a couple of follow ups:

It doesn't seem to work if you autocompleted, get the pill, and then add a comma (doesn't trigger autocomplete when you write then). Also for cases you come back to fill in more addresses.

Why would you type a comma after a pill is created? After an address is converted into a pill, it should be natural to just type a new address since the previously typed value was converted, and there's no need to write a comma between pills.
Sorry if I misunderstood what you wrote.

For the above mapping it means you'd set pill.value="foo" to get the label to be foo. Seems more readable to just have it be pill.label="foo" to set the label. So the rule would be ".emaillabel": "crop,label".

That's not really what I'm doing.
The pills structure is the following:

<pill>
  <label class="emaillabel"/>
  <html:input>
</pill>

So when I set pill.label="foo", which is what I'm doing when creating a new pill, the child <label> gets that attribute and adds it to its value:

<pill label="foo">
  <label class="emaillabel" value="foo"/>
  <html:input>
</pill>

Is this a wrong approach?

I removed the gMsgToRecipientElement variable. Do you want me to do the same for gMsgIdentityElement and gMsgSubjectElement since they're doing the same thing?

On a more general note, the "Add Newsgroup" and "Add Followup-To" headers are now quite in your face, so to speak. They used to be much more hidden. I'm not sure what you have planned for them? The other UI alternatives you sent would have been nicer re that. As is, I think this will be confusing to many, who have no idea even what a newsgroup is.

This patch comes with the proposed solution 1 applied.
I'm now working on a dedicated patch for solution 2, with the labels inline the To field so we can test both by simply apply the secondary patch on top of this.

I know you're not a fan of this solution, but I wanted to avoid working on solution 2 before the overall code and base structure was close to a somewhat final state. :)

For the pill styling, maybe they should be less rounded in the corners?

I recommend to leave it like that. The rounded border style is a pretty common solution in other email clients, and the fact that it's not used anywhere else in the software it makes the pill a more recognizable element and unique to an email address.

For newsgroups, the confirmation of removing the field does not show.

Fixed.

Comment 175

[Alessandro Castellani [:aleca]]

Attachment: 440377-ui-solution2.patch

And here's the patch with solution 2 for the UI.
This patch puts the most common extra recipients inline with the To container, and collects extra recipients inside a popup panel.

Richard, let me know if I messed something up, especially on Windows.
Cheers

Comment 176

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

I had to rebase the patch and fix various merging issues with the latest updates that landed on trunk.

Comment 177

[Alessandro Castellani [:aleca]]

Attachment: 440377-ui-solution2.patch

And also this one is rebased.
Here's also a try run, let's see how this one goes: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=0c8944c58d1c7f43fbd6299ae93dec5f66ff6805

Comment 178

[Richard Marti (:Paenglab)]

Comment on attachment 9113385 [details] [diff] [review]
440377-recipients-pills.patch

Looks good to me.

Comment 179

[Richard Marti (:Paenglab)]

Comment on attachment 9113386 [details] [diff] [review]
440377-ui-solution2.patch

Review of attachment 9113386 [details] [diff] [review]:
-----------------------------------------------------------------

This works for me too.
I'm not a big fan of this because when a address while dropping isn't correctly placed over the Cc, Bcc buttons, the address is added to the To field. Solution 1 hasn't this issue. Also when a user uses full screen window on a wide screen, the additional recipient buttons are far away.
And the recipient types in the overflow arrow aren't direct drop targets, two additional mouse clicks are needed to add the field.
The `extraRecipientsPanel` could get a --arrowpanel-padding: 4px; like we have for the `reorderAttachmentsPanel`.

I give ui-r+ because it works like the solution 2 proposal but I'm still in favour of solution 1.

::: mail/themes/shared/mail/messengercompose.css
@@ +632,5 @@
>  
>  .address-extra-recipients {
> +  margin-block: 4px;
> +  margin-inline-start: 4px;
> +	align-self: end;

Here a tab slipped in.

Comment 180

[Magnus Melin [:mkmelin]]

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

Why would you type a comma after a pill is created?

I'd like the commas to be visible. Why you'd write it - well if you know commas separate emails, it's a natural thing to do so it shouldn't send you off to an error mode.

Is this a wrong approach?

Ah, I misread. Please ignore.

I removed the gMsgToRecipientElement variable. Do you want me to do the same for gMsgIdentityElement and gMsgSubjectElement since they're doing the same thing?

Sure, but feel free to do it in a followup.

Comment 181

[Magnus Melin [:mkmelin]]

Comment on attachment 9113385 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9113385 [details] [diff] [review]:
-----------------------------------------------------------------

This thing is huge... so not easy to review. Some smaller and larger things yet to address, see below.

* Regarding the commas, I think we should show them between addresses (but not in red like now). This makes it more clear, and also allows for more advanced addressing in the future, like group syntax.
* select all + copy: please make the pasted addresses separated by ", " and not just ", "
* newsgroups/followup-to fields should not autocomplete to news addresses, not to email addresses (originally implemented in bug 61491)

::: mail/base/content/mailWidgets.js
@@ +1754,5 @@
>    customElements.define("attachment-list", MozAttachmentlist, {
>      extends: "richlistbox",
>    });
> +
> +  class MailAddressPill extends MozXULElement {

please add JsDoc documentation for the class.

@@ +1774,5 @@
> +      this.emailLabel = document.createXULElement("label");
> +      this.emailLabel.classList.add("emaillabel");
> +
> +      this.emailDeleteImage = document.createXULElement("image");
> +      this.emailDeleteImage.classList.add("emailDelete");

nit: would be nicer with css classes all-lowercase-with-dash instead, like delete-pill-icon (it's also for newsgroups, so not necessarily an email)

@@ +1928,5 @@
> +          this.startEditing(event);
> +          break;
> +
> +        case "Delete":
> +          this.removePills(this.nextSibling);

these would probably want to be nextElementSibling and such?

@@ +1999,5 @@
> +
> +    clearSelected() {
> +      for (let pill of getAllPills()) {
> +        pill.removeAttribute("selected");
> +      }

You moved the getAllPills out of the pill class, but you still have the implementation acting on more than this pill inside the class, which is wrong. Maybe there needs to be a mail-pills container class for this functionality?

@@ +2033,5 @@
> +        this.clearSelected();
> +      }
> +    }
> +
> +    removePills(element) {

please document

@@ +2066,5 @@
> +        return;
> +      }
> +
> +      for (let pill of getAllPills()) {
> +        pill.finishEditing("Escape");

might be nicer to make the key be optional

@@ +2137,5 @@
> +      if (!isValid && !isMailingList && !isNewsgroup) {
> +        this.classList.add("error");
> +      } else {
> +        this.classList.remove("error");
> +      }

for this and below, it would be shorter with toggle

this.classList.toggle("error", !isValid && !isMailingList && !isNewsgroup)

::: mail/components/compose/content/MsgComposeCommands.js
@@ +2321,5 @@
> +
> +  // Wait for the autocomplete input to attach all the controllers.
> +  setTimeout(() => {
> +    CompFields2Recipients(gMsgCompose.compFields);
> +  });

This is not really appropriate, and will likely cause non-determinism => problems in tests at least

@@ +3387,5 @@
> +      let addressingWidgetLabels = document.getElementById(
> +        "addressingWidgetLabels"
> +      );
> +      let recipientsContainer = document.getElementById("recipientsContainer");
> +      let other_headers_Array = other_headers.split(",");

can we change the variable snake namings while touching this

@@ +3393,1 @@
>        for (let i = 0; i < other_headers_Array.length; i++) {

just do 
for (let header of other_headers.split(",")) {

}

@@ +3493,5 @@
> +function createRecipientLabel(labelID) {
> +  let label = document.createXULElement("label");
> +  let addLabel = getComposeBundle().getString("recipientAddLabel");
> +  label.setAttribute("id", labelID);
> +  label.textContent = ` ${addLabel} ${labelID}`;

please use appropriate l10n functionality to achieve this, per my previous review comment

@@ +3525,5 @@
> +  row.setAttribute("id", `addressRow${labelID}`);
> +  row.classList.add("addressingWidgetItem", "address-row", "hidden");
> +
> +  let firstCol = document.createXULElement("hbox");
> +  firstCol.setAttribute("align", "top");

top shouldn't be used anymore, see bug 1599548. should be "start" now. 
you have some more of this later

@@ +3992,5 @@
> + */
> +function focusAddressInput(event) {
> +  let container = event.originalTarget;
> +  if (container.classList.contains("address-container")) {
> +    let input = container.lastChild;

please make sure to use the Element versions, like lastElementChild
Not using that will cause problems when in an html document (where space nodes matter)

@@ +4026,5 @@
> +    if (!parent) {
> +      continue;
> +    }
> +
> +    let addresses = parent.querySelectorAll(".address-pill");

could just inline addresses

::: mail/components/compose/content/addressingWidgetOverlay.js
@@ +36,5 @@
>  
> +  for (let pill of document.getElementsByTagName("mail-address-pill")) {
> +    let fieldValue = pill.fullAddress;
> +    let recipientType = pill.getAttribute("recipienttype");
> +    let recipient = null;

move down two lines to where you first use it

@@ +440,5 @@
> +  pill.label = address.toString();
> +  pill.emailAddress = address.email || "";
> +  pill.fullAddress = address.toString();
> +  pill.displayName = address.name || "";
> +  pill.originalInput = element.id;

I'd suggest not having the originalInput. Looks like you could do without: store the aria-labelled-by + whether it's a newsgroup and whatever you need right here.

@@ +528,5 @@
> +  container.removeAttribute("focused");
> +}
> +
> +/**
> + * Activate the pill's EditMode from the context menu.

edit mode?

@@ +539,5 @@
> +  if (element.tagName == "mail-address-pill") {
> +    element.startEditing(event);
> +  } else {
> +    editAddressPill(element.parentNode, event);
> +  }

whops. I thing you want 
let pill = element.closest("mail-address-pill");

@@ +551,5 @@
> + */
> +function copyEmailNewsAddress(element) {
> +  if (!element) {
> +    return;
> +  }

wouldn't thing we run into that.

@@ +553,5 @@
> +  if (!element) {
> +    return;
> +  }
> +
> +  if (element.tagName == "mail-address-pill") {

here too, just look up closest and don't recurse

@@ +584,5 @@
> + * Delete the selected pill/pills.
> + *
> + * @param {XULElement} element - The element from which the context menu was
> + *   opened.
> + * @param {Event} event - The DOM event.

no such thing here

@@ +589,5 @@
> + */
> +function deleteAddressPill(element) {
> +  if (!element) {
> +    return;
> +  }

I would expect we never get there do we?

@@ +591,5 @@
> +  if (!element) {
> +    return;
> +  }
> +
> +  if (element.tagName == "mail-address-pill") {

closest

@@ +644,5 @@
> + * @param {XULelement} element - The clicked label.
> + * @param {string} labelID - The ID of the label to show.
> + */
> +function hideAddressRow(element, labelID) {
> +  let container = element.parentNode.parentNode;

for clarity it might be easier to use closest() here, matching on some class

@@ +645,5 @@
> + * @param {string} labelID - The ID of the label to show.
> + */
> +function hideAddressRow(element, labelID) {
> +  let container = element.parentNode.parentNode;
> +  let pills = container.querySelectorAll("mail-address-pill");

please move down to where it's first used

@@ +671,5 @@
> +  }
> +
> +  // Reset the original input.
> +  let input = container.querySelector(`input[is="autocomplete-input"]`);
> +  input.textValue = "";

for the inputs, you want input.value ?

@@ +706,5 @@
> + *
> + * @param {XULElement} pill - The mail-address-pill element.
> + */
> +function setFocusOnFirstPill(pill) {
> +  pill.parentNode.firstChild.focus();

firstElementChild

::: mail/components/compose/content/messengercompose.xul
@@ +759,5 @@
> +            class="menuitem-iconic"/>
> +  <menuitem id="menu_delete" label="&deleteCmd.label;"
> +            accesskey="&deleteCmd.accesskey;"
> +            oncommand="deleteAddressPill(document.popupNode)"
> +            class="menuitem-iconic"/>

the context menus typically don't have the menuitem-iconc, so I guess we should skip it here too

::: mail/locales/en-US/chrome/messenger/messengercompose/messengercompose.dtd
@@ +258,5 @@
>  <!--LOCALIZATION NOTE headersSpace.style is for aligning  the From:, To: and
>      Subject: rows. It should be larger than the largest Header label  -->
> +<!ENTITY headersSpace.style "width: 8em;">
> +<!ENTITY recipientAdd.label "Add">
> +<!ENTITY fromAddr.label "From">

you can't change the localization value without changing the key, goes for all the blow too.

::: mailnews/addrbook/content/abMailListDialog.js
@@ +481,5 @@
> + * Returns the recipient type popup for a row.
> + *
> + * @param row  Index of the recipient row to return. Starts at 1.
> + * @return     This returns the menulist (not its child menupopup), despite the function name.
> + */

please remove the extra spaces after return

Comment 182

[Magnus Melin [:mkmelin]]

Comment on attachment 9113386 [details] [diff] [review]
440377-ui-solution2.patch

Review of attachment 9113386 [details] [diff] [review]:
-----------------------------------------------------------------

I do like this better (didn't look much at the code yet)

::: mail/locales/en-US/chrome/messenger/messengercompose/messengercompose.dtd
@@ +257,5 @@
>  <!-- Headers -->
>  <!--LOCALIZATION NOTE headersSpace.style is for aligning  the From:, To: and
>      Subject: rows. It should be larger than the largest Header label  -->
>  <!ENTITY headersSpace.style "width: 8em;">
> +<!ENTITY extraRecipients.tooltip "More recipients">

"More addressing fields"

Comment 183

[Thomas D. (:thomas8)]

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

Created attachment 9113340 [details] [diff] [review]
440377-recipients-pills.patch
I implemented the full UI for solution 1 in this new patch, and fixed almost everything that was reported.

This sounds like an interesting candidate for a try build to test UI1. I guess testing outdated try builds wouldn't help. Not sure how hard it is to create try jobs, but regular try builds for this sensitive change to the traditional recipients area would be very useful and appreciated.

Comment 184

[Alessandro Castellani [:aleca]]

(In reply to Thomas D. from comment #183)

This sounds like an interesting candidate for a try build to test UI1. I guess testing outdated try builds wouldn't help. Not sure how hard it is to create try jobs, but regular try builds for this sensitive change to the traditional recipients area would be very useful and appreciated.

Sorry, I forgot to paste the link.
I actually run a full try build pretty much every day for these updates.

macos and Linux try build for solution 1: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=fffa69633898192ebc30dbc2646189db98c304ab

all platforms try build for solution 2: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=0c8944c58d1c7f43fbd6299ae93dec5f66ff6805

Comment 185

[Thomas D. (:thomas8)]

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

Created attachment 9113386 [details] [diff] [review]
440377-ui-solution2.patch
Here's also a try run, let's see how this one goes: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=0c8944c58d1c7f43fbd6299ae93dec5f66ff6805

So I've finally had a chance to test this out for the first time (looking at UI-2).
Impressive it is, a fine work, and hard labor; thanks a lot Alessandro! :-)

As a general caveat, I believe that we should not underestimate the impact of this change to existing users who have developed their workflows in the current "vertical" recipient UI since time immemorial. Which should be an impetus to be as convincing as possible with a fine-tuned, flawless, maximally convenient, and value-added UX.

Here's some remarks/issues of my first-impression ui-review (with a special focus on keyboard operation)...

1. Pls implement Shift+Movement keys for selecting multiple consecutive pills
2. Pls remove Ctrl+Movement keys for selecting multiple consecutive pills, and allow selection of non-consecutive pills instead. Pls try keyboard selection methods in windows explorer; let's be consistent! For Ctrl+Movement keys, the current selection must be preserved, and we need a focus ring (only!) when moving to the next pill, which can then be selected (and de-selected) with Space bar.
3. Pls distinguish the effects of hover vs. select. See windows explorer. Otherwise Ctrl+Left-click to select becomes a guessing game.
4. Hand cursor? Is it a link?
5. Per my comment 117, I still feel that: Double-click is non-standard and undiscoverable to edit the label of an object. Typically, single-click selects, another single-click edits, and double-click opens. Compare e.g. the behaviour of file name labels in Windows Explorer ;-)
6. For windows users, pls implement F2 to edit the label (which is the Windows default shortcut for editing texty things, cf. renaming files, editing Excel cells, etc.)
7. Behaviour inconsistency:

• Type [e-mail], press Enter -> get cursor to continue (correct). vs.
• Cursor onto existing pill {[e-mail]}, press Enter to edit, Re-Type, press Enter -> still stuck on the pill. This should return the cursor to continue, just as it did for the first round of typing.

8. Pressing Backspace or DEL on an empty pill in edit mode should remove the pill and just show cursor, similar to the current behaviour when pressing DEL or Backspace on an empty row (consider recipient rows a vertical precursory version of pills) - possible with event.repeat, see patch of Bug 1527547.
9. Cursoring onto a single pill pretends to select it, but doesn't. Please do select. Just like... drumroll... Windows Explorer. We really want to use Ctrl+X and Ctrl+C on that single pill.
10. Please allow me to enter |Johnson, Peter| even when I already have |Johnson, Jane| in my address book. Current: comma after a name which has a (single?) partial autocomplete match will create the pill prematurely: {Johnson}
11. Do we need a permanent delete button on CC and BCC rows? I think it's enough to show the delete button when the row (or the CC label) is hovered or focused. We've just de-cluttered the recipient area, and the general trend in UX is hiding what's not immediately needed in the given context.
12. Thou shalt not kill:

• have a couple of pills

• start typing another one:
  {pill-1} {pill-2} ... {pill-N} John D|

• notice that you want Jane Doe instead, hold Backspace to correct what you just typed...

  BOOM! All pills gone. :-(( Please don't. Backspace must break/stop after deleting the unfinished pill. Only after releasing and re-pressing the backspace key, that's when we want to start deleting other pills. Use event.repeat as above in my point 8, see patch of Bug 1527547.

So much for now. It's a lot of work. Again, much appreciated! :-)

Comment 186

[Thomas D. (:thomas8)]

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

macos and Linux try build for solution 1: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=fffa69633898192ebc30dbc2646189db98c304ab

Unfortunately not testable for Windows-only consumers like myself :/

Comment 187

[Alessandro Castellani [:aleca]]

Thanks Magnus for the review.
I fixed pretty much everything except the originalInput, which now I'm defining as a scoped variable in the pill CE constructor.
I tried not to have it at all, but a pill needs to know from which html:input element was originated from to get attributes and proper focus handling between elements, and I couldn't figure out a way to achieve that without bleeding into the document from within the pill CE.

Also, for the getAllPills and similar methods, I think we can have those into a parent CE which wraps the entire recipients area, but I'd like to tackle that in a follow up patch. Would that be OK?

Thomas, thanks for the detailed feedback, I applied pretty much all those improvements, which were minor thanks to the somewhat modular approach I tried to give to this thing.

A couple of notes:

Per my comment 117, I still feel that: Double-click is non-standard and undiscoverable to edit the label of an object. Typically, single-click selects, another single-click edits, and double-click opens.

I think at the pill as a closed element, that's why the double-click makes sense for me, since you need to "open" the element to access the input field within. I see how it can be seen a bit of a learning curve, but I'd like to land the patch with this approach, and gather feedback from users on daily.

Please allow me to enter |Johnson, Peter| even when I already have |Johnson, Jane| in my address book. Current: comma after a name which has a (single?) partial autocomplete match will create the pill prematurely: {Johnson}

That's a bit of an annoying issue with the autocomplete. I'll see if I can do something about it, otherwise we'll need to deal with this in a followup patch.

What's missing

• I'm trying to find a decent solution to all those setTimeout(), trying to avoid using them.
• Implement the event.repeat to prevent unwanted deletions.
• Fix the remaining couple of failing tests.

A new patch a new try run will come later tomorrow.

Side note
Many more improvements could be done to this patch, that's why for future additions and suggested edits, I will open dedicated follow up bugs I will deal with once this has landed.
This patch is already massive and pretty hard to review, and I'm sure a bunch of regressions will unfortunately come with it, so I think it's better to have something landing at a state we're pretty satisfied with in order to allow everyone to test it and keep iterating throughout smaller patches.

Cheers.

Comment 188

[Magnus Melin [:mkmelin]]

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

Also, for the getAllPills and similar methods, I think we can have those into a parent CE which wraps the entire recipients area, but I'd like to tackle that in a follow up patch. Would that be OK?

Sure, but please add comments in the code too about this, and reference the bug to fix it.

Comment 189

[Thomas D. (:thomas8)]

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

Thomas, thanks for the detailed feedback, I applied pretty much all those improvements

Awesome!!! Looking forward to your next Windows try build.

A couple of notes:

Per my comment 117, I still feel that: Double-click is non-standard and undiscoverable to edit the label of an object. Typically, single-click selects, another single-click edits, and double-click opens.

I think at the pill as a closed element, that's why the double-click makes sense for me, since you need to "open" the element to access the input field within. I see how it can be seen a bit of a learning curve, but I'd like to land the patch with this approach, and gather feedback from users on daily.

Agreed, thank you for sharing the rationale, now it makes sense. In fact, my postulate was inaccurate: double-click is default action (which may or may not be "open"), must match ENTER on keyboard, and our default action here is "Edit pill" - fair enough.

1. So now we have:

• Double-click on pill -> Edit pill
• Enter on selected pill -> Edit pill
  What if you just add this as a complementary method:
• Single-click on selected pill -> Edit pill (like mouse-renaming of files in Win Explorer)
  I'm suggesting this here (as opposed to followup bug) because I think it could significantly ease the transition from the current "editable text" recipients: Click once expecting to edit text -> pill selected (fail, but there must be a way!?). Stubbornly click again on selected pill -> pill in edit mode (success, even somewhat discoverable). In fact, that mimics the current behaviour exactly, as we're already auto-selecting recipients "pill-style" since my bug Bug 1527547. With the current iteration of the patch here, it may feel too hard/undisdoverable for users how to break that resistance of pills against being edited... ;-)

2. As a side note (for followup), here's some ideas how to enable "Edit (address book) Contact":

• Pill context menu: Edit Contact (vs. Edit Recipient)

• Alt+Enter / Ctrl+I on selected pill -> Edit Contact (default shortcuts for "Properties"; try it in TB's contacts side bar)

  I'm hesitating a bit because I'm not sure about the correlation between a recipient email address and contact card(s) in the AB, which is currently flawed in the ancient design of TB's address book. If the correlation can be made in a predictable way which does not hinder the new AB design, the above should be implemented. Oh, in a followup bug...

Comment 190

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

I may have solved the setTimeout issue, as I stated in bug 1601830, forcing the blur() seems to do the trick.
I cleaned up some methods and started writing more comments to properly describe what it's happening.

I'll finish writing comments to cover all the methods I introduced, and fix the newly implemented mochitests.

Try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=674bb73bd2b093deb8170dc8ecd7d67b5d1239a9

Comment 191

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

I made some progress on this thanks to the suggestion from bug 1601830.
I also fixed some mochitests and other random intermittent tests I stumbled upon, and commented pretty much everything.

Magnus, please take a look at what I did with the observer inputObserver and let me know if you think this is a correct implementation.

I'm not asking for a full review because c-c has been busted almost all afternoon and I couldn't run proper indepth tests.

Comment 192

[Jorg K (CEST = GMT+2)]

Attachment: postbox-pills.png

Can I suggest something terrible? Postbox has had this function for years. Maybe it wouldn't be totally absurd to check how their stuff is working.

Comment 193

[Thomas D. (:thomas8)]

(In reply to Jorg K (GMT+1) (PTO to 15th Dec 2019, sporadically reading bugmail) from comment #192)

Created attachment 9114666 [details]
postbox-pills.png
Can I suggest something terrible? Postbox has had this function for years. Maybe it wouldn't be totally absurd to check how their stuff is working.

Absolutely. Some interesting ideas there, like the dropdown context which reduces the delete button clutter at the cost of two-click recipient deletion. A bug which eats recipients upon shift-selection (shift-select last recipient, then shift+home). Incomplete selection experience (no Ctrl-selection at all). Alessandro's interim stage is already better than theirs.

Another point of comparison: gmail's webmail composition interface. Far from perfect, but nice reduced and calm layout of pills, and some good behaviour there.

Comment 194

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

I think this is ready for a full review, and at a stage where we can land it so I can start working on the follow up bugs to improve it.

Thanks to the "autocomplete-did-enter-text" event and the handleEnter() method, I was able to remove all those awful timeouts and event dispatching, as well as hijacking the enter keypress or the click event inside the popup.
Everything looks and feels more linear.

Can I suggest something terrible? Postbox has had this function for years. Maybe it wouldn't be totally absurd to check how their stuff is working.

For sure. I've actually been using and testing various apps to write down the various pros and cons of similar solutions, and Postbox is one of them.
This patch is more of an "initial working implementation" of the pill system, in order to have something complete and fully working to land, before continuing onto more thorough and detailed patches.

Thomas, thanks for filing those bugs, which I will be ignoring for now until this patch lands, but definitely a much needed overview of what needs to be done.

Here's the latest try-run with many improvements, and hopefully all the tests fixed: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=eef5e4aa573cad31df6382748f984d696b654752

If, for whatever reason, that try-run fails or it's busted, here's a fairly recent one with some small missing improvements, but still better than last week: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=dce9bf4efcc506e81a1404791a843f19a6cdfac7

Comment 195

[Magnus Melin [:mkmelin]]

Comment on attachment 9114788 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9114788 [details] [diff] [review]:
-----------------------------------------------------------------

comm/mail/test/browser/composition/browser_sendButton.js is failing on all platforms on the try run. But not locally it seems... :/

I think it's pretty close to be in land-able shape.

::: mail/base/content/mailWidgets.js
@@ +1773,5 @@
> +        return;
> +      }
> +
> +      // Used to store the html:input element from where the pill was generated.
> +      this.originalInput;

this doesn't do anything. move the documentation elsewhere?

@@ +1804,5 @@
> +
> +      Services.obs.addObserver(
> +        this.inputObserver,
> +        "autocomplete-did-enter-text"
> +      );

Observers and custom elements have a life cycle problem, so I think you need to set up a window unload event listener too, where you remove the observer. disconnectedCallback isn't called at that time, so otherwise we leak.

::: mail/test/mozmill/composition/test-draft-identity.js
@@ +256,5 @@
>          false
>        );
>      }
>  
> +    close_compose_window(cwc);

looks like a wrong change. we should still test and ensure there is no prompt, which is what the false did

::: mail/test/mozmill/composition/test-forwarded-eml-actions.js
@@ +144,5 @@
>          subjectText
>      );
>    }
>  
> +  close_compose_window(compWin);

same here

::: mail/test/mozmill/composition/test-save-changes-on-quit.js
@@ +234,5 @@
>    let msg = select_click_row(0);
>    assert_selected_and_displayed(mc, msg);
>  
>    let nwc = open_compose_new_mail();
> +  close_compose_window(nwc);

and these... here the sole reason for the test is to verify there were no prompts ;)

Comment 196

[Alessandro Castellani [:aleca]]

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

comm/mail/test/browser/composition/browser_sendButton.js is failing on all platforms on the try run. But not locally it seems... :/

Yeah, this is super annoying, but I'll get to the bottom of it.

Observers and custom elements have a life cycle problem, so I think you need to set up a window unload event listener too, where you remove the observer. disconnectedCallback isn't called at that time, so otherwise we leak.

Ah, I didn't know that, thanks for the heads up.
Does something like that look good?

// Remove the observer for each pill as the disconnectedCallback() might not
// be called on time and we might leak.
  for (let pill of getAllPills()) {
    Services.obs.removeObserver(
      pill.inputObserver,
      "autocomplete-did-enter-text"
    );
  }

Comment 197

[Magnus Melin [:mkmelin]]

Probably best to add it straight into the CE connectedCallback, like https://searchfox.org/comm-central/rev/065eb16ca361300b4b116f6c69ebd10ea1e6c1d0/mail/components/addrbook/content/menulist-addrbooks.js#97

Comment 198

[Magnus Melin [:mkmelin]]

To clarify, that should be for the window unload event, so that we make sure to kill off any observer references.

Comment 199

[Alessandro Castellani [:aleca]]

Sure, I added that into the connectedCallback() method of the pill CE.
I also have an observer for the main input field, which gets removed on windows unload, unrelated to the pills.

Comment 200

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

Patch updated with all tests passing on the recent try run, except for those intermittent failures which shouldn't be related to my patch.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=bec9f1c7f543c51307397ea1cafbfe321c2db83e

Comment 201

[Óvári]

If an pill (i.e. email address) has an associated photo, could the photo please show when the mouse hovers above the pill. Thank you

Comment 202

[Magnus Melin [:mkmelin]]

Comment on attachment 9115034 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9115034 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/content/mailWidgets.js
@@ +1807,5 @@
> +        "autocomplete-did-enter-text"
> +      );
> +
> +      // Remove the observer on window unload as the disconnectedCallback()
> +      // might not be called on time and we might leak.

Slightly wrong comment. disconnectedCallback will never be called when closing a window, so we might therefore leak if XPCOM isn't smart enough. This is not a problem for normal event listeners, they go away with the dom.

::: mail/locales/en-US/chrome/messenger/messengercompose/messengercompose.dtd
@@ +256,5 @@
>  
>  <!-- Headers -->
>  <!--LOCALIZATION NOTE headersSpace.style is for aligning  the From:, To: and
>      Subject: rows. It should be larger than the largest Header label  -->
> +<!ENTITY headersSpace.style "width: 8em;">

I think this needs a bump too (to be sure)

::: mail/test/browser/composition/browser_saveChangesOnQuit.js
@@ +232,5 @@
>    let msg = select_click_row(0);
>    assert_selected_and_displayed(mc, msg);
>  
>    let nwc = open_compose_new_mail();
> +  close_compose_window(nwc);

please add back the false argument, so it's properly checking there was no prompt

@@ +237,3 @@
>  
>    let rwc = open_compose_with_reply();
> +  close_compose_window(rwc);

same here

@@ +240,3 @@
>  
>    let fwc = open_compose_with_forward();
> +  close_compose_window(fwc);

and here 

.... and all the other cases where that got removed

::: mail/test/browser/composition/browser_sendButton.js
@@ +179,5 @@
>    check_send_commands_state(cwc, true);
>  
> +  // Clear the CC list.
> +  clear_recipient(cwc);
> +  // No other pill is there, Send should become disabled.

. not ,

Comment 203

[Alessandro Castellani [:aleca]]

Attachment: 440377-recipients-pills.patch

Updated, and here's the new try-run.

Comment 204

[Magnus Melin [:mkmelin]]

You forgot the link to try. Looks like it's this (which looks good): https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=e8fc86894b3288d4360b195fe7a952aec10302b9

Comment 205

[Magnus Melin [:mkmelin]]

Comment on attachment 9115307 [details] [diff] [review]
440377-recipients-pills.patch

Review of attachment 9115307 [details] [diff] [review]:
-----------------------------------------------------------------

I'll do the minor adjustments below, and go ahead with landing this. Nothing major lacking that we don't already know about and knowingly will defer to follow-ups.
r=mkmelin

::: mail/base/content/mailWidgets.js
@@ +1854,5 @@
> +    /**
> +     * Check if the pill is currently in "Edit Mode", meaning the label is
> +     * hidden and the html:input field is visible.
> +     *
> +     * @returns {bool} If the pill is currently beign edited.

{boolean}
And a typo: beign

::: mail/components/compose/content/addressingWidgetOverlay.js
@@ +17,2 @@
>  /**
> + * Convert all the written recipients into string and store them intro the

into

@@ +320,5 @@
> +/**
> + * Find the autocomplete input when an address is dropped in the compose header.
> + *
> + * @param {XULElement} target - The element where an address was dropped.
> + * @param {String} recipient - The email address dragged by the user.

In general strings should be lowercase {string}, unless it actually should be a String object. Not a very notable distinction though...

@@ +444,5 @@
> +/**
> + * Add a new "address-pill" to the parent recipient container.
> + *
> + * @param {HTMLElement} element - The element that triggered the keypress event.
> + * @param {Boolean} automatic - Set to true if the change of recipients was

boolean

Comment 206

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/019eaf0f9cf7
Implement new recipients address fields. r=mkmelin, ui-review=Paenglab

Comment 207

[ruediger]

1. No need for a static To:-Field in Newsgroups
2. Cursor stays at a reply in the To:-Field and not in the Body.
3. The whole change looks terrible and I see no advantage in it.
4. Please build in a Backdoor to the old functionality, or the possibility to customize it.
5. Thanks

Comment 208

[Magnus Melin [:mkmelin]]

While it's landed we know there are still some work to be done, so stay tuned.
Keeping the old version in parallel would be too large of an undertaking. There are at least a few features/bug fixes that will be much easier to do with this new UI but would have been more difficult to get right UI wise in the old system. E.g. bug 271405.

Comment 209

[Alessandro Castellani [:aleca]]

(In reply to ruediger.lahl from comment #207)

1. No need for a static To:-Field in Newsgroups

I will explore the possibility of hiding the "To" field if the "Newsgroup" field is revealed.

2. Cursor stays at a reply in the To:-Field and not in the Body.

Fixed in bug 1603526.

3. The whole change looks terrible and I see no advantage in it.

The current UI is not final and many more iterations and improvements are needed.
Can you elaborate on the "no advantage statement"?
We implemented this for these reasons:

• Ability to write multiple recipients in the same field.
• Cut, Copy, Paste, and move multiple recipients from a field to another with one action (eg. 100+ addresses from To to CC).
• More compact and streamlined UI (still a WIP).
• Simpler and cleaner code based on Custom Elements without node cloning and duplicated methods.
• Making life easier for developers.

4. Please build in a Backdoor to the old functionality, or the possibility to customize it.

Like Magnus said in comment 208.

5. Thanks

While we know we won't be able to satisfy everyone, I'd like to reiterate that we're implementing these changes to improve the usability and stability of the software. Simply saying "the old is good don't change it" is not enough.

Please, take a look at the multiple follow up bugs we created to continue working on this area, and feel free to provide constructive feedback and suggestions on how to make it better.

Comment 210

[ruediger]

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

(In reply to ruediger.lahl from comment #207)

1. No need for a static To:-Field in Newsgroups

I will explore the possibility of hiding the "To" field if the "Newsgroup" field is revealed.

TB can answer to 'Newsgroup' or 'Reply to Sender only' via Menu->Message. 'To Sender only' is no default in Newsgroups and should only be a hidden feature, even as 'Reply to all'.

2. Cursor stays at a reply in the To:-Field and not in the Body.

Fixed in bug 1603526.

Great.

3. The whole change looks terrible and I see no advantage in it.

The current UI is not final and many more iterations and improvements are needed.
Can you elaborate on the "no advantage statement"?
We implemented this for these reasons:

• Ability to write multiple recipients in the same field.

Was ever available with mail1@adress; mail2@adress; mail3@adress; mail3@adress

• Cut, Copy, Paste, and move multiple recipients from a field to another with one action (eg. 100+ addresses from To to CC).
  ctrl-a - ctrl x(v) works between the fields.

• More compact and streamlined UI (still a WIP).

Compact is a single Button that rolls out an Menu, like the old UI does. I have all under the Mouse if I need it, by clicking on [To:]

• Simpler and cleaner code based on Custom Elements without node cloning and duplicated methods.

I don't understand that. Custom Elements like additional Entries/Buttons can be generated by Entries in user.js like: user_pref("mail.compose.other.header", "Supersedes,Control,X-No-Archive");

• Making life easier for developers.

Okay.

4. Please build in a Backdoor to the old functionality, or the possibility to customize it.

Like Magnus said in comment 208.

The ability to customize should be given.

5. Thanks

While we know we won't be able to satisfy everyone, I'd like to reiterate that we're implementing these changes to improve the usability and stability of the software. Simply saying "the old is good don't change it" is not enough.

In this case the old is (IMHO) better then the new.

Please, take a look at the multiple follow up bugs we created to continue working on this area, and feel free to provide constructive feedback and suggestions on how to make it better.

Okay and thank you.

Comment 211

[Marco A.G.Pinto [:marcoagpinto]]

Attachment: "Reply to" appears in red

Hello @aleca,

Like mentioned on IRC yesterday, the "reply to" pill appears in red.

I am not sure if it is a bug or a feature.

See the screenshot.

Comment 212

[Alessandro Castellani [:aleca]]

Thanks for the feedback Marco.
That's not red but orange, which means the address you're using is not currently stored in your address book.

I know it looks confusing, but that's just temporary as a better UI is getting explored in the followup bug 1601748.

Comment 213

[Jaise]

[Feature request] In capsule itself, add an option toggle to, cc, bcc [like close button on end, in front To/CC/BCC] if available, it will be very nice.

Comment 214

[Jorg K (CEST = GMT+2)]

Hmm, I don't have the latest build, but in my local build of a few days old, there is not [x] to delete the pill. There's only a context menu to delete, cut, edit, etc. So what's the design to change a To to a CC? Cut and paste? Put that onto the context menu: "Move to CC"? What are the plans?

(Pasting a pill seems to paste raw text in red, and I have to hit enter to turn it into a pill. Sorry if the question is silly, I haven't followed the action closely in all related bugs, but that seems to be bug 1602456).

Comment 215

[Alessandro Castellani [:aleca]]

Hmm, I don't have the latest build, but in my local build of a few days old, there is not [x] to delete the pill. There's only a context menu to delete, cut, edit, etc. So what's the design to change a To to a CC? Cut and paste? Put that onto the context menu: "Move to CC"? What are the plans?

The [X] was removed to clear UI noise, prevent accidental address deletion while selecting a pill, and to behave more like a desktop app and not a web app.
The plans for the "move to another recipient" option are the following:

• Copy/Cut and past pills to another recipient row: bug 1602456
• Enable drag&drop of pills: bug 1601749

We could consider adding a a menu item into the context menu as you suggested, if that's the case of the request.
I'm against adding labels or extra items in the pills unless absolutely necessary as it creates a lot of noise and makes the keyboard accessibility a bit of a nightmare.

Comment 216

[Jorg K (CEST = GMT+2)]

OK, well, yes, adding it to the context menu might be useful.

Comment 217

[Alex ARNAUD]

(In reply to Jorg K (GMT+1) (PTO to 26th Jan 2020, sporadically reading bugmail) from comment #216)

OK, well, yes, adding it to the context menu might be useful.

Confirmed, could really be useful, as keyboard users like blind couldn't do drag and drop and copy/paste from one field to another one with the keyboard would be really practical.

Comment 218

[Jaise]

Attachment: mail addressconcept 1.jpg

Just a concept. First box area for selection[for example click box one turn blue fill. so in selection mode]. second area mail address. single click start edit. Third area options[cut/copy/move to To/move to To Cc/move to To BCC etc.].

Comment 219

[Jaise]

Attachment: mail address concept 2.jpg

Concept 2.
To/cc/Bcc will have only single input box. In front of address pill we can change.

Comment 220

[Alex ARNAUD]

(In reply to Jorg K (GMT+1) (PTO to 26th Jan 2020, sporadically reading bugmail) from comment #216)

OK, well, yes, adding it to the context menu might be useful.

Confirmed, could really be useful, as keyboard users like blind couldn't do drag and drop and copy/paste from one field to another one with the keyboard would be really practical.(In reply to Jaise from comment #218)

Created attachment 9121222 [details]
mail addressconcept 1.jpg

Just a concept. First box area for selection[for example click box one turn blue fill. so in selection mode]. second area mail address. single click start edit. Third area options[cut/copy/move to To/move to To Cc/move to To BCC etc.].

I imagine a simple stuff in the contextual-menu "move to". It's more consistent with what TB does in message.

Comment 221

[Jaise]

It is OK.

In my context, our organization have aged people [say 45-55]. so teach them again new method may be problem [say double click to edit, right click to get option etc]. At least if downward arrow is there, they see & click.

Comment 222

[Thomas D. (:thomas8)]

(In reply to Alex ARNAUD from comment #217)

(In reply to Jorg K from comment #216)

OK, well, yes, adding it to the context menu might be useful.

Confirmed, could really be useful, as keyboard users like blind couldn't do drag and drop and copy/paste from one field to another one with the keyboard would be really practical.

Alex Arnaud, we are still working on improving the keyboard experience, and I'm known for my focus on ensuring correct and efficient keyboard UX. Please note that at this early stage, we still have a number of related bugs (see bugs in the "Blocks" section of this bug). Right now, as a keyboard user (including the blind), you can already select pills using some of the known keyboard methods like space bar to select or deselect, Shift+cursor for contiguous selection, etc. Then you can use Ctrl+X to cut, tab to move focus into the next recipient type (two tab stops after bug 1602372), Ctrl+V to paste. I think that's acceptable, but it could be even easier from context menu. We'll explore doing the context menu "Move to..." in bug 1609647.

As we're still in the process of getting the code and behaviour right, details of ARIA requirements may not immediately get the attention they deserve. Please bear with us, but do feel free to file respective bugs if they haven't been filed yet.

Comment 223

[Thomas D. (:thomas8)]

(In reply to Jaise from comment #218)

Created attachment 9121222 [details]
mail addressconcept 1.jpg

Just a concept. First box area for selection[for example click box one turn blue fill. so in selection mode]. second area mail address. single click start edit. Third area options[cut/copy/move to To/move to To Cc/move to To BCC etc.].

Hi Jaise, thank you for sharing your ideas! (Longer discussions would probably need another platform).
I can see where you are coming from with your concept 1, which is one possible way of doing it, and certainly deserves consideration.
Concept 1 is trying to make 3 different actions available by single-click on a certain subsection of a pill:

• Select (click left corner of pill)
• Edit (click middle part of pill)
• Context menu (click dropdown arrow in right corner of pill)

On the other hand, there's a worthwhile objection from Alessandro, our UX lead (comment #215):

I'm against adding labels or extra items in the pills unless absolutely necessary as it creates a lot of noise and makes the keyboard accessibility a bit of a nightmare.

So I'm not sure if the benefits of your concept 1 are strong enough to justify the costs.

So far, we're a desktop application, so imo context menu is a well-known and efficient way of accessing contextual functionality.
I've previously advocated for the single-click-to-edit myself, but easy selection of pills looks like an even more important aspect. Consider that editing a pill has easy access with Enter (and F2 on Windows), and it may not be the most frequent action on pills.
Your concept 1 requires a lot of user attention for all use patterns because each pill is now divided into three different click targets, also making pills significantly bigger and more noisy.

Comment 224

[Thomas D. (:thomas8)]

Comment on attachment 9121223 [details]
mail address concept 2.jpg

(In reply to Jaise from comment #219)
> Created attachment 9121223 [details]
> mail address concept 2.jpg
> 
> Concept 2. 
> To/cc/Bcc will have only single input box. In front of address pill we can change.

It's probably better to file such concepts in their own dedicated bug / RFE and make that dependent on this bug to create the link.

Concept 2 is not feasible in terms of UX design (wontfix) as it duplicates and/or obfuscates recipient type selection by reintroducing the old and unsorted way of defining recipient types. One of the key benefits introduced by this bug is that recipients of different types are now sorted together into their own respective box, so that you can see e.g. all of your BCC recipients at a glance. The answer to this problem of transitioning from the legacy UX is bug 1609647, having a context menu entry to change the recipient flavor of each pill, which will move that pill into the right container.

Comment 226

[doug2]

Hard to follow all this "pill" stuff, but having problems in beta. First, want to enter multiple addresses on one line but need single character to complete current address and start next. Have tried several chars for this, but have not found one that works. Nothing in Beta Notes. Second, appears to be discussed above, not easy to move address to different line (was easy before). Third, installation of beta wiped out toolbar customization (may not be related to this "enhancement"). Fourth, the "Cc" etc, buttons to create the lines are in an odd place. Would like the option to just auto add the two lines and hide the buttons.

Comment 227

[Alessandro Castellani [:aleca]]

First, want to enter multiple addresses on one line but need single character to complete current address and start next. Have tried several chars for this, but have not found one that works.

I'm not sure I understand your problem.
If you start typing an address, does it prompt the autocomplete? Once the autocomplete gives you the right address, you can hit enter or comma and start typing a new address, which should prompt again the autocomplete.
Are you trying to enter multiple addresses all at once by pasting a string of emails?

not easy to move address to different line (was easy before)

You can cut and paste that address (or multiple addresses at once) from a row to another. Before it was pretty much impossible to do it quickly for more than 1 address at a time.
Drag & Drop functionality is getting introduced in a follow up bug.

installation of beta wiped out toolbar customization

Not related to this, check bug 1613965.

the "Cc" etc, buttons to create the lines are in an odd place.

What's odd about the positioning? That's quite a standard location similar to other email clients, and is very easy to reach with both mouse and keyboard shortcuts.

Would like the option to just auto add the two lines and hide the buttons.

That's a bit of an edge case that doesn't introduce any real benefit other than removing 1 click to show that line, and it's quiet odd to always need CC and BCC always visible for every email.

Comment 228

[doug2]

entering multiple addresses with return between now works in b2. Did not in b1.
"old" way with single address per line took 1 click to change from "to" to "cc", etc. cut & paste is awkward in comparison.
I have never seen buttons to the far right. Been using mailers 35-40 years. Unix, windows, multiple other systems.

Comment 229

[Matt]

I think I need to chime in here, there is already some push back in support over the pills and the inability to set a default BCC in particular. Many Thunderbird users curate newsletters etc for community organizations, from their church to the local football club. These people want to add all addresses to a BCC line by default for obvious reasons. Many do not send using a To: header at all, until they get SPAM classifications at least.

A couple of support topics
https://support.mozilla.org/en-US/questions/1278219
https://support.mozilla.org/en-US/questions/1278340

An alternative would be to allow an option to designate mailing lists to be set to add to a default send type To: CC: BCC: in the address book. But we have to consider these users, they are not an edge case in my opinion.

Comment 230

[Óvári]

(In reply to Matt from comment #229)

Many Thunderbird users curate newsletters etc for community organizations, from their church to the local football club. These people want to add all addresses to a BCC line by default for obvious reasons. Many do not send using a To: header at all, until they get SPAM classifications at least.

Adding all addresses to a BCC line may also give a SPAM classification. Newsletter are best sent using the Mail Merge add-on.

Should Mail Merge functionality be added to Thunderbird core?

Thank you

Comment 231

[Thomas D. (:thomas8)]

(In reply to doug2 from comment #228)

Hey doug2, thank you for your feedback, and sorry that you couldn't warm up yet with our new recipient area.

entering multiple addresses with return between now works in b2. Did not in b1.

We're still working on improving this feature, please stay tuned. We do appreciate that it's a change which requires a bit of adjustment from the existing legacy layout. We are striving to deliver a new user experience which we believe will ultimately be significantly better, more versatile, and more efficient than the status quo.

"old" way with single address per line took 1 click to change from "to" to "cc", etc.
cut & paste is awkward in comparison.

Well, it was two clicks, one to open dropdown, next to choose type. And if you wanted to change 50 existing recipients, that was 100 clicks - now that's truly akward if you ask me. Now you can easily change 100 recipients with 3 simple steps: Ctrl+A to select all of same type, Ctrl+X to cut, Ctrl+V to paste in the other row. I find that awesome. As Alessandro said, more methods like drag and drop are coming, and I am working with Alessandro to complete the mouse and keyboard selection experience. We also have bug 1609647 about offering a two-click way of changing recipient type from context menu of each recipient, which may ease the transition for your scenario.

I have never seen buttons to the far right. Been using mailers 35-40 years. Unix, windows, multiple other systems.

Well, for now we've placed them there after careful consideration of pros and cons. Every UI design decision has them. You can now create a CC recipient with a single click on an openly available button, as opposed to carefully picking from a covert dropdown. They are also currently prime members of the tab focus ring, easy to reach with keyboard as Alessandro pointed out. If we put them to the left, others will say, why clutter the prime side with secondary options. Every change is a challenge to some extent, but no change spells doom...

That said, please do not hesitate to file bugs for regressions, or specific proposals for improvement!

Comment 232

[Matt]

(In reply to Óvári from comment #230)

Adding all addresses to a BCC line may also give a SPAM classification. Newsletter are best sent using the Mail Merge add-on.

Despite what you saying being correct, it does not change the usage that people want to make of the program. I have spent the last 10 years trying to educate, but there are millions of them, and one of me. So let's just make a product that the users want To use in the way they want to use it. When it breaks because of spam rules we can explain that to them. It is much harder to explain why we are making it harder for them to conceal their newsletter recipients' addresses. They want to do that for privacy and EU regulations.

It is far easier to tell them spam rules are constantly changing and they got caught when it breaks, because it is not Thunderbird doing it.

Should Mail Merge functionality be added to Thunderbird core?

Certainly, it should be there, it should have been there since about version 2. But then MAPI should have been working for the last decade, but I can still not make it work.

Thunderbird reminds me of Lotus 123. Powerful and bare bones, requiring the user to build the caravan they work from on the chassis supplied by Mozilla. Microsoft always just supplied the lot and found most users used less than 20% of the features, but for the others, it was only a case of discovering them, not having to build their own.

Comment 233

[Thomas D. (:thomas8)]

(In reply to Matt from comment #229)

I think I need to chime in here, there is already some push back in support over the pills and the inability to set a default BCC in particular.

Thanks Matt! If you are talking about the per-identity auto-BCC feature from account settings, then the "inability to set a default BCC" is a myth. I've just tried it and it works just like before. So I'm not seeing how the new design affects that. If there's anything which you think is not working, please file a bug and link it to this bug. Or is it about not finding the BCC button in the far-right top corner?

A couple of support topics
https://support.mozilla.org/en-US/questions/1278219

I believe that report has never been accurate, but I have personally seen to it that virtually all of the unexpected jumps have been eliminated. In the new design, you can still type a recipient, press Enter, type another recipient, press Enter, no jumps. Only if you press Enter on an empty input, that will obviously move you to the next addressing row.

https://support.mozilla.org/en-US/questions/1278340

Yes, this one is more serious now, describing the effects of habit formation and feeling initially lost in the new layout. I do believe that the new layout is pretty much self-explaining, but yes, to existing users, there's a bit of a learning curve. We should probably prioritize bug 1609647 to ease the transition. We could also explore if there are any sustainable (or maybe temporary) ways of reacting to clicks on the To label (on the right) in an attempt to change the flavor. Postbox has a context menu to show and hide rows right there. We might even show a context menu on left-click. Or flash our new buttons on the right when you click on the left. Such transition candy might be considered when the feature is more complete.

An alternative would be to allow an option to designate mailing lists to be set to add to a default send type To: CC: BCC: in the address book.

On record as bug 163498. Yes, that could be useful to prevent accidental disclosure of the list. On the other hand, you can just add the list into the correct addressing row, and that should work, and it does per my quick tests.

But we have to consider these users, they are not an edge case in my opinion.

We should always consider our users. Edge case or not is a bit hard to tell, but the transition experience would probably deserve some more attention.

Comment 235

[Thomas D. (:thomas8)]

It's unfortunate that the most relevant UX change of Thunderbird 78 has been associated with the wrong product (MailNews Core) all along without anyone taking notice, and hence many bugs cloned from here (and their subsequent clones) are also in the wrong product. There is very little TB code which still lives in MailNews Core (shared with SeaMonkey), so the correct product is Thunderbird.

Can we make sure that at least it will be included in the release notes for TB78, because users won't read the release notes for TB73beta [1] where this landed? Thanks.

[1] https://www.thunderbird.net/en-US/thunderbird/73.0beta/releasenotes/

Comment 236

[Anje]

re :I do believe that the new layout is pretty much self-explaining

Sorry, but I disagree....No it isn't.
Why is Cc and Bcc associated with the FROM by placing it to the right of 'FROM' ?
It is not logical by any stretch of the imagination. It certainly will be a headache for those who have an eyesight issue and cannot locate because they are not where expected.

You can have the same buttons with same effect in the logical position on left, one above the other, next to the 'TO'.
Then it would be intuitive and users would only need to discover/learn the new functionality.
You will always need to use the TO/Cc anyway, so mouse is naturally focussed at start of TO on the left.
The mouse would only be to the right if needing to change the 'FROM' which is probably less used.

Adding and improving functionality is great and much needed, I like the sound of the proposed new functionality and it's potential, but there is no way it is logical to put buttons for cc, Bcc to appear to be associated with FROM.

re :I have never seen buttons to the far right. Been using mailers 35-40 years. Unix, windows, multiple other systems.
I completely agree with this statement.
I'm not aware of other email clients that would put TO and Bcc functionality buttons off top right of a FROM totally unassociated with the whereabouts of the actual TO,Cc,Bcc.

re :If we put them to the left, others will say, why clutter the prime side with secondary options.
They are cluttered on the right side and in the wrong place. The 'prime' side is adjacent to the TO, cc and Bcc.

Have you considered what happens when you have a longer FROM name and email address, this means in order to see that and the buttons, you will have to have a wider window, but if the same space is used for new functionality as is currently used then a greater width of window is not necessary.

Regarding the use of RED in font:
This is never a good idea and has been a serious problem and flaw in Thunderbird for decades. It is important to stop using red for the sake of 10% of men who suffer from various degrees of colour blindness. When I was learning about design decades ago, this was common knowledge that you did not use red on items like email addresses. Red should only be used if the user has full control over change of colour. I still see people asking for how to stop red font colour in eg: TO fields when typing addresses. Please take this into consideration whilst you are redigning this area.

Comment 237

[Thomas D. (:thomas8)]

(In reply to Anje from comment #236)

Hi Anje, thanks for feedback!
It was me who wrote your initial quote, so let me reply. :-)

re :I do believe that the new layout is pretty much self-explaining
Sorry, but I disagree....No it isn't.

If the position of CC and BCC is your only worry, that means that the other 90% of the new layout is self-explaining. Even with CC and BCC I don't think anyone would fail to understand what they will do when you are looking for them...

Why is Cc and Bcc associated with the FROM by placing it to the right of 'FROM' ?

I hear you. There's a separator, so they are not meant to be associated as such. We've tried and discussed many possible positions, and users have requested that we push them more to the middle, so that's where they are now. It's a well-considered compromise position which has several advantages: clean layout, using up less space, allow focus on more important elements, better keyboard access, etc...

Also, there are many other ways of displaying CC/BCC which do NOT require the disclosure buttons on the right:

• right-click on selected items in To-field, then "Move to CC/BCC" from context menu.
• From contacts side bar, select recipients and use Add-to-CC/BCC buttons.
• We'll also implement some variant of bug 1616514, allowing users to start compositions with CC/BCC already open (per identity).

It is not logical by any stretch of the imagination. It certainly will be a headache for those who have an eyesight issue and cannot locate because they are not where expected.

That's a non-argument, honestly. Unexpected location is not an eyesight issue. If you have eyesight issues, we're striving to provide full accessibility for screen readers, but that's all we can do. In fact, by the current position we have optimized the focus ring for the basic case of a simple message with To-recipients so that it's less of a hassle to users with eyesight issues and screenreaders.

You can have the same buttons with same effect in the logical position on left, one above the other, next to the 'TO'.

Where they would use up more space and clutter the area with less used options.

Then it would be intuitive and users would only need to discover/learn the new functionality.

Learning the new position of two buttons isn't very hard imho. We didn't hide them.
In fact, they are now more exposed than before. In the old layout, having to change the type for each and every recipient from a dropdown which is typically hidden was not very intuitive, it's just that we all got used to it.

You will always need to use the TO/Cc anyway, so mouse is naturally focussed at start of TO on the left.
The mouse would only be to the right if needing to change the 'FROM' which is probably less used.

re :I have never seen buttons to the far right. Been using mailers 35-40 years. Unix, windows, multiple other systems.

Well, the Google Web UI has them more far to the right than us, in their default compose UI... So we're NOT the lonely only one out there.

I completely agree with this statement.
I'm not aware of other email clients that would put TO and Bcc functionality buttons off top right of a FROM totally unassociated with the whereabouts of the actual TO,Cc,Bcc.

Hold on Anje, these are just disclosure buttons. After disclosure of CC/BCC rows, labels precede their row.

re :If we put them to the left, others will say, why clutter the prime side with secondary options.
They are cluttered on the right side and in the wrong place. The 'prime' side is adjacent to the TO, cc and Bcc.

Ah well...

Have you considered what happens when you have a longer FROM name and email address,

Yes. I have just tried it on half-screen size and there is no usability issue. Not for the short labels in en-US at least.
That's something to talk about. I like Google's solution where they just use the short English labels CC/BCC even for other localizations like German, to avoid long and clumsy "Kopie / Blindkopie".
But let's not forget that we're a desktop app so far, and screens are typically long horizontally.

this means in order to see that and the buttons, you will have to have a wider window, but if the same space is used for new functionality as is currently used then a greater width of window is not necessary.

I don't think that is true. If I understand you correctly, you'll have two columns for the recipient type labels, so that's using up more horizontal space than now.

Regarding the use of RED in font:
This is never a good idea and has been a serious problem and flaw in Thunderbird for decades. It is important to stop using red for the sake of 10% of men who suffer from various degrees of colour blindness. When I was learning about design decades ago, this was common knowledge that you did not use red on items like email addresses. Red should only be used if the user has full control over change of colour. I still see people asking for how to stop red font colour in eg: TO fields when typing addresses. Please take this into consideration whilst you are redigning this area.

Kindly file a separate bug if you think that's an issue. We're using red to mark erroneous pills, which looks like the only intuitive color for that.
I think it's customizable through adjusting the CSS, probably userChrome.css just like before. Not sure.

Bottom line:
I think compared to the massive overall benefits of the new recipients area (not explained in this comment), getting used to a slightly different position of CC/BCC disclosure buttons is a minor issue of adjusting your muscle memory.

Comment 238

[Thomas D. (:thomas8)]

Attachment: For comparison: Gmail's compose window with CC/BCC disclosure buttons hidden unless Recipients area has focus.

Comment 239

[Thomas D. (:thomas8)]

Comment on attachment 9156532 [details]
For comparison: Gmail's compose window with CC/BCC disclosure buttons hidden unless Recipients area has focus.

.

Comment 240

[Thomas D. (:thomas8)]

Attachment: For comparison: Gmail's CC/BCC disclosure buttons on the far right of the maximised compose window.

Let's please stop claiming that no other email interface has CC/BCC disclosure buttons on right side of the composition window. Gmail's are actually more to the right than ours. And only shown when focus is on Recipient area, as my previous screenshot shows.

(Sorry, something went wrong when filing the first screenshot).

Comment 241

[Wayne Mery (:wsmwk)]

(In reply to Thomas D. from comment #235)

It's unfortunate that the most relevant UX change of Thunderbird 78 has been associated with the wrong product (MailNews Core) all along without anyone taking notice, and hence many bugs cloned from here (and their subsequent clones) are also in the wrong product.

I can fix that en-mass after consulting Alex.

Can we make sure that at least it will be included in the release notes for TB78, because users won't read the release notes for TB73beta [1] where this landed? Thanks.

The ESR release notes are always compiled from all the previous beta notes (of which this bug is a part), which I think should address your concerns.

Comment 242

[Anje]

re:Let's please stop claiming that no other email interface has CC/BCC disclosure buttons on right side of the composition window.

Agree, in gmail webmail, the image shows Cc and Bcc to the far right of the TO field. It is Not to the right of the FROM field and that is the point.

The improvement of a single click rather than current drop down was the original request for this bug meaning a reduction in unnecessary clicks which I've been a supporter of in the menu app icon (it currently does not follow the same 'single click' design idea as in this bug)

I'm not using beta, so this a question, i notice there is a >> insinuating there are more options eg: TO, Cc, Bcc, Reply-to. if you have a narrower window for whatever reason, does this mean you would have only >> visible?
So, is this not defeating the whole object of stopping a double click, just because the Cc and Bcc or other options (I'm assuming TO and Reply-To) can actually be hidden and you need to click just to see them ?

re :If I understand you correctly, you'll have two columns for the recipient type labels, so that's using up more horizontal space than now.

They are only buttons, so they do not need to be horizontal, they can be a vertical alignment, after all a drop down view has always worked. It would just mean the buttons would be visible one click options and not a drop down.

Or a single menu type button on left side of 'TO' which displays options on 'hover' (not click), so you can then do a single click on eg: Cc to get same functionality as the new design.

OR put all the TO, Cc, Bcc and Reply-To options on the Composition Toolbar, then users can customise as they like eg: Remove the 'Reply-to' because they already have this set up in Account Settings. They could have simple box icons with a 'T' for 'TO', 'C' for 'Cc', 'B' for 'Bcc' and 'R' for 'Reply-to', so if someone wishes to use only icons to use even less space.

Comment 243

[Jesse Glessner]

Tuesday, July 28, 2020 - Jesse Glessner
Just had an update of Thunderbird to 78.0.1 (32-bit) - AND THINGS HAVE CHANGED!! I’m running on Windows 7 PRO with plenty of RAM and I know my system is clean as I just this past weekend used MalwareBytes, SuperAntiSpyware, and Avast Premium AntiVirus software packages.

In TB I’m trying to FORWARD a message I noted that the “To:” button did not allow me to change to a “BCC”

I found the “To:”, “Bcc”, etc. buttons in the upper right corner and it did let me select a “Bcc” which appeared in the next field down, leaving the “To” field blank.

I filled in the “Bcc” field but could net get TB to allow more “Bcc” entries. I tried entering several times and nothing happened. I went back up to the upper right where I found the Button links and “Bcc” did not show in there.

Is there any possibility of getting this fixed somehow? I haven’t had problems in TB in a very long time and this definitely surprised me!

Comment 244

[Magnus Melin [:mkmelin]]

There is not just one row per field type. You then comma separate the addresses when you want more than one.

Comment 245

[Alessandro Castellani [:aleca]]

Hi Jesse,
The fields have been redesigned to allow users to input multiple addresses in the same row without the necessity of creating/removing rows.

In your BCC field, write an address and hit enter (or select it from the autocomplete if it's listed there).
After the address is entered and "converted" into a grey pill, keep typing another address. You can add as many addresses as you want in the same row.

If you have an address in another field, eg. To, and you want to move it to another field, you can either cut and paste the address pill, or right click on it and select "Move to..." from the context menu.

Comment 246

[M Lopez-Ibanez]

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

Hi Jesse,
The fields have been redesigned to allow users to input multiple addresses in the same row without the necessity of creating/removing rows.

MRC Compose shows a message explaining this when the field is empty. You can see it in the screenshots here: https://addons.thunderbird.net/en-GB/thunderbird/addon/mrc-compose/?src=ss

That addon has a number of clever and useful features worth checking out (only TB 68 and earlier, though)

Comment 247

[Anje]

Can you clarify a point.
It is easy to add email addresses, select a bunch from Contact Pane and clickon 'add to bcc', all email addresses are added each in their own pill, but I notice there are no auto inserts of a comma, so is it longer necessary for user to enter the comma when adding more than one?

Comment 248

[Anje]

Sorry, saved as draft and I can see the comma is now auto inserted.
However, If you choose to add an email address and then add a comma, when you start typing to add a second address the auto complete fails to work. I presume this is by design as comma is now auto added. Please confirm this is the case.

Comment 249

[Richard Leger]

(In reply to Anje from comment #248)

However, If you choose to add an email address and then add a comma,...

Instead of adding comma, press Enter key, this will convert email address into a pill, and allow you to type new email address afterwards with auto-complete running...

The comma is more used if you copy/paste list of email adresses from another program at once then press Enter to convert the list comma-separated into individual pills.

I think there were a discussion to make the typed comma act as Enter key (and convert address to pills) but it was decided not to for some reason or implemented later... but cannot recall reference about it... someone else may clarify on that ;-)

Comment 250

[doug2]

(In reply to Richard Leger from comment #249)

(In reply to Anje from comment #248)

However, If you choose to add an email address and then add a comma,...

Instead of adding comma, press Enter key, this will convert email address into a pill, and allow you to type new email address afterwards with auto-complete running...

The comma is more used if you copy/paste list of email adresses from another program at once then press Enter to convert the list comma-separated into individual pills.

I think there were a discussion to make the typed comma act as Enter key (and convert address to pills) but it was decided not to for some reason or implemented later... but cannot recall reference about it... someone else may clarify on that ;-)

Comma does work to accept/ complete pill.

Comment 251

[Thomas D. (:thomas8)]

(In reply to Anje from comment #248)

Thanks Anje for requesting confirmation on the exact patterns of creating recipient items (aka "pills") in the composition window, with a view on supporting end users correctly.

The following reflects my personal understanding and evaluation of the current design.
I've been involved in this, and I'm officially involved in support and documentation.

The small print of how (not) to create recipient pills in new Thunderbird addressing rows

1. Recommended way to add new recipient from scratch (same as TB 68): Type, [select autocomplete result], press Enter -> pill is created. Type next, press Enter -> another pill. Simple! :-) Of course, copy and paste will also do the trick (see 4 below).

• Note: Commas between recipient pills are neither needed nor possible; the new design emphasizes recipients as items (more than plain text). In the message source, commas will be correctly added as required to separate recipients.

2. Tolerated alternative: Tab works to confirm the best autocomplete match & create a recipient pill, so it's currently same as Enter (I have some reservations here, because tab is a navigation key, not a confirmation key. However, it's quite convenient...).
3. Subject to change / discouraged until further notice: Typing comma currently also works to confirm autocomplete & create a pill. This is an experimental feature and bug 1642279 suggests that it is not sustainable, at least not for the "autocomplete-on-comma" part; maybe for typing full address and creating pill with comma. (I recommend against using "autocomplete-on-comma" because it may be removed at any time).
4. Accepted use of comma: Pasting a comma-separated list of recipients into addressing rows, then pressing Enter/Tab to convert them into pills should work without fault.
5. Discouraged/disfunctional uses of comma:

• Typing a comma after an existing recipient pill for the sake of separating recipient pills is not needed (see 1), prevents autocompletion, and it won't work correctly if you just want to create another pill.
• Starting a new recipient with a comma might be valid syntax, because comma in display name is legitimate, and we don't always expect quotes, so entering ,Paul <[e-mail]> will create a valid pill with ",Paul " <[e-mail]> in source. Obviously discouraged, confusing and error-prone for users and mail clients.
• There's a bug that typing comma prevents getting autocomplete suggestions (already in TB 68), probably because the autocomplete widget itself considers that a separator. So even after fixing the worst of bug 1642279, entering a known email address with comma in display name may not yet autocomplete.

Hth! ;-)

Comment 252

[Anje]

I'm finding the new design to be very pretty and so easy to add recipients in various ways.

However, it comes with some irritating usability flaws.

The one huge advantage of the old design was the fact it was automatically in a listed type of view. This meant if you needed to scan down through to check recipients it was much easier as they were in listed form.
This was particulary important if using a mailing list.
AS you scanned down with mouse, the X (for delete) auto appeared and a simple single click removed that recipient from list.
It was very efficient and user friendly.

Under this new design, you have to play around with the width of the window to try to get one per line view which is a real nuisance and time waster as some email addresses with Display Name can be twice as long as others. Why this 'play with window width' method was considered the way to organise as a visible list is not really an acceptable method.
Now as you scan down there is no auto X appearing, so simple one click delete has been removed or perhaps when designed this aspect of use was not considered. Now each time you have to right click to get a drop down option and then select to delete OR left click and use backspace. The point is it is taking twice as much clicking, so hardly an improvement. From a business point of view, this part of the design slows down work throughput and increases clicking which is undesirable from a health point of view.

So basically the new design needs to be able to be quickly put into list mode without playing around with the width of the window and at the same time offer an auto X to appear in left column facilitating a quick delete from list. But if you can at least get it into list mode that would be a start.
I cannot find how to quickly achieve this. I cannot see anything under 'View' nor 'Customise' which does not seem to have anything I can drag onto toolbar to do this task.
What am I missing?

Comment 253

[Anje]

Can I ask a question on what the developer included to perform the following:
If I wanted to change an email address currently in a TO field into a Cc field using keyboard.
Use Ctrl+tab to get into the TO field and use arrow keys to select
At this point I'm lost - what is the keyboard selection to get the drop down list to appear which you can select using right click?

Comment 254

[Alessandro Castellani [:aleca]]

If you have a keyboard with the right-click shortcut key, just press that so you can select "Move to..."
This article shows other approaches to use a keyboard for right clicking: https://www.online-tech-tips.com/computer-tips/right-click-keyboard-shortcut/

Otherwise, you can always CLTR+X to cut, then open the CC field, and CTRL+V to paste the addresses.

Comment 255

[Anje]

Thanks Alessandro.

Comment 256

[Thomas D. (:thomas8)]

(In reply to Anje from comment #252)

I'm finding the new design to be very pretty and so easy to add recipients in various ways.

Thanks! :-)

The one huge advantage of the old design was the fact it was automatically in a listed type of view. This meant if you needed to scan down through to check recipients it was much easier as they were in listed form.

We're exploring options to provide a single-column layout for recipients again in bug 1635207. Any comments there (if at all) should be as short as possible.

AS you scanned down with mouse, the X (for delete) auto appeared and a simple single click removed that recipient from list.
It was very efficient and user friendly.

You'll never get maximum efficiency with mouse. Apart from the lack of single-column mode (acknowledged), pls try this on keyboard:

• Use cursor ⯇ or ⯈ to navigate through addresses, which auto-selects each recipient item. Just press DEL to remove a selected item.
• To delete neighbouring items, you can even use consecutive DEL (forwards) or Backspace (backwards) keypresses which will also auto-select without requiring manual selection.
• You can even hold those deletion keys (say to delete 5th to 10th items forwards with DEL, or the 3rd to 1st backwards with Backspace) and they'll stop fast-track deletion when you touch a boundary. If you deliberately press DEL or Backspace again, it might delete an item or an empty CC/BCC input row again as appropriate (same as TB 68).

Now as you scan down there is no auto X appearing, so simple one click delete has been removed or perhaps when designed this aspect of use was not considered.

We carefully considered and removed single-click deletion: Permanent delete icons on every pill were a lot of visual clutter, taking up space.
That said, we appreciate constructive user feedback and Thunderbird is all about efficiency, so I have just filed Bug 1661438 and wrote up a patch to restore single-click deletion of recipient items using middle mouse button (similar to removing tabs in Firefox with middle-click)! How's that? :-)

Now each time you have to right click to get a drop down option and then select to delete OR left click and use backspace.

You could use keyboard (explained above) or right-click and press D access key to delete (in en-US), and of course left-click, then DEL key also works.

The point is it is taking twice as much clicking, so hardly an improvement. From a business point of view, this part of the design slows down work throughput and increases clicking which is undesirable from a health point of view.

Good points! Let's see if we can do Bug 1661438.
Please note that all selection/action patterns for multiple recipients are now radically more efficient, requiring up to 99% less clicks and key presses. Try deleting all recipients in TB 68 (50+ recipients...), then try Ctrl+A, Ctrl+A (sic, twice), DEL in TB 78! :-)

Thanks for your feedback, Anje!

Comment 257

[Thomas D. (:thomas8)]

Important notice

---

Please note that we now have updated documentation (currently en-US only) for the new addressing field:

https://support.mozilla.org/en-US/kb/addressing-email [English template version]
https://support.mozilla.org/kb/addressing-email [official link which redirects to localized versions; translations pending]

For any support questions beyond that article, please use:
https://support.mozilla.org/products/thunderbird

---

Comment 258

[Richard Leger]

(In reply to Thomas D. (:thomas8) from comment #256)

(In reply to Anje from comment #252)

Now as you scan down there is no auto X appearing, so simple one click delete has been removed or perhaps when designed this aspect of use was not considered.

We carefully considered and removed single-click deletion: Permanent delete icons on every pill were a lot of visual clutter, taking up space.
That said, we appreciate constructive user feedback and Thunderbird is all about efficiency, so I have just filed Bug 1661438 and wrote up a patch to restore single-click deletion of recipient items using middle mouse button (similar to removing tabs in Firefox with middle-click)! How's that? :-)

Thomas, Alex,

Having contributed in the past to current pills design and after using it for a few months on a daily bases two major things seems missing:

• Delete pills with mouse over + one click (I will expand my view below from experience and natural expectation) - Please reconsider implementing this feature which I agree with Anje from daily use is missing. Especially on windows and laptop users using a trackpad.
• drag and drop of pill(s) - already under implementation in a separate bug.

My suggestion:

I agree that to avoid clutter the cross icon to delete a pill shall not always be visible... but when mouse over the pill the icon shall appear as a small overlay (align to the right) as well as a three-dot vertical (or hirizontal or burger menu) to reach the right click options via left click on menu icon) something like:

([e-mail]) <-- default display - no mouse over

(name@domai [=|x] ) <-- mouse over display - small minimal menu appears - the x icon allowing to delete in one click - the = menu icon allowing to access more option - same as right click on pills -
the look and feel would be at you discretion of course... the above is just a proof of concept... but may look good ;-)

And if you have many pills only the moused over one would show icons...

([e-mail]) ([e-mail]) (name@domai [=|x] ) ([e-mail]) ([e-mail]) ([e-mail])

The middle click is not much used by average Windows users nor I know how to do it on laptop tracpad! (my bad) - not much intuitive but I am sure Linux users may love it for a quicker click delete ;-)

The additional solution above has many advantages, it keep clutter at minimum while allowing quick one click deletion, it is simple self explanatory and intuitive... especially for those that click... a lot ;-) or new comers to email client software.

Everyone is different some users use keyboard, some use mouse, some use both... so why not to polish the compose windows recipient (probably one of the most important feature in TB) to make it user friendly to all :-)

I wish you may reconsider...

Comment 259

[Alessandro Castellani [:aleca]]

Thanks for raising these points.
I'd say this is worth exploring and doing some quick interactive prototyping to see how it feels and looks during simulated real life interaction.
Let's open a dedicated bug for that and continue the conversation there.
We should try to stop adding comments to this bug :D

Comment 260

[Matt]

(In reply to Thomas D. (:thomas8) from comment #257)

Important notice

---

Please note that we now have updated documentation (currently en-US only) for the new addressing field:

https://support.mozilla.org/en-US/kb/addressing-email

For any support questions beyond that article, please use:
https://support.mozilla.org/de/products/thunderbird

---

Actually already translated to the languages shown below https://support.mozilla.org/en-US/kb/addressing-email/show_translations

Note when posting a link to the SUMO site knowledge base, remove the locale identifier unless you want the person selecting the link to be directed to a specific language version. So that link I placed above should properly be posted as https://support.mozilla.org/kb/addressing-email/show_translations Based on the browser's locale the language will be selected or an English version served if there is no translation.

English (en-US)
বাংলা (bn)
Čeština (cs)
Dansk (da)
Deutsch (de)
Español (es)
suomi (fi)
Français (fr)
Italiano (it)
日本語 (ja)
한국어 (ko)
Nederlands (nl)
Polski (pl)
Português (do Brasil) (pt-BR)
Português (Europeu) (pt-PT)
Русский (ru)
slovenščina (sl)
中文 (简体) (zh-CN)

Comment 261

[Thomas D. (:thomas8)]

(In reply to Matt from comment #260)

Actually already translated to the languages shown below https://support.mozilla.org/en-US/kb/addressing-email/show_translations

Actually NOT yet translated to any other language, exactly as I said. show_translations is lying to you because the TB78 en-US template version for this article has not yet been released for translations. Please compare the contents. All those translations are still the old version of the article, where the second headline involves Google. I have fully rewritten the article for TB78, added 12 new screenshots, the Google section is gone, many new sections added, and the new version so far is ONLY available in en-US, hence I used the hard en-US link (but thanks for pointing that out anyway).

Comment 262

[Thomas D. (:thomas8)]

(In reply to ni?thomas8 by Richard Leger from comment #258)
Richard, good point about laptop trackpads which don't have an explicit middle button, so using Bug 1661438 is tricky for them. I guess you could always connect a 3-button mouse if you really need that. Many trackpads also allow emulating the middle button (similar to emulating the right button), e.g. as 3-finger-tap.
https://www.howtogeek.com/howto/26552/stupid-geek-tricks-use-both-laptop-mouse-buttons-to-simulate-middle-click/

Yes, extra icons on hover is another way. Makes the UI more nervous. Error-prone if delete icon suddenly appears under your mouse where before you'd see just a plain pill which you want to select; we had that in TB68 on the recipient type selector and users hated that. But there are other ways of "reveal-on-hover", outside the pill, like to the right (overlapping the next pill which you have not hovered), or below. As Alex said, we cannot explore the details here, needs another bug.

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

Thanks for raising these points.
I'd say this is worth exploring and doing some quick interactive prototyping to see how it feels and looks during simulated real life interaction.
Let's open a dedicated bug for that and continue the conversation there.

+1

We should try to stop adding comments to this bug :D

Wouldn't enforce it (yet), but worth a try :D

Comment 263

[Matt]

@ Thomas I have released it now as you pointed it out to me. Hopefully, we can do better next time. I do not spend much time on kb issues, but if thigs are drawn to my attention I do try and correct them

Comment 264

[Thomas D. (:thomas8)]

(In reply to Matt from comment #263)

https://support.mozilla.org/en-US/kb/addressing-email
Thomas I have released it now [for translation] as you pointed it out to me.

Awesome, thank you, Matt! I have requested SUMO reviewing rights a while back but they have not yet materialized...

Comment 266

[Marco A.G.Pinto [:marcoagpinto]]

Is it too much to ask for a small font line with a similar structure of the text:
"Total: 340 (To: 100, Cc: 100, Bcc: 140)"
?

Comment 267

[Thomas D. (:thomas8)]

(In reply to Marco A.G.Pinto [:marcoagpinto] from comment #266)

Is it too much to ask for a small font line with a similar structure of the text:
"Total: 340 (To: 100, Cc: 100, Bcc: 140)"
?

Please provide a dedicated RFE with details, scenario, maybe UI mockup. This isn't quite the right place in comment 266. As a general guide, we prefer a clean and simple UI, so you'd probably have to propose a way of tucking that information away, say in an 🛈 icon on the header, status bar, tooltip, context menu or some such.

Comment 268

[Marco A.G.Pinto [:marcoagpinto]]

(In reply to Thomas D. (:thomas8) from comment #267)

(In reply to Marco A.G.Pinto [:marcoagpinto] from comment #266)

Is it too much to ask for a small font line with a similar structure of the text:
"Total: 340 (To: 100, Cc: 100, Bcc: 140)"
?

Please provide a dedicated RFE with details, scenario, maybe UI mockup. This isn't quite the right place in comment 266. As a general guide, we prefer a clean and simple UI, so you'd probably have to propose a way of tucking that information away, say in an 🛈 icon on the header, status bar, tooltip, context menu or some such.

I believe we just need an empty line below the pills TO, Cc and Bcc with this information in a small font.

EDIT: Maybe using an 🛈 icon.