1720214 - `Escape` keypress or [x] click on no-reply alert confusingly triggers `Reply anyway`, and alert message needs improving

Status: VERIFIED FIXED

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

Description

[Thomas D. (:thomas8)]

+++ This bug was initially created as a clone of Bug #1342809 +++

There are several things which are wrong and odd about the new helpful alert when replying to no-reply addresses, introduced in bug 1342809.

STR

• Reply to a message from "[e-mail]", which will trigger the Reply not Supported alert.
• When the alert is showing:
  • Click Cancel button (and compare its position with other dialogs)
  • Press Escape on keyboard
  • Click [x] in alert corner

Actual results:

• Cancel button is on the left, violating mouse muscle memory (for Windows at least), because all Cancel buttons are on the right (plus we don't have a choice if we want to fix the behaviour, see below).
• Escape triggers Reply Anyway, making the alert a no-op for keyboard users, and very confusing.
• Clicking [x] to close the alert without action also triggers Reply Anyway.
• The wording is odd, as Henry already mentioned: What is a monitored address? Never heard of that, hard to translate, but if anything, it sounds like espionage... let's try to keep it simple, please...

Expected results:

• Cancel button should be in default position on the right.
• Escape and [x] must trigger the same action as Cancel.
• Improve the wording and avoid the term monitored address.

The confirmEx() alert misbehaviour is bug 345067 (which sucks), but we can easily work around that by reversing the button sequence so that Cancel button will be in position 1 and return 1, consistent with Escape and [x].

Comment 1

[Thomas D. (:thomas8)]

Attachment: 1720214_noReplyAlertFix.diff

This patch puts Cancel into the right position and makes the behaviour of Cancel, Escape, and [x] consistent. Also touch up the wording and add an access key to the Reply Anyway button.

New wording:

no-reply-alert-message = The reply address <{ $email }> does not appear to support replies.
    Messages to this address will probably not be read by anyone.

Example (see screenshot attachment 9230834 [details]):

The reply address <[e-mail]> does not appear to support replies.
Messages to this address will probably not be read by anyone.

Comment 2

[Thomas D. (:thomas8)]

Attachment: Screenshot 1: Before and after patch attachment 9230833

Top: before the patch.
Bottom: after the patch.

Comment 3

[Alessandro Castellani [:aleca]]

Comment on attachment 9230833 [details] [diff] [review]
1720214_noReplyAlertFix.diff

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

Great improvements.
But there are some nits that need fixing before landing this.

::: mail/locales/en-US/messenger/messenger.ftl
@@ +146,5 @@
>  repair-text-encoding-button =
>    .label = Repair Text Encoding
>    .tooltiptext = Guess correct text encoding from message content
>  
> +# Alert when replying to messages having a sender address like [e-mail]

This is not a correct fluent comment unfortunately.
We use the format `## Something something` as section header.
With this format, translators will always see the section header as a reference when they're translating all the IDs underneath it.

@@ +148,5 @@
>    .tooltiptext = Guess correct text encoding from message content
>  
> +# Alert when replying to messages having a sender address like [e-mail]
> +# $email (String): The reply address (as specified by sender) having 'no-reply'
> +#   substring which triggered the alert, e.g. [e-mail].

Wrong position. This should be right above the `no-reply-alert-message` ID.

Wrong format. We use this format for variables comment in fluent:
# Variables:
# $email (string) - The reply address (as specified by sender) having 'no-reply' substring which triggered the alert, e.g. [e-mail].

@@ +149,5 @@
>  
> +# Alert when replying to messages having a sender address like [e-mail]
> +# $email (String): The reply address (as specified by sender) having 'no-reply'
> +#   substring which triggered the alert, e.g. [e-mail].
> +no-reply-alert-title = Reply Not Supported

No need to update this ID as there's no real string change, otherwise translators will need to re-translate this.

Comment 4

[Magnus Melin [:mkmelin]]

Since we're past string freeze you shouldn't change the strings. (Which is why I slightly rushed it in in the first place).
I also disagree with the "does not seem to support replies" - what we have is correct, and it's not that uncommon (3rd hit on google for monitored address is about no-reply)

Comment 5

[Thomas D. (:thomas8)]

Attachment: 1720214_noReplyAlertFix.diff

Alright then, here is a no-strings-attached patch!

Comment 6

[Alessandro Castellani [:aleca]]

Comment on attachment 9231144 [details] [diff] [review]
1720214_noReplyAlertFix.diff

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

Great, thanks!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/66a36e0beed9
Fix wrong behaviour of no-reply alert for Escape and [x] due to confirmEx() limitations. r=aleca

Comment 8

[Rob Lemley [:rjl]]

Comment on attachment 9231144 [details] [diff] [review]
1720214_noReplyAlertFix.diff

[Approval Request Comment]
Regression caused by (bug #): 1342809
User impact if declined: Confusing UX for new feature
Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky):
Low, Moves buttons to a more natural order and fixes keyboard action.

Comment 9

[Wayne Mery (:wsmwk)]

Comment on attachment 9231144 [details] [diff] [review]
1720214_noReplyAlertFix.diff

[Triage Comment]
Approved for beta

Comment 10

[Dave Miller [:justdave]]

Thunderbird 91.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/79bd069b42cd

Comment 11

[WaltS48 [:walts48]]

Tested in 92.0a1 and 91.0b3 on Windows 10.