Closed Bug 1556981 Opened 5 years ago Closed 5 years ago

When attaching a message or forwarding a message as attachment, the attached message doesn't show the correct icon in the attachment pane in the compose window

Categories

(Thunderbird :: Message Compose Window, defect)

x86_64
macOS
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 69.0

People

(Reporter: jorgk-bmo, Assigned: jorgk-bmo)

References

Details

(Keywords: regression)

Attachments

(4 files)

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

When attaching a message or forwarding a message as attachment, the attached message doesn't show the correct icon in the attachment pane in the compose window.

It should be a TB icon, either an egg of Daily or the standard blue icon. Now it's showing a generic Windows icon.

This is working in TB 68, so this regressed in the last few weeks.

Alice, can you please find it for us?

Flags: needinfo?(alice0775)
Attached image screenshot BAD vs GOOD

STR

  1. Right click on a message in thread pane
  2. Choose "Forward As..." > "Attachment"

Actual Results:
See screenshot BAD vs GOOD

  • There are missing some destination rows
  • Destination input field(To:) does not get the focus when the composing window is open
  • It needs click on generic icon to see attachments list
  • There are no title(number of attachments and size)

And Error shows in error console:
NS_NOINTERFACE: Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsISupports.QueryInterface] stack-trace-collector.js:75

[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIIOService.newURI]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: chrome://messenger/content/messengercompose/MsgComposeCommands.js :: AddAttachments :: line 4408" data: no] MsgComposeCommands.js:4408:29
AddAttachments chrome://messenger/content/messengercompose/MsgComposeCommands.js:4408
ComposeStartup chrome://messenger/content/messengercompose/MsgComposeCommands.js:2911
ComposeLoad chrome://messenger/content/messengercompose/MsgComposeCommands.js:3035
onload chrome://messenger/content/messengercompose/messengercompose.xul:1

NS_ERROR_NOT_AVAILABLE: Cannot call openModalWindow on a hidden window Prompter.jsm:345

Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=ac121e4bba12a4b47de324c7d0653124ff8d3cb8&tochange=69ac929f4cbbe773fa321c0d501bb7731dcad62f
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3a79d3be67486be1d30bda47988cf8c76ee3a3ee&tochange=2bb77ed1fcc5ad06f91612d419160f54c09369db

Flags: needinfo?(alice0775)
Attached image wrong-icon.png

Thanks, Alice. I think you found the regression from bug 1556242. That's fixed already and only happened when using IMAP folders.

For the incorrect icon, please use a message from a local folder. That always worked, but started showing the incorrect icon.

Flags: needinfo?(alice0775)

(In reply to Jorg K (GMT+2) from comment #2)

Created attachment 9069939 [details]
wrong-icon.png

Thanks, Alice. I think you found the regression from bug 1556242. That's
fixed already and only happened when using IMAP folders.

For the incorrect icon, please use a message from a local folder. That
always worked, but started showing the incorrect icon.

OK
STR

  1. Right click on a message in thread pane of Local Folder
  2. Choose "Forward As..." > "Attachment"

I got same regression window of comment#1

Flags: needinfo?(alice0775)

Thanks for confirming the regression range, that's useful since it rules out de-XBL changes as a cause. I vaguely remember fixing those icons before in bug 1427265.

Assignee: nobody → jorgk
Status: NEW → ASSIGNED

So the code is here:
https://searchfox.org/comm-central/rev/66bdc4565824a8c59d34ba88c3d8e70448356a71/mail/components/compose/content/MsgComposeCommands.js#4412

I added some debugging and saw that now we appear to have a proper nsIURL and the first part of the if triggers.

What happens is that the *-message URLs now create a proper mailbox/imap/news URL and those are nsIURLs.

The alternative change would be to remove the *-message URLs from here:
https://searchfox.org/comm-central/rev/4a11fb532256bbd0901758d25ec6a79fb2d87afd/mailnews/base/util/nsNewMailnewsURI.cpp#36
It would then go back to M-C and that would fall-through to creating a "simple URL" which may or may not be an nsIURL. I'll try it. Then we could also revert the patch from bug 1556242.

OK, let's of with this. As per bug 1556242 comment #11 the *-message URLs never ran through the (old) protocol handlers, so they were just some sort of cheap URL to address messages.

The patch restores that behaviour.

Attachment #9070066 - Flags: review?(acelists)

Damn, I left the debug in, well, Aceman can play with it.

Sigh, which this change, mailnews/news/test/unit/test_uriParser.js now fails since that test does mailnews-y things with the a news-message URL. I can't win here.

So the alternative fix is to reshuffle the conditions.

Comment on attachment 9070066 [details] [diff] [review]
1556981-no-message-urls.patch

I'll leave this here for reference.
Attachment #9070066 - Flags: review?(acelists)
Attachment #9070189 - Flags: review?(acelists)
Comment on attachment 9070189 [details] [diff] [review]
1556981-reshuffle-icon.logic.patch

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

I'm not getting  TB icon but a plain text document icon, but better than nothing as without the patch. Thanks.
Attachment #9070189 - Flags: review?(acelists) → review+

Use a real OS ;-) - Did you ever get a EDIT: bug good icon like in attachment 9069929 [details], I mean in TB 60/68?

No. I get the text document icon on TB60 so I'm fine :)

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/2600d6fff1bf
Reshuffle icon logic for attachments since *-message URLs are mailnews URLs now. r=aceman

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 69.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: