Note: There are a few cases of duplicates in user autocompletion which are being worked on.

remove console noise for external attachments that cannot be found

RESOLVED FIXED in Thunderbird 15.0

Status

MailNews Core
Attachments
--
trivial
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: alta88, Assigned: Magnus Melin)

Tracking

unspecified
Thunderbird 15.0
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.

Comment 1

5 years ago
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]
(Reporter)

Comment 2

5 years ago
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.
(Reporter)

Comment 3

5 years ago
forgot to add ;)
(Assignee)

Updated

5 years ago
Summary: Remove invalid attachment throw → remove console noise for external attachments that cannot be found
(Assignee)

Comment 4

5 years ago
Created attachment 617205 [details] [diff] [review]
proposed fix

FTR, normal nntp attachments seemed fine to me.
Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #617205 - Flags: review?(squibblyflabbetydoo)
(Assignee)

Comment 5

5 years ago
Ugh, and without the trailing "live"
(Reporter)

Comment 6

5 years ago
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.

Comment 7

5 years ago
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?
(Reporter)

Comment 8

5 years ago
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.
(Assignee)

Comment 9

5 years ago
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 10

5 years ago
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+
(Assignee)

Comment 11

5 years ago
http://hg.mozilla.org/comm-central/rev/bb5a9e8521a2 ->FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 15.0

Updated

5 years ago
Blocks: 654933
You need to log in before you can comment on or make changes to this bug.