1664733 - After correcting an invalid recipient address with 'Edit Address', "Send" button fails to become enabled if there are no other recipients with @, and no prompt to save changes

Status: RESOLVED FIXED

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

Description

[gene smith]

STR:

• Open a new write window
• Enter an invalid recipient address such as broken.
• Enter a subject and type some body text
• Notice that "Send" button is not enabled (as expected).
• Without completely erasing it, correct the address to [e-mail].
• Notice that "Send" button is still not enabled.

Correcting the address should cause the "Send" button to become enabled but it doesn't. A way to enable the "Send" button is to completely erase the address and re-enter a valid address again.

This only occurs with 78 and personal trunk builds. With 68, immediately correcting the invalid address causes the "Send" button to become enabled and no full erasure of the address is needed.

Comment 1

[Thomas D. (:thomas8)]

Thanks Gene, excellent find! Annoying for those affected.
Incorrect address must be the only recipient (no other correct addresses), and fails for correction via "Edit Address".
Only deleting and recreating the entire pill re-enables the send button.

Alex, can you check this out? Should fix this fast for release.

Reverse case: I'm surprised that localpart@server (without domain) gets enabled for sending. Are there still local server scenarios where that can succeed to send?

Comment 2

[gene smith]

I noticed this while working on Bug 1563891.

Also, if you initially enter a valid address like gds@localhost and then edit it back to invalid gds, the Send button remains enabled. I think it should become disabled. It does for 68.
But with 78, if you still try to send to the bad address gds, you get the alert:
"gds <> is not a valid e-mail address because it is not of the form user@host. You must correct it before sending the e-mail."
So sending to the bad address is still inhibited.

Reverse case: I'm surprised that localpart@server (without domain) gets enabled for sending. Are there still local server scenarios where that can succeed to send?

Seems like gds@localhost might be a possible valid example of this. (I assume that by "with domain" you mean [e-mail] ?)

Comment 3

[Alessandro Castellani [:aleca]]

Ah, really good discovery, thank you for this.
I'll have a patch to review soon.

Comment 4

[Alessandro Castellani [:aleca]]

Attachment: 1664733-pill-edit-send.diff

Comment 5

[Thomas D. (:thomas8)]

Comment on attachment 9175571 [details] [diff] [review]
1664733-pill-edit-send.diff

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

Awesome, two bugs squashed with one stroke by the master of pills! :-)) This fixes not just the send button, but also ensures that correcting a recipient address is now preserved/asked for via gContentChanged when closing draft. Before, such corrections would just silently disappear.

Comment 6

[Thomas D. (:thomas8)]

Alex, pls request uplifts as appropriate.

Comment 7

[Thomas D. (:thomas8)]

(In reply to gene smith from comment #2)

Also, if you initially enter a valid address like gds@localhost and then edit it back to invalid gds, the Send button remains enabled. I think it should become disabled. It does for 68.

Both cases covered by Alex patch, and we're also preventing dataloss of address corrections when draft gets closed.

Seems like gds@localhost might be a possible valid example of this. (I assume that by "with domain" you mean [e-mail] ?)

Ah, ok. Yes.

Comment 8

[Alessandro Castellani [:aleca]]

I run the tests related tot he send button locally and they all pass, but since we're touching that section, better launch a full try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=57d912630004e201a9f3e2842022fc98b79de5dd

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fa6b284953df
Update the Send button after a pill is edited in the message compose window. r=thomas8

Comment 10

[Alessandro Castellani [:aleca]]

Comment on attachment 9175571 [details] [diff] [review]
1664733-pill-edit-send.diff

[Approval Request Comment]
Regression caused by (bug #): -
User impact if declined: If a user types an incorrect address and then edits the pill, the new address won't enable/disable the send button accordingly, forcing the user to delete and recreate the pill.
Testing completed (on c-c, etc.): on c-c
Risk to taking this patch (and alternatives if risky): low

Comment 11

[Wayne Mery (:wsmwk)]

Comment on attachment 9175571 [details] [diff] [review]
1664733-pill-edit-send.diff

[Triage Comment]
Approved for esr78 (even though it hasn't gone through beta - but let's remember to test it)
No more betas for 81

Comment 12

[Rob Lemley [:rjl]]

Thunderbird 78.3.0:
https://hg.mozilla.org/releases/comm-esr78/rev/c0dd4e7a6408

Comment 13

[WaltS48 [:walts48]]

Testing 78.3.0 rc on Windows10.

If I type "broken" and hit Enter to create a pill, the Send button remains disabled.
Leaving it there, when I enter a valid address creating a pill, the Send button becomes enabled.
Entering a valid address and creating a pill first enables the Send button, then typing "broken" and creating a pill, the button remains enabled.

If I try to send a message with the broken pill I get an "Invalid Recipient Address" error message telling me to correct it before sending the email.

Comment 14

[gene smith]

(In reply to WaltS48 [:walts48] from comment #13)
Looks like all the pills have to have invalid addresses for the Send button to become disabled. E.g., if you have 3 pills and only the last one has valid user@domain then Send is enabled. Looks like 68 works the same way with multiple "To:" lines corresponding to pills.
With both 68 and 78 if you attempt to send with any bad address, you get the pop-up about the first bad address and the message isn't sent to the SMTP server until all are in the format user@domain.
I don't see this as a big problem. Maybe ideally the send button should remain disabled if one of the many entered addresses is not in a valid format, but this would be a new feature compared to 68 and probably earlier.

Comment 15

[Alessandro Castellani [:aleca]]

Ah, good catch, thanks for reporting this.
We could consider a follow up enhancement to keep the Send button disabled if at least one address is invalid.

Comment 16

[WaltS48 [:walts48]]

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

Ah, good catch, thanks for reporting this.
We could consider a follow up enhancement to keep the Send button disabled if at least one address is invalid.

I think that might generate "I can't send email because the Send button is disabled" support/bug reports for users that don't know they have a mistake in an address.

The error message would make them look for a mistake.

Comment 17

[Thomas D. (:thomas8)]

(In reply to WaltS48 [:walts48] from comment #16)

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

Ah, good catch, thanks for reporting this.
We could consider a follow up enhancement to keep the Send button disabled if at least one address is invalid.

I think that might generate "I can't send email because the Send button is disabled" support/bug reports for users that don't know they have a mistake in an address.

The error message would make them look for a mistake.

+1. Please leave it as is, the explicit error message if only one out of many addresses is wrong will spare us from many bug reports.

Comment 18

[gene smith]

I think that might generate "I can't send email because the Send button is disabled" support/bug reports for users that don't know they have a mistake in an address.

The error message would make them look for a mistake.

Again, not a big problem. But by the same argument you might just enable the send button no matter what is in any pill and let the error message pop-up be the only means to tell the user about a bad address.