1609894 - Typing comma (,) in new recipient display name creates invalid pill prematurely (cannot enter: Surname, First Name <[e-mail]>)

Status: RESOLVED FIXED

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

Description

[Thomas D. (:thomas8)]

STR

1. New message, try to enter new recipient (unknown in AB): Doe, John <[e-mail]>

Actual

• entering comma prematurely creates an invalid pill without address validation: (Doe)
• impossible to enter display name with comma (even in double quotes)

Expected

• comma must only auto-create pill after validation (failing that, we'd have to wait for Enter before creating multiple pills in one go). We already have such validation algorithms because if you paste |Doe, John <[e-mail]>, [e-mail]|, that's parsed correctly.
• don't create pill prematurely for invalid recipient
• allow entering display names with comma
• also handle double quotes according to specifications (iirc, almost anything can be display name if in double quotes)

Comment 1

[Alessandro Castellani [:aleca]]

Attachment: 1609894-typing-comma.diff

With this patch the generation of the pills is not automatically triggered if:

• The value typed is not valid (and therefor it wasn't autocompleted into a valid address)
• The string starts with double quotes.

This is a bit of an edge case in which is hard to cover every scenario. Example:

• The user has Doe, John <[e-mail]>
• If the user types doe, the autocomplete will kick in as that address with the comma exists and it was suggested.
• If the user types "Doe, John <[e-mail]>", the autocomplete doesn't kick in as we're preventing it altogether.

As a general rule, we should prevent users from typing multiple addresses before triggering pills as the autocomplete doesn't like that and highlights the text as error.

Comment 2

[Magnus Melin [:mkmelin]]

Comment on attachment 9127429 [details] [diff] [review]
1609894-typing-comma.diff

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

::: mail/components/compose/content/addressingWidgetOverlay.js
@@ +534,5 @@
> +      // Don't trigger autocomplete if the typed value is not a valid email
> +      // address or the string starts with a double quote.
> +      if (!isValidAddress(element.value) || element.value.includes('"')) {
> +        return;
> +      }

I think the quotes part should be removed.

For this parsing we don't really want to look at what technically would/should be quoted or not, we want the real input and for sending our code will quote as appropriate.

Comment 3

[Alessandro Castellani [:aleca]]

Attachment: 1609894-typing-comma.diff

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1d6c19070bff
Prevent prematurely creating addressing pills when typing commas on invalid address. r=mkmelin

Comment 5

[Thomas D. (:thomas8)]

FTR: This might be a bit better than before, but it's definitely NOT fixed.

E.g. after this patch, when "Johnson" happens to match even several different autocomplete suggestions, trying to enter a new recipient "Johnson, Newman <...>" will still prematurely and randomly confirm the first autocomplete suggestion upon typing the comma (without any explicit user confirmation like Enter or Tab), making it impossible to enter any new recipients |Last, First <...>| when Last happens to match anything.

We must simply NOT autocomplete on comma. We can create a pill after user has explicitly accepted an autocomplete proposal by Enter or Tab (which is exactly current release behaviour), or maybe if what the user has typed (without the inline autocompletion part!) is a valid address followed by comma. Typing "Johnson, " is NOT a complete and valid address by any standards; for multiple autocomplete matches, we don't know the right one, and we can't read user's mind wrt autocomplete vs. new recipient.

Comment 6

[Alessandro Castellani [:aleca]]

I'm personally not totally against removing the "autocomplete-on-comma" feature as I personally don't use it, but I see how it can be useful to users, but also how it requires a bit of a learning curve.

Once the autocomplete kicks in, typing comma to start typing another address is a bit more natural and keeps the flow going then hitting Enter or Tab.
Having addresses saved as "FirstName, LastName ..." seems a bit of an edge case, quiet unusual, and IMHO wrong as we should discourage using these special characters for saved entries.
If the user types "Johnson, ", and more than 1 autocomplete gets suggested, he should select the preferred address from the dropdown instead of typing the entire name + email + special characters. Seriously, who does that?

As I said, I don't have a strong opinion on this feature, but I think having it is better than not.

Comment 7

[Thomas D. (:thomas8)]

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

I'm personally not totally against removing the "autocomplete-on-comma" feature as I personally don't use it, but I see how it can be useful to users, but also how it requires a bit of a learning curve.

All but undiscoverable, so users may never even try. Has anyone ever seen autocomplete-on-comma, anywhere? Firefox location bar does not even allow Tab for autocompletion.

Once the autocomplete kicks in, typing comma to start typing another address is a bit more natural and keeps the flow going then hitting Enter or Tab.
Having addresses saved as "FirstName, LastName ..." seems a bit of an edge case, quiet unusual, and IMHO wrong as we should discourage using these special characters for saved entries.

Based on what data? Imho, "Last, First" is quite far from being an edge case...

• Dedicated view mode "Last, First" in TB's address book, and fully supported in current TB (composition, address book)
• Widespread use case for business, schools, authorities, you name it. Anecdotal evidence: I actually have a bunch of messages from our local administration right in front of me, the person works in section 12 of the local administration of our city, using the following address pattern:
  Miller, 12, OurCity <[e-mail]>
• Default order for many other languages/cultures (although there's a tendency towards the "Western" norm for purposes of communication).

In view of the above, I doubt that comma in display name qualifies as a "special character" in users' perception. The above example from local authorities also shows that there are many more use cases than just "Last, First" (note the name of town appended with comma above).

If the user types "Johnson, ", and more than 1 autocomplete gets suggested, he should select the preferred address from the dropdown instead of typing the entire name + email + special characters. Seriously, who does that?

• Hmmm, how do I select a new address from autocomplete dropdown?
• If I can't select it because it doesn't exist, how do I type it into recipient input with this bug?
• How do I even autocomplete an existing address |Last, First <[e-mail]>| from my AB which may have several matching addresses like the following?
  • John Paul Smith <[e-mail]>
  • John, Elton <[e-mail]>

Typing "John" autocompletes inline to "John Paul Smith", typing "John," prematurely confirms that (this bug), so how do I get the other one with comma from my address book?

As I said, I don't have a strong opinion on this feature, but I think having it is better than not.

Well, I'd think having this "feature" is a no-go as it can render the following user actions disfunctional (wrt comma in display name):

• entering new valid recipient addresses
• retrieving existing valid recipients via autocomplete

Comment 8

[Thomas D. (:thomas8)]

(In reply to Thomas D. from comment #7) [snip]

(In reply to Alessandro Castellani (:aleca) from comment #6) [snip]

As I said, I don't have a strong opinion on this feature, but I think having it is better than not.

Well, I'd think having this "feature" is a no-go as it can render the following user actions disfunctional (wrt comma in display name):

• entering new valid recipient addresses
• retrieving existing valid recipients via autocomplete

For all the reasons mentioned in comment 7: followup
Bug 1642279 - Email addresses with comma in display name cannot be entered via keyboard or via context menu