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)
Tracking
(Not tracked)
People
(Reporter: jorgk-bmo, Assigned: jorgk-bmo)
References
Details
(Keywords: regression)
Attachments
(4 files)
38.28 KB,
image/png
|
Details | |
1.63 KB,
image/png
|
Details | |
5.09 KB,
patch
|
Details | Diff | Splinter Review | |
2.64 KB,
patch
|
aceman
:
review+
|
Details | Diff | Splinter Review |
+++ 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?
Assignee | ||
Updated•5 years ago
|
Comment 1•5 years ago
|
||
STR
- Right click on a message in thread pane
- 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
Assignee | ||
Comment 2•5 years ago
|
||
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.
Comment 3•5 years ago
|
||
(In reply to Jorg K (GMT+2) from comment #2)
Created attachment 9069939 [details]
wrong-icon.pngThanks, 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
- Right click on a message in thread pane of Local Folder
- Choose "Forward As..." > "Attachment"
I got same regression window of comment#1
Assignee | ||
Comment 4•5 years ago
|
||
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 | ||
Comment 5•5 years ago
|
||
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.
Assignee | ||
Comment 6•5 years ago
|
||
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.
Assignee | ||
Comment 7•5 years ago
|
||
Damn, I left the debug in, well, Aceman can play with it.
Assignee | ||
Comment 8•5 years ago
|
||
Assignee | ||
Comment 9•5 years ago
|
||
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.
Assignee | ||
Comment 10•5 years ago
|
||
Comment on attachment 9070066 [details] [diff] [review] 1556981-no-message-urls.patch I'll leave this here for reference.
Assignee | ||
Comment 11•5 years ago
|
||
Comment 12•5 years ago
|
||
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.
Assignee | ||
Comment 13•5 years ago
•
|
||
Use a real OS ;-) - Did you ever get a EDIT: bug good icon like in attachment 9069929 [details], I mean in TB 60/68?
Comment 14•5 years ago
|
||
No. I get the text document icon on TB60 so I'm fine :)
Comment 15•5 years ago
|
||
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
Assignee | ||
Updated•5 years ago
|
Description
•