1656666 - Mail compose: when changing From: field, CC & BCC buttons switch their order

Status: VERIFIED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Andreas]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/84.0.4147.105 Safari/537.36 Edg/84.0.522.50

Steps to reproduce:

I have 4 mail identities in Thunderbird.
Open a new mail compose window. Don't activate CC and BCC fields (keep the buttons next to the From: field). Choose a different From: identity.

Actual results:

CC and BCC buttons switch their position with every change of the mail identity.
From: field CC BCC >>
< change mail identity >
From: field BCC CC >>
The order of the buttons isn't related to a certain identity. If you switch from identity 1 to 2 to 3 back to 1, you end up with a different order for identity 1.

Expected results:

Buttons should always keep their order.

Comment 1

[Wayne Mery (:wsmwk)]

related to bug 1635124 ?

Comment 2

[Thomas D. (:thomas8)]

Confirming exactly as described.

(In reply to Wayne Mery (:wsmwk) from comment #1)

related to bug 1635124 ?
No, this bug is also happening with mouse, so premature application of identity when keyboard navigating the From dropdown is not involved here.
We should fix this because it's really weird for CC/BCC buttons to change their location without apparent reason.

Comment 3

[Thomas D. (:thomas8)]

Alex, do you want a regression window for this? If yes, pls set regressionwindow-wanted keyword.

Comment 4

[Jorg K (CEST = GMT+2)]

Has this regressed or was it always like this after the pills landed?

Comment 5

[Alessandro Castellani [:aleca]]

This is weird, those buttons are statically written in the XHTML file and should simply be hidden/showed based on the type of identity (mail, nntp).
I never experienced this issue while building it, so it might be a regression, but I'm not 100% sure.
I'll investigate this to understand the reason of this issue and I'll know if it's a regression or something we've been having since day 1 of the pills implementation.

Comment 6

[Alessandro Castellani [:aleca]]

Attachment: 1656666-compose-buttons.diff

This bug seems to have always been present since the first implementation.
Good find.

Comment 7

[Alessandro Castellani [:aleca]]

The better solution would be to not run the updateUIforIMAPAccount at all if the previous and current account type match the same recipient.
We currently only check for the nntp account type when deciding if a UI update is necessary.
What's the full list of all the account types we handle?
Is it a finite list or it might stumble upon some obscure protocols we don't know?

Comment 8

[Magnus Melin [:mkmelin]]

updateUIforIMAPAccount is wrongly named, it could be any mail type. We have at least imap,movemail,pop3,none,rss,nntp (not all have compose capabilities), and then there are types from extensions such as "exchange"

Comment 9

[Magnus Melin [:mkmelin]]

Comment on attachment 9167760 [details] [diff] [review]
1656666-compose-buttons.diff

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

::: mail/components/compose/content/addressingWidgetOverlay.js
@@ +308,3 @@
>    for (let label of document.querySelectorAll(".mail-label")) {
> +    // Always keep the overflow button last.
> +    extraRecipients.insertBefore(label, extraRecipientsLabel);

doesn't seem you should keep inserting this inside the loop

Comment 10

[Alessandro Castellani [:aleca]]

Attachment: 1656666-compose-buttons.diff

All right, this should do it.
I renamed the method as updateUIforMailAccount and excluded the extra recipients overflow button from the reordered labels.

Comment 11

[Magnus Melin [:mkmelin]]

Comment on attachment 9168219 [details] [diff] [review]
1656666-compose-buttons.diff

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

Looks good, thx! r=mkmelin

Comment 12

[Alessandro Castellani [:aleca]]

Comment on attachment 9168219 [details] [diff] [review]
1656666-compose-buttons.diff

[Approval Request Comment]
Regression caused by (bug #): -
User impact if declined: Users with multiple identities will experience an order change of the Cc and Bcc labels when changing identity in the compose window.
Testing completed (on c-c, etc.): soon on c-c
Risk to taking this patch (and alternatives if risky): low as the changes are very minimal

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b27e202977b9
Fix Cc and Bcc buttons changing position when switching identity in compose dialog. r=mkmelin

Comment 14

[Wayne Mery (:wsmwk)]

Comment on attachment 9168219 [details] [diff] [review]
1656666-compose-buttons.diff

[Triage Comment]
Approved for beta

Comment 15

[Rob Lemley [:rjl]]

Thunderbird 80.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/110ff8e02b0c

Comment 16

[WaltS48 [:walts48]]

Looks fixed in my testing of the 80.0b2 release candidate on Ubuntu 18.04.4.

Comment 17

[Wayne Mery (:wsmwk)]

Comment on attachment 9168219 [details] [diff] [review]
1656666-compose-buttons.diff

[Triage Comment]
Approved for esr78

Comment 18

[Rob Lemley [:rjl]]

Thunderbird 78.1.2:
https://hg.mozilla.org/releases/comm-esr78/rev/800a52e6d751