1653814 - Placement of the reply-to field in email composer in Thunderbird 78

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, enhancement)

Description

[Kai Engert [:KaiE:]]

I used "reply all" to an email that has a "reply-to" field.

When that email opens, it will show two entry fields:

• To
• Reply-To

I wanted to add someone to CC.

My muscle memory from past Thunderbird versions has caused me to quickly click that second field, and add the person to it. I've added it to the "Reply-To" field without thinking. I realized my mistake only after I had already hit send.

I think the reply-to field shouldn't be shown last.

Reply-to is a property related to the sender. I think it should be shown closer to the sender address, ABOVE the To/CC/BCC fields.

Comment 1

[Alessandro Castellani [:aleca]]

Thanks for raising this issue.
There have been discussions somewhat related to this in order to improve the revealing of addressing rows.

Right now those rows are hardcoded in the HTML file, so no matter in which order you open them, they always appear in the same order.
The discussed idea was to generate those on the fly when the user decides to show them, therefore respecting the order in which they're generated.

What do you think?
Other email clients always have the same order of fields if I'm not wrong, so I'm not sure that idea would be well received by users.
Do you think it's better to stick with the "expected" ordering?

• From
• To
• Reply To
• Cc
• Bcc
• Others (Newsgroups and custom headers)

I'd consider this an enhancement more than a defect.

Comment 2

[Kai Engert [:KaiE:]]

I'd personally prefer "always the same order".

The order you gave in comment 1 is what I'd expect.

Comment 3

[Thomas D. (:thomas8)]

(In reply to Kai Engert (:KaiE:) from comment #2)

I'd personally prefer "always the same order".

The order you gave in comment 1 is what I'd expect.

Mmmmh, you said in comment 0 (sic; aka description) that you want:

• From / Reply-To ("Reply-to related to sender, should be ABOVE the To/Cc/Bcc fields")
• To / CC / BCC
• Others

And you explicitly said that you find the following confusing:

• To
• Reply-To

So the order mentioned by Alex in comment 1 is clearly not what you want, unless if you have changed your mind from comment 0.
I think Kai has a point that From and Reply-To belong together logically, and this could be explored with mockups.
I'm not sure if having Reply-To further up as Kai proposes is not going to re-create Kai's problem for other users who expect recipients to come immediately after sender, and Reply-to last.

Currently, this works as designed, so no defect here.

Comment 4

[Thomas D. (:thomas8)]

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

• From (sender)
• To (recipient)
• Reply To (sender)
• Cc /Bcc (recipient)

I think that order would be worse because it's now an alternating mix of sender-related fields and recipient fields.

Comment 5

[Thomas D. (:thomas8)]

(In reply to Kai Engert (:KaiE:) from comment #0)

I used "reply all" to an email that has a "reply-to" field.

When that email opens, it will show two entry fields:

• To
• Reply-To

I am failing to understand your STR/Actual Result, and I don't think that is what happens.
If the original email (which you are replying to) has Reply-to, that should become To when replying, isn't it? Am I missing something?

Comment 6

[Kai Engert [:KaiE:]]

Apologies, I confirm that my reply in comment 2 was incorrect. I had not reviewed Alex's comment 1 sufficiently.

I would prefer the following order:

• From
• Reply To
• To
• Cc
• Bcc
• Others (Newsgroups and custom headers)

Comment 7

[Alessandro Castellani [:aleca]]

Attachment: 1653814-recipients-order.diff

Just a simple repositioning of the xhtml container.
Asking Thomas for a review since he worked on this area as much as I did.

I also launched a try-run to be sure we're not breaking any keyboard navigation test: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=70296c6e8734e097d7deed9394a78e424c8d900d

Comment 8

[Magnus Melin [:mkmelin]]

Reply-to is a bit special. I'm really not sure we want to move it.
It's special in that if you have a pre-set one, then that is basically taking up extra space for no gain. Seems many other email clients simply do not show the information in the compose UI when this is the case.

But it has the use case of setting reply-to to something different, one a case-by-case basis. Then it seems more appropriate to have further down like we've traditionally had it.

Comment 9

[Alessandro Castellani [:aleca]]

I personally never used that field, so I'm not the most educated to propose an approach.
Would be worth pulling in some users that rely on that field a lot?
What would make more sense semantically?

Comment 10

[Magnus Melin [:mkmelin]]

Semantically Kai's right that it's related to the sender identity, at least for the auto Reply-To obtained from the identity settings.

I think it might make sense to not show an auto Reply-To row at all in the UI (as it's just clutter), but then if the header is added from the overflow bar then it would be pre-filled into the shown field.

Comment 11

[Thomas D. (:thomas8)]

Kai, could you pls correct your STR (see my comment 5)?

Comment 12

[Magnus Melin [:mkmelin]]

Same hiding would also apply to auto-cc and auto-bcc

Comment 13

[Thomas D. (:thomas8)]

Comment on attachment 9171959 [details] [diff] [review]
1653814-recipients-order.diff

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

LGTM. I think this is a reasonable change. F6 fast focus ring now skips reply-to (not sure why tests don't show that) - TB 68 includes all addressing fields. r+ with that fixed. (Ctrl+Shift+Tab was broken before this patch, so we can do that in another bug).

STR aren't clear but I think we're talking about Auto-Reply-To from account settings. In TB 68, if you have more than one auto-reply-to address, you'll get this:

**TB 68**
```
- From
- Auto-Reply-To 1
- Auto-Reply-To 2 etc.
- Auto-CC/BCC
- To
```

This patch restores same traditional order as in TB 68 wrt auto-reply-to, whilst *reducing* the space needed for more than one auto-reply-to:

**TB 78 with patch**
```
- From
- {Auto-Reply-To 1} {Auto-Reply-To 2}
- To
- Auto-CC/BCC
``` 
Imho the idea of just hiding Auto-Reply-To and Auto-CC/BCC fields away would be very delicate behaviour wrt privacy, ux-wysiwyg, and ux-error-prevention. So that would need more thoughts in another bug.
At least we'd need some sort of trace in the UI, but full transparency looks good and correct. With TB 78 release immediately pending, this doesn't look like the right time to experiment and invest developer time into redesigning this. The patch as-is is a safe and risk-free improvement.

Comment 14

[Alessandro Castellani [:aleca]]

I suggest to move the reply-to below the to field since it's semantically correct and it respects the pre-filling fields currently happening in 68.
I agree that we should deal with these auto-fills in a slightly less intrusive way, as suggested by Magnus, but we should do it in a follow up bug.

Comment 15

[Magnus Melin [:mkmelin]]

Sure.

Comment 16

[Alessandro Castellani [:aleca]]

Attachment: 1653814-recipients-order.diff

Patch updated to fix the F6 keyboard navigation.
I had to change a couple of things around, so here's another try-run to see if I broke any test: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ccfd3eb1435b6a95a1f2a9a1ac076cef710c6510

Comment 17

[Alessandro Castellani [:aleca]]

Super green try run!

Comment 18

[Alessandro Castellani [:aleca]]

Pinging Thomas for this review.

Comment 19

[Thomas D. (:thomas8)]

Comment on attachment 9172473 [details] [diff] [review]
1653814-recipients-order.diff

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

Nice cleanup! LGTM, this does what it says (existing bugs notwithstanding). One polish nit.

::: mail/components/compose/content/MsgComposeCommands.js
@@ -7818,5 @@
>      // Backwards focus ring: e.g. Ctrl+Shift+Tab | Shift+F6
>      switch (focusedElement) {
>        case document.getElementById("newsgroupsAddrInput"):
> -        SetFocusOnPreviousAvailableElement(focusedElement);
> -        break;

Nice cleanup here and below. Good first step towards a more radical cleanup along bug 1612761.

@@ +7863,5 @@
> +      // Move the focus on the first available recipient field.
> +      document
> +        .getElementById("recipientsContainer")
> +        .querySelector(".address-row:not(.hidden)")
> +        .querySelector(`input[is="autocomplete-input"][recipienttype]`)

Let's simplify the last selector and combine the last two with descendant combinator (space):
```
        .querySelector(
          ".address-row:not(.hidden) .address-input[recipienttype]"
        )
```

Comment 20

[Alessandro Castellani [:aleca]]

Attachment: 1653814-recipients-order.diff

Comment 21

[Thomas D. (:thomas8)]

Comment on attachment 9173687 [details] [diff] [review]
1653814-recipients-order.diff

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

Hmmm, I think you broke it now.

::: mail/components/compose/content/MsgComposeCommands.js
@@ +7867,5 @@
> +    case document.getElementById("msgIdentity"):
> +      // Move the focus on the first available recipient field.
> +      document
> +        .getElementById("recipientsContainer")
> +        .querySelector(".address-row:not(.hidden)  .address-input")

Yeah, it's tempting and I tried that, too, but using `.address-input` without `[recipienttype]` attribute failed in my tests if there are pills with their own .address-input. That's why I requested `.address-input[recipienttype]`

Comment 22

[Alessandro Castellani [:aleca]]

Ah damn, that's right.
I completely forgot about that!

Comment 23

[Thomas D. (:thomas8)]

Removing checkin-needed per comment 21.

Comment 24

[Alessandro Castellani [:aleca]]

Attachment: 1653814-recipients-order.diff

Fixed.

Comment 25

[Thomas D. (:thomas8)]

Comment on attachment 9173763 [details] [diff] [review]
1653814-recipients-order.diff

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

Let's remove one extra space in css descendant combinator and we're good!

::: mail/components/compose/content/MsgComposeCommands.js
@@ +7868,5 @@
> +      // Move the focus on the first available recipient field.
> +      document
> +        .getElementById("recipientsContainer")
> +        .querySelector(
> +          ".address-row:not(.hidden)  .address-input[recipienttype]"

This will probably work because descendant combinator can take any whitespace*, but a *single* space between selectors would be sufficient ;-) Sorry for nitpicking, but you clearly caused this yourself! :-p

*https://developer.mozilla.org/en-US/docs/Web/CSS/Descendant_combinator

Comment 26

[Alessandro Castellani [:aleca]]

Attachment: 1653814-recipients-order.diff

Ah, just a typo.

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1854e4717c16
Reorder recipient fields in message compose window. r=thomas8

Comment 28

[Magnus Melin [:mkmelin]]

Comment on attachment 9173898 [details] [diff] [review]
1653814-recipients-order.diff

[Approval Request Comment]
Regression caused by (bug #): pills
User impact if declined: non ideal placement of reply-to

Comment 29

[Wayne Mery (:wsmwk)]

Comment on attachment 9173898 [details] [diff] [review]
1653814-recipients-order.diff

[Triage Comment]
Approved for beta

Comment 30

[Rob Lemley [:rjl]]

Thunderbird 81.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/d7cbd90ccc4d

Comment 31

[Wayne Mery (:wsmwk)]

Comment on attachment 9173898 [details] [diff] [review]
1653814-recipients-order.diff

[Triage Comment]
Approved for esr78

Comment 32

[Rob Lemley [:rjl]]

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