Error: Couldn't open external attachment!
Source file: chrome://messenger/content/msgHdrViewOverlay.js
this is an invalid throw; nntp or feed attachments, or mail attachments that have been moved trigger this, wrongly.
plus, it doesn't seem this, even if valid, would help a user in any way.
It's not actually throwing, just reporting to the console. I'm not opposed to removing it, but it's not hurting anything. Patches welcome.
the console pollution is, in fact, a detriment and distraction to your fellow developers and alarming to any user who happens upon it. first patch? it seems better manners for the author of this 'work' to take the 30 seconds and clean up after himself.
forgot to add ;)
Created attachment 617205 [details] [diff] [review]
FTR, normal nntp attachments seemed fine to me.
Ugh, and without the trailing "live"
if an email attachment has been detached, and then moved/deleted, it's incorrect to raise an error simply because the original file is not there. since it cannot be determined if this is a real error or user action, it seems the whole premise is not right.
I think the message would be useful for people who didn't intend to delete their external attachment. How about downgrading it to a warning, and also indicating the name of the attachment we failed on?
the number of people who did that and went to look in Tb console is next to 0.
the proper way to indicate this to a user is to check the existence of an external attachment, file:// protocol, on local disk at attachment list creation time and set an attribute for styling if it's not found (X icon or strikethrough eg), and/or a tooltiptext stating the original detached location no longer exists. but not send it to console.
Yeah i agree we should have a proper ui for those cases.
So the current patch would only log to the console if we can't open for unknown reasons. I'm not sure what those would be, but i didn't dare to loose the try/catch completely.
Comment on attachment 617205 [details] [diff] [review]
Review of attachment 617205 [details] [diff] [review]:
I've thought about this, and I think this method is ok. I'm not sure we'd ever hit the catch block though, so I'd be ok with not reporting the error too.