User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

This happens with TB 64 Bit 78.2 <=78.2.3 on Windows 10 64 Bit.

1. Add an attachment (it now has the icon of the Windows file type).
2. Convert the attachment to an Filelink attachment (it now has the Filelink provider's icon).
3. Add another attachment

Actual results:

The icon of the first attachment changes back to Windows' file type icon.

Expected results:

The icon should remain the Filelink providers icon.

This still happens in Thunderbird 91 and also on Ubuntu 20.

(minor so somewhat unclear if warranted for 91)

It is a visual annoyance and only needs a minor unproblematic patch. Let's get this on ESR, but let it right the train.

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/73a0f03a037d
Do not forget cloud icons during refresh. r=mkmelin

Can we back this out? I did not know that the loaded flag was not only used by cloudFile in compose but also while fetching emails. The fix must look different and having the regression fix ontop of this one, is bad for the history / readability. I also want to add some tests which will take a bit of time and do not want this the regression to exist for so long in the product.

Sure, will back it out when there's a merge

Backout by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/eba5ece82c4e
Backed out changeset 73a0f03a037d for causing bug 1741195. r=backout

Did you test to see how this works with a external http attachment? The comment states why the loading indicator was put where it was put.

I have no idea what comment you are talking about. Feel free to add more tests like I did with this patch, as this section is/was heavily missing tests.

Without a clue what you mean, I did test to attach web pages and that looked ok.

This comment, right above code you removed:

// Set elements busy to show the user this is potentially a long network
// fetch for the link attachment.

I presume you know how to link those comment lines to this bug, so you can understand the usage and testcases there.

Your defensive tone is unwarranted; the goal here is to avoid regressions. This bug has already been backed out once, and it's disappointing to see this project operating on a death by a thousand bug reports strategy. Finally, do not ask others to do work for you.

I did not remove that code, I moved it into refreshAttachmentIcon() which is now called by setAttachmentLoaded().

And it was your tone which I found unwarranted.

Pushed by thunderbird@calypsoblue.org:
https://hg.mozilla.org/comm-central/rev/6ab422bc28d9
Handle cloud icons separately and improve state awareness of refreshAttachmentIcon(). r=mkmelin

Looks like test failure @ https://treeherder.mozilla.org/logviewer?job_id=358883647&repo=comm-central&lineNumber=3340

(In reply to alta88 from comment #10)

Did you test to see how this works with a external http attachment? ,,,

See attachment "test" in bug 1284004.

It looks like I deliberately changed the tested file to force the test to fail, to check whether the test is actually run or not. Sorry that it slipped into the final patch.

I've tested both detached and external attachments (test case from bug 1284004). Both appear to work, however, there's something wrong with the highlight/focus (background) colors, the result can be white on white or blue on blue (on Windows).

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/759e39ffe7ca
Fix typo in browser_repeat_upload.js and check the correct file. r=mkmelin

(In reply to newsfan from comment #19)

I've tested both detached and external attachments (test case from bug 1284004). Both appear to work, however, there's something wrong with the highlight/focus (background) colors, the result can be white on white or blue on blue (on Windows).

Thanks for your help. Does the unreadable situation occur for you only when hovering over the selected and active/focused attachment?

Yes, well, sort of.
"Link" attachment (external or detached): Background goes blue if selected attachment is not hovered --> blue on blue.
Normal attachment: Background goes white if selected attachment is hovered --> white on white.
Do you see that as well? I can (or you can) file a bug if there isn't one already (didn't check).

Yes, I see that as well. Moz-regression pointed to
https://phabricator.services.mozilla.com/D131187

But I am not 100% sure that this is true, but it does not seem to be a regression caused by this bug. Filing a new bug would be great, thanks.

Bug 1742567. I didn't imply that the regression was caused here. Are you sure about the result of moz-regression? That's a total backend/C++ change unrelated to theming/CSS.

I didn't imply that the regression was caused here

This patch of course could have been the cause for it, so I checked and made sure it isn't. Your reporting was totally fine.

Are you sure about the result of moz-regression?

No, I am not sure. It could also be some change in mozilla-central, which landed during the same time. We will have to investigate. But let us move that discussion to Bug 1742567.

Comment on attachment 9251518 [details]
Bug 1667652 - Handle cloud icons separately and improve state awareness of refreshAttachmentIcon(). r=mkmelin

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined:
Missing out the improved icons of cloud files.

Testing completed (on c-c, etc.):
Baked on 96 for more than 3 weeks.

Risk to taking this patch (and alternatives if risky):
Low.

Comment on attachment 9252031 [details]
Bug 1667652 - Fix typo in browser_repeat_upload.js and check the correct file. r=mkmelin

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined:
Missing out the improved icons of cloud files.

Testing completed (on c-c, etc.):
Baked on 96 for more than 3 weeks.

Risk to taking this patch (and alternatives if risky):
Low.

Comment on attachment 9252031 [details]
Bug 1667652 - Fix typo in browser_repeat_upload.js and check the correct file. r=mkmelin

[Triage Comment]
Approved for esr91

Comment on attachment 9251518 [details]
Bug 1667652 - Handle cloud icons separately and improve state awareness of refreshAttachmentIcon(). r=mkmelin

[Triage Comment]
Approved for esr91

Thunderbird 91.4.1:
https://hg.mozilla.org/releases/comm-esr91/rev/92e1d805bd7a

Thunderbird 91.4.1:
https://hg.mozilla.org/releases/comm-esr91/rev/11529576c3a7