1674054 - All non-pillified recipients (pasted, typed) silently discarded when clicking [Send] button directly

Status: RESOLVED FIXED

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

Description

[josh]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:81.0) Gecko/20100101 Firefox/81.0

Steps to reproduce:

1. Compose email body + subject
2. Enter (or paste) e-mail address in To field. Do not press Space, or Comma, etc, to complete the Pill.
3. With the mouse cursor, click "Send".

Actual results:

4. Will receive a message: "No recipients were specified. Please enter a recipient or newsgroup in the addressing area."
5. After dismissing the e-mail address Pill is completed.
6. You can send now.

(Alternatively if you have a correct address in the To field already, or you're entering an address into CC etc, then you do not get the warning message at step #4. Instead the extra addresses are lost and the email is sent with an incomplete list of recipients).

Expected results:

The e-mail pill should be completed or treated as text and sent.

Comment 1

[Thomas D. (:thomas8)]

Wow! Thank you Josh for reporting this.

This is highly error-prone and datalossy, regardless of the likelihood.

STR:

• Have just 1 validated pill (e.g. auto-Bcc)
• paste a csv list of 50 recipients, without confirming them or clicking anywhere else
• click Send immediately

Actual result:

• upon sending, the whole set of 50 recipients is silently discarded!

Aleca, can you look into this?

Proposed fix:

• Imho, Thunderbird should just disable the Send button as long as there are unfinished pills, because it's also error-prone if we do autocomplete-on-send and just pick the currently best autocomplete result or convert csv-lists to pills whilst the user isn't watching and has no way of double-checking because the message will be already out. We should expect users to confirm their recipients, at least by clicking somewhere in the message.
• Personally, I find even autocomplete-on-blur quite questionable - what if another app steals focus whilst I'm in the middle of typing a recipient? Maybe we should only autocomplete for Enter, Tab, cursor-right (Bug 1672916).

Josh, I am unable to reproduce your first scenario - if there's no finished pill, send button should be disabled. Can you provide more precise STR for that case? Does it involve newsgroups or active addons? Every single detail matters so that we can reproduce.

Comment 2

[Thomas D. (:thomas8)]

Technically, I'm actually surprised that clicking [Send] does not trigger blur on the address input, assuming that we pillify on blur... but it wouldn't be a very good/safe solution anyway as outlined above.

Comment 3

[josh]

Josh, I am unable to reproduce your first scenario - if there's no finished pill, send button should be disabled. Can you provide more precise STR for that case? Does it involve newsgroups or active addons? Every single detail matters so that we can reproduce.

• OpenSUSE Tumbleweed
• Thunderbird 78.3.2 (64-bit)
• KDE Plasma
• Extensions: CardBook, Enigmail, Sieve, ThreadVis. (No extensions relating to Composition AFAIK).

The send button is never disabled for me (I didn't even realise it should be). When I open the compose window I can immediately press the Send button and receive the error about not having any recipients.

My opinion is that auto-complete should not happen on blur/de-focus. Instead if I'm partially through typing an address, it should just attempt to send with whatever that may be. For example, "[e-mail]" should be sent exactly as that, not tried to complete to "Mr. Person <[e-mail]>" as I may have been specifically sending something to the "example.com" domain (not .au).

Comment 4

[Alessandro Castellani [:aleca]]

Technically, I'm actually surprised that clicking [Send] does not trigger blur on the address input

Indeed, this is very strange and it should happen.

Imho, Thunderbird should just disable the Send button as long as there are unfinished pills...

Mh, I think we could implement that, but maybe we should also add a tooltip explanation when hovering over the disabled Send button. Something like "unfinished typed recipients" or something like that.
This might also be a bit confusing for users so we need to be careful.

Personally, I find even autocomplete-on-blur quite questionable - what if another app steals focus whilst I'm in the middle of typing a recipient? Maybe we should only autocomplete for Enter, Tab, cursor-right

We should keep autocomplete-on-blur as it's part of the natural progression for users that only use the mouse (eg. my wife never uses tab to move the cursor to another field, which drives me crazy, but that's another story).

Comment 5

[Thomas D. (:thomas8)]

(In reply to josh from comment #3)

Josh, I am unable to reproduce your first scenario - if there's no finished pill, send button should be disabled. Can you provide more precise STR for that case? Does it involve newsgroups or active addons? Every single detail matters so that we can reproduce.

• OpenSUSE Tumbleweed

Did u download TB as distro package or directly from thunderbird.net?

• Thunderbird 78.3.2 (64-bit)
• KDE Plasma
• Extensions: CardBook, Enigmail, Sieve, ThreadVis. (No extensions relating to Composition AFAIK).

At least Enigmail and CardBook might certainly relate to composition.

The send button is never disabled for me (I didn't even realise it should be). When I open the compose window I can immediately press the Send button and receive the error about not having any recipients.

Is the send button still always enabled after "Help > restart with addons disabled" ?

My opinion is that auto-complete should not happen on blur/de-focus. Instead if I'm partially through typing an address, it should just attempt to send with whatever that may be. For example, "[e-mail]" should be sent exactly as that, not tried to complete to "Mr. Person <[e-mail]>" as I may have been specifically sending something to the "example.com" domain (not .au).

Good points! You could file a bug or RFE if you're unhappy with the current behaviour.
Fyi, the current way of dealing with entering example.com (not in your AB) vs. example.com.au (in your AB) is that you are expected to manually remove the autocomplete suggestion using DEL or backspace before proceeding - which sort of reverts the burden into autcomplete inconvenience as you have already entered a complete and correct address...

Comment 6

[Thomas D. (:thomas8)]

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

[Thomas:] Personally, I find even autocomplete-on-blur quite questionable - what if another app steals focus whilst I'm in the middle of typing a recipient? Maybe we should only autocomplete for Enter, Tab, cursor-right

We should keep autocomplete-on-blur as it's part of the natural progression for users that only use the mouse (eg. my wife never uses tab to move the cursor to another field, which drives me crazy, but that's another story).

The question is, when your wife needs to enter [e-mail] (not in AB) manually with [e-mail] already in your AB, with inline autocomplete triggering, how do you know which address she really wants? All those guessing games without explicit user confirmation are actually a high risk of causing privacy violations via unintended addresses. Even <tab> to autocomplete is an abuse, because <tab> should be respected as a pure navigation key instead of changing my input. Firefox location bar certainly had their reasons to limit autocomplete to explicit confirmation with Enter - nothing else - take it or leave it! Tab from location bar will navigate into autocomplete suggestions and place them into location bar, but you still need to press Enter to accept the proposal.

Comment 7

[Thomas D. (:thomas8)]

Iow, stopping to type your recipient halfway and then expecting TB to autocomplete anyway without any explicit confirmation like Enter is a wrong and error-prone expectation.

Comment 8

[Thomas D. (:thomas8)]

(In reply to Thomas D. (:thomas8) from comment #5)
^^ ni?reporter

Comment 9

[josh]

(In reply to Thomas D. (:thomas8) from comment #5)

(In reply to josh from comment #3)

Josh, I am unable to reproduce your first scenario - if there's no finished pill, send button should be disabled. Can you provide more precise STR for that case? Does it involve newsgroups or active addons? Every single detail matters so that we can reproduce.

• OpenSUSE Tumbleweed

Did u download TB as distro package or directly from thunderbird.net?

It is installed from system packages:

Repository : openSUSE-Tumbleweed-Oss
Name : MozillaThunderbird
Version : 78.4.0-1.1

• Thunderbird 78.3.2 (64-bit)
• KDE Plasma
• Extensions: CardBook, Enigmail, Sieve, ThreadVis. (No extensions relating to Composition AFAIK).

At least Enigmail and CardBook might certainly relate to composition.

Indeed you are right (see below).

The send button is never disabled for me (I didn't even realise it should be). When I open the compose window I can immediately press the Send button and receive the error about not having any recipients.

Is the send button still always enabled after "Help > restart with addons disabled" ?

Starting in safe-mode has the Send button disabled in the composition window.

Restarting Thunderbird and then disabling CardBook appears to also have the Send button disabled. In other words, I think CardBook may be the offending plugin. https://addons.thunderbird.net/en-US/thunderbird/addon/cardbook (I am running the latest 52.6).

The bug still exists if I have a valid email address in the To field, I can have incomplete/partial addresses in CC or elsewhere which are subsequently lost on pressing Send.

My opinion is that auto-complete should not happen on blur/de-focus. Instead if I'm partially through typing an address, it should just attempt to send with whatever that may be. For example, "[e-mail]" should be sent exactly as that, not tried to complete to "Mr. Person <[e-mail]>" as I may have been specifically sending something to the "example.com" domain (not .au).

Good points! You could file a bug or RFE if you're unhappy with the current behaviour.

There appears to be some good points/discussions on this topic in this thread. I'll leave it for somebody else to open.

Comment 11

[Thomas D. (:thomas8)]

Are we doing something to get this bug with substantial covert dataloss potential out of the way? See my STR in comment 1.

Comment 12

[Alessandro Castellani [:aleca]]

I'll try to take care of this bug later this week.
I agree that the Send button should be disabled if we have non-pillified values in the addressing rows.

Comment 13

[Magnus Melin [:mkmelin]]

Great!

Comment 14

[Alessandro Castellani [:aleca]]

Attachment: 1674054-send-disable.diff

This patch disables the send button on typing, running the check only once if the button is currently enabled.

It seems that I'm not able to disable the Ctrl+Enter shortcut, which sends the message regardless by creating pills of whatever string is present in the field.
I thought the keyboard shortcut would be disabled alongside the send button.
Am I missing something?

Comment 15

[Magnus Melin [:mkmelin]]

Comment on attachment 9190706 [details] [diff] [review]
1674054-send-disable.diff

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

I think we need to figure out why it doesn't pillify in time onblur for these cases, which is likely the right solution. The patch looks like it wouldn't work for paste either.

Comment 16

[Alessandro Castellani [:aleca]]

Mhh...so here maybe the solution is to intercept the send action and go through these checks:

• Pillify all the leftover strings in the input fields.
• Check if any of those pill is invalid (prompt the user about it and interrupt the send).
• Send.

Does this approach sound good?

Comment 17

[Alessandro Castellani [:aleca]]

Attachment: 1674054-pill-before-send.diff

With this patch, all the leftover strings are pillified before sending.
The sending stops if there's an invalid address in the newly generated pills.

I didn't tackle the paste-and-convert-to-pill approach, which I'd like to handle in a dedicated bug as I think it needs a little bit more UX thinking to prevent unexpected behaviour.

Comment 18

[Magnus Melin [:mkmelin]]

Comment on attachment 9190891 [details] [diff] [review]
1674054-pill-before-send.diff

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

LGTM, r=mkmelin

::: mail/components/compose/content/MsgComposeCommands.js
@@ +4838,5 @@
> +      `input[is="autocomplete-input"][recipienttype]`
> +    );
> +    // If we find a leftover string in the input field, create a pill. If the
> +    // newly created pill is not a valid address, the sending will stop.
> +    if (input.value.trim().length) {

nit: don't need the .length as this is not an array

Comment 19

[Alessandro Castellani [:aleca]]

Attachment: 1674054-pill-before-send.diff

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/4859c8cc0e87
Convert leftover strings inside recipient inputs into pills before sending. r=mkmelin

Comment 21

[Magnus Melin [:mkmelin]]

Comment on attachment 9191141 [details] [diff] [review]
1674054-pill-before-send.diff

[Approval Request Comment]
Regression caused by (bug #): pills
User impact if declined: sending may leave out an address under certain conditions
Testing completed (on c-c, etc.): on c-c and beta
Risk to taking this patch (and alternatives if risky): not risky

Comment 22

[Wayne Mery (:wsmwk)]

Comment on attachment 9191141 [details] [diff] [review]
1674054-pill-before-send.diff

[Triage Comment]
Approved for esr78

Comment 23

[Rob Lemley [:rjl]]

Thunderbird 78.6.1:
https://hg.mozilla.org/releases/comm-esr78/rev/03df3fd6ee99