1602442 - Pill edit mode: Entering multiple comma-separated recipients discards all but the first one

Status: RESOLVED WONTFIX

(Thunderbird :: Message Compose Window, enhancement)

Description

[Thomas D. (:thomas8)]

I am looking at the latest available iteration of bug 440377 (try build 73.0a1 (2019-12-06) (64-bit).*

STR

1. compose with recipients: pill1, pill2, pill3 with correct email addresses
2. edit pill2, type: "[e-mail], [e-mail]" (without quotes), press Enter (trying to add another address in between - why not? users do all sorts of things, this looks like a text input, so the normal input method should work...)

Actual result

• [e-mail] is discarded without warning or any other prior/post-factum indication of error. This also applies for the last pill, less likely, but even even more surprising.

Expected result

• From user's pov, a correctly entered address should not be discarded without trace or warning
• Users are not wrong to regard the entire recipient field as a big textbox, so whenever they have a cursor anywhere in that box they should be able to type along freely, multiple recipients or not.
• This might be mitigated by actually allowing cursor to add recipients between existing pills (bug 1603051).
• With bug 1603051: As soon as user types a comma after a full address in edit mode, we could finish the current pill, so that user continues to type the new recipient at the insertion point right after that pill (current Postbox behaviour). For the scenario of this bug, gmail pushes the cursor to the end (less ideal).
• Arguably, anything which is typed after or between commas, should also auto-complete.

*: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=674bb73bd2b093deb8170dc8ecd7d67b5d1239a9

Comment 1

[Magnus Melin [:mkmelin]]

Thomas, please hold off on creating follow-ups until the feature is actually landed (along with planned clean-up)

Comment 2

[Thomas D. (:thomas8)]

Related: bug 1609894

Comment 3

[Alessandro Castellani [:aleca]]

I'm not sure I agree with this.
If you're editing a pill it means you want to update the address you previously wrote.
I don't think we should allow adding multiple addresses in 1 pill, that doesn't make sense from a workflow point of view and we shouldn't enable this behaviour.
It creates extra complexity for us to maintain and it's a paradigm I don't think we should introduce.
I would mark this as WONTFIX.
Magnus and Richard, what do you think?

Comment 4

[Richard Marti (:Paenglab)]

Yes, when editing a pill, we shouldn't allow this. New contacts should only be added in the normal fields.

Comment 5

[Thomas D. (:thomas8)]

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

I'm not sure I agree with this.
If you're editing a pill it means you want to update the address you previously wrote.

To you as the designer, yes. Are we currently offering any other way of inserting another recipient at the user's preferred position?
Order of recipients does matter in real life...

I don't think we should allow adding multiple addresses in 1 pill, that doesn't make sense from a workflow point of view and we shouldn't enable this behaviour.

Well, this bug is not exactly about "enabling" or "encouraging" that behaviour, but about error prevention, and handling predictable scenarios (more so as long as there are no other alternatives of getting the desired result).

It creates extra complexity for us to maintain and it's a paradigm I don't think we should introduce.

Well, if we allow the user to type "[e-mail], [e-mail]" inside a single pill in the first place (which may make perfect sense from their pov), you can't just discard legitimate input without warning. Either that, or just prevent entering more than one address into the pill. We know the syntax of a single address, isn't it? We need to work on refined address validation anyway, see bug 1609894.

Btw, but just as a minor footnote, I guess we're still ignoring Bug 919953: RFC822 groups, i.e. named lists with concluding semicolon, e.g. |testlist: [e-mail], [e-mail];|

I would mark this as WONTFIX.

I suggest that we re-evaluate this bug after you are done with bug 1601749 (drag n drop of pills), where you want to

allow manual reordering of pills within the same recipient container.

There's no way of doing that without implementing at least half of bug 1603051 (Enable reordering and inserting of pills), i.e. to have inter-pill insertion points (as drop targets). The other half is making reordering keyboard-accessible. After that, it'll make more sense to talk about this bug again, and I think we'd still have to do the error-prevention part one day.

Comment 6

[Thomas D. (:thomas8)]

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

It creates extra complexity for us to maintain and it's a paradigm I don't think we should introduce.

Every good behaviour adds extra complexity. I think it's not too hard because already you are suceeding to cut off everything after the comma which follows the first valid address. Just parse the rest of the string as you would parse a regular comma-separated text input on rowInput, adding pills as required.

Comment 7

[Alessandro Castellani [:aleca]]

Are we currently offering any other way of inserting another recipient at the user's preferred position?

This is not a good excuse to implement something that it doesn't make sense.
We shouldn't try to solve a problem by tackling it from multiple bugs and adding multiple solutions to it.

A pill is a single, containerize, entity that should not bleed out.
You constantly reference Windows folder. Imagine editing a folder name and, because you put a comma in the name, expecting a second folder to popup right after. That doesn't make sense and we shouldn't enable this.

The problem of reordering pills and adding addresses in between pills will be tackled in the respective bugs. This is not a bug, this is a wanted and expected behaviour that should stay as it is and not be changed to accommodate something for later.

As a general rule, please:

• Don't change the flags in a bug. If I set this as P3, leave it as P3.
• Don't open 3 or 4 bugs all related to each other, this is terribly confusing and hard to track.

Comment 8

[Magnus Melin [:mkmelin]]

I agree, I don't think we want to allow this. Sometimes it's just better to teach users not to do stuff the wrong way.

Comment 9

[Thomas D. (:thomas8)]

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

As a general rule, please:

• Don't change the flags in a bug. If I set this as P3, leave it as P3.

Sorry, no offence meant, I hadn't seen that you had deliberately set it to P3, I actually thought the priority had been copied over by myself when cloning to create this bug, so my intention was to reduce the priority/importance of this bug as it was contested. Please feel free to re-set P3 if it's still needed now that the bug has been wontfixed.

• Don't open 3 or 4 bugs all related to each other, this is terribly confusing and hard to track.

Sorry if that's your impression; I'm surprised and I'd love to hear the details of that concern in private mail, maybe I can help to clear up misunderstandings. From our good cooperation so far, I'm confident we'll find common ground. I always strive to follow BMO's "one bug per issue" rule (1) quite painstakingly, also knowing from 13 years of bug filing and triaging experience that conglomerate bugs/RFE's typically have low chances of getting addressed. Bugs/RFEs on recipient pills are necessarily all somehow related to each other, but we have to keep things separate somehow. It's in the nature of RFEs that until they are approved, exploring alternatives may occur, and fixing existing bugs can take different directions depending on pending features / UX changes. Again from my experience, it's much easier to merge bugs if we find that they can be handled as one than to dissect an overloaded bug... Ymmv. Just my 2 cents...

(1) https://developer.mozilla.org/en-US/docs/Mozilla/QA/Bug_writing_guidelines

Comment 10

[Thomas D. (:thomas8)]

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

I agree, I don't think we want to allow this. Sometimes it's just better to teach users not to do stuff the wrong way.

I'm not contesting the wontfix decision of this bug, but let me say this:

• I have never insisted that we should implement a new behaviour here. I have filed this as primarily as a bug that valid recipient data entered by the user gets eaten, which is wrong in terms of ux-error-prevention, and I've suggested several possible ways of addressing that, including technical prevention of undesired user input.
• Two competitors (Gmail and Postbox) support creating new recipients from editing an existing pill by just typing comma. That's a fact.
• Thunderbird legacy behaviour is "one address per box", but we've also supported typing more than one comma-separated address into the single "box" since time immemorial. So I wouldn't be surprised if users try the same pattern inside pills, the new "boxes".
• My understanding of "teaching the users" is different from just letting them fall into the trap, "You see now! That's what you get, dummy" style...

Comment 11

[Thomas D. (:thomas8)]

Right, so here's a proper bug which is indeed directly related to this very bug as originally filed. Please advise if you want this reported on a new ticket, or to reopen this one.

STR

1. New message with 2 recipient pills: ([e-mail]) ([e-mail])
2. Edit pill1, and (from known use patterns on other mailers and current TB, and as a user unaware of the new design intentions) try to insert another recipient from editing pill1: "[e-mail], [e-mail]". Press Enter and observe the result.
3. Edit pill1 again.

Actual result

• Enter pretends to discard "[e-mail]", pill remains with "[e-mail]" as a label
• Re-edit surprisingly makes the seemingly discarded recipient reappear: "[e-mail], [e-mail]" (text input storing the last confirmed value, only display string of the label got chopped).
• User starts wondering if TB is a trustworthy mail client, hiding invisible recipients in some other pill...
• User now confused as to whether message will get sent to "[e-mail]" or not...?
• At least we don't send the message to the hidden newrecipient, but user can't tell...

Expected result

• Don't pretend to discard newrecipient if it's not properly discarded, which is confusing and useless
• Ideally, if we don't accept adding comma-separated recipients from pill edit mode, we should technically prevent that input (not sure how hard that would be: on input keypress, check string for the first valid recipient, then chop the value accordingly and discard the rest of it).

I reserve my comment...

Comment 12

[Thomas D. (:thomas8)]

Mass-changing bugs around the new recipient area (pills) from product/component MailNews Core/Composition to Thunderbird/Message Compose Window, because composition frontend code is not shared with SM. Mostly cloned from Bug 440377 which started out in MailNews Core long back.

20200614002RecipientPillsProductChangeTypeEnhancement