Closed Bug 746450 Opened 10 years ago Closed 10 years ago

remove console noise for external attachments that cannot be found

Categories

(MailNews Core :: Attachments, defect)

defect
Not set
trivial

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 15.0

People

(Reporter: alta88, Assigned: mkmelin)

References

Details

(Whiteboard: [good first bug])

Attachments

(1 file)

Error: Couldn't open external attachment!
Source file: chrome://messenger/content/msgHdrViewOverlay.js
Line: 610

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.
Severity: minor → trivial
Whiteboard: [good first bug]
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 ;)
Summary: Remove invalid attachment throw → remove console noise for external attachments that cannot be found
Attached patch proposed fixSplinter Review
FTR, normal nntp attachments seemed fine to me.
Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #617205 - Flags: review?(squibblyflabbetydoo)
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]
proposed fix

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.
Attachment #617205 - Flags: review?(squibblyflabbetydoo) → review+
http://hg.mozilla.org/comm-central/rev/bb5a9e8521a2 ->FIXED
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 15.0
Blocks: 654933
You need to log in before you can comment on or make changes to this bug.