Closed Bug 284381 Opened 20 years ago Closed 13 years ago

open/view/save attachments of .eml file in browser

Categories

(MailNews Core :: MIME, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 707631

People

(Reporter: nian.liu, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [patchlove][only affects seamonkey])

Attachments

(1 file, 6 obsolete files)

1.open eml file in browser 2.for the attachment, there is only a table shows it is there. However, there is no way to access it.
Attached file test case (deleted) —
Attached patch patch (obsolete) (deleted) — Splinter Review
Attached patch patch (obsolete) (deleted) — Splinter Review
file mimeeobj.cpp is missed in last patch
Attachment #175999 - Attachment is obsolete: true
Boris, The reason why I didn't reuse the code in nsMultiMixedConv.cpp are 1)For nsStreamConverter, nsPartChannel is much easier than in nsMultiMixedConv. An adapter pattern is enough. 2)To reuse the code in nsMultiMixedConv, local class nsPartChannel must be refactored to a public class which means add new files, change make files, and so on. I'm not sure whether this is acceptable in community. Pls. advice.
There is nothing wrong with adding new files and changing makefiles. If there is really no code to be shared with nsMultiMixedConv, then I can see not sharing the code. Use a different class name, though, otherwise static builds that link in both classes will break.
In fact, I think nsPartChannel in nsMultiMixedConv can use part of code in nsStreamConver. And I want to file new bug to refactor after this bug is fixed.
Attached patch patch (obsolete) (deleted) — Splinter Review
rename class name in case of static link conflict according to Boris's suggestion
Attachment #176001 - Attachment is obsolete: true
Hmm ... I don't see a difference to your Bug 270115. I guess the other could be duped against this one.
Blocks: 269826
Summary: open attachment of eml file in browser → open attachment of .eml file in browser
See Bug 264167 too.
Nian, the code you're adding is in mailnews, while nsPartChannel is in necko. Mailnews can depend on necko for proper functioning, but not the other way around.
*** Bug 270115 has been marked as a duplicate of this bug. ***
Boris, I see. So after this is done, I'll fix another bug to refactor which makes the reuse part kept in necko.
Attached patch patchv2 (obsolete) (deleted) — Splinter Review
refer 264167 and make link part in table
Attachment #176017 - Attachment is obsolete: true
Why not just do it right the first time instead of filing a followup bug to do it right?
I just wnat to concentrate on issues one by one. However, I'll try to make a patch including refactor part. Before that, Can you have a look on patchv2?
I'm really not the best person to review mailnews code... I don't know much about it.
OS: Linux → All
Hardware: PC → All
*** Bug 264167 has been marked as a duplicate of this bug. ***
Attachment #176096 - Flags: review?(bienvenu)
Summary: open attachment of .eml file in browser → open/view/save attachments of .eml file in browser
Would this patch fixing bug 270772 and bug 174692?
Comment on attachment 176096 [details] [diff] [review] patchv2 Jean-Francois should review this, since it's mime.
Attachment #176096 - Flags: superreview?(bienvenu)
Attachment #176096 - Flags: review?(ducarroz)
Attachment #176096 - Flags: review?(bienvenu)
If all the nsIRequest methods forward to mChannel, why not just use the macro we have for the purpose? What's with the random printf you're checking into the tree?
for comment#18, no, image display uses different module
(In reply to comment #20) > If all the nsIRequest methods forward to mChannel, why not just use the macro we > have for the purpose? Pls. show me any example of it > > What's with the random printf you're checking into the tree? > Sorry, debug code and my mistake >
> Pls. show me any example of it http://lxr.mozilla.org/seamonkey/source/modules/libpr0n/decoders/icon/gtk/nsIconChannel.h#54 All idl interfaces automatically define a like macro (just like they define NS_DECL_*).
Attached patch patchv2.1 (obsolete) (deleted) — Splinter Review
modified according Boris's suggestion in comment#23 BTW, Boris, it's really fantastic! Can I find any backgroud knowledge of that design?
Maybe in the xpidl documentation?
Sorry for the delay! I am really busy right now, I'll try to review it next week.
Comment on attachment 176543 [details] [diff] [review] patchv2.1 Patch looks good. R=ducarroz
Attachment #176543 - Flags: superreview?(bienvenu)
Attachment #176543 - Flags: review+
I presume patchv2 can be marked as obsolete?
Comment on attachment 176096 [details] [diff] [review] patchv2 as your wish^_^
Attachment #176096 - Attachment is obsolete: true
David, can you help me sr this patch or is there someone else you recommend to sr it?
I tried this patch against seamonkey - it applied and built, but what effect should I have seen when I opened a .eml file in the browser? I didn't see the attachments displayed in such a way that I could do anything with them...
There should be a link for each attachment. With the link, you can open/save the attachment.
with my trunk seamonkey build, and this patch, I don't even see a table or link for the attachments - they just display inline as hex or text attachments.
not sure what made that. with my src(seamonkey downloaded May 20th) and this patch , test case works fine.
(In reply to comment #34) > not sure what made that. with my src(seamonkey downloaded May 20th) and this > patch , test case works fine. Worksforme?
"with this patch". The patch is not checked in. Please test things yourself before saying "worksforme", ok?
(In reply to comment #36) > "with this patch". The patch is not checked in. Please test things yourself > before saying "worksforme", ok? Sorry. I read that as if it were checked in.
Flags: blocking-aviary2.0?
David, Can you sr this patch or attach your local pathc of related bugs?
Comment on attachment 176543 [details] [diff] [review] patchv2.1 I tried this patch on some other .eml files, and it did work - I don't remember what file it didn't work on before, but it did work on your test case, in a way...except that since thunderbird is now registered as the handler for .eml files, opening the zip file in your test case says that it's going to open with winzip but clicking on the link ends up just launching thunderbird on the .eml file. I'm not sure if that behaviour is acceptable to seamonkey. + *aReturn = mChannel; + NS_IF_ADDREF(*aReturn); this can be NS_IF_ADDREF(*aReturn = mChannel); + if (filenameField) { + mContentDisposition.Append("inline; filename="); + mContentDisposition.Append(filenameField); + } the prevailing braces style in this file is if (a) { ... } not, if (a) { ... } can you fix those? There are several instances of this. I'm going to try this patch in Thunderbird to try to make sure it doesn't break anything (since this is shared code). If you address my nits, and I verify that it doesn't break thunderbird, then I'll sr+ this. Sorry for the long delay!
Attachment #176543 - Flags: superreview?(bienvenu) → superreview-
Attached patch fix opening of attachments from saved messages (obsolete) (deleted) — Splinter Review
this fixes the opening of attachments from saved local messages - the mailbox uri in the case of saved local messages is not useful, so we use the url, which should always be useful. I'll look into the account central issue next.
Assignee: nian.liu → bienvenu
Status: NEW → ASSIGNED
Attachment #193466 - Flags: superreview?(mscott)
Comment on attachment 193466 [details] [diff] [review] fix opening of attachments from saved messages wrong bug, sorry
Attachment #193466 - Attachment is obsolete: true
Attachment #193466 - Flags: superreview?(mscott)
(In reply to comment #39) > (From update of attachment 176543 [details] [diff] [review] [edit]) > I tried this patch on some other .eml files, and it did work - I don't remember > what file it didn't work on before, but it did work on your test case, in a > way...except that since thunderbird is now registered as the handler for .eml > files, opening the zip file in your test case says that it's going to open with > winzip but clicking on the link ends up just launching thunderbird on the .eml > file. I'm not sure if that behaviour is acceptable to seamonkey. > seems open take the eml file instead of the attachment. also wrong with linux. save is OK. I'll investigate it.
works for remote file. doesn't work for local file. related to bug 73757
Comment on attachment 176543 [details] [diff] [review] patchv2.1 David, pls. re-sr the patch since 309241 is checked in and should resolve the local file issue
Attachment #176543 - Flags: superreview- → superreview?(bienvenu)
Comment on attachment 176096 [details] [diff] [review] patchv2 Cancelling review request on obsolete patch.
Attachment #176096 - Flags: superreview?(bienvenu)
Attachment #176096 - Flags: review?(ducarroz)
QA Contact: mime
Product: Core → MailNews Core
does patch still apply? David, SR?
Whiteboard: [patchlove]
Attachment #176543 - Attachment is obsolete: true
Attachment #176543 - Flags: superreview?(bienvenu)
Comment on attachment 176543 [details] [diff] [review] patchv2.1 $ patch -p0 --dry-run < ~/Desktop/patch21 patching file emitters/src/nsMimeHtmlEmitter.cpp Hunk #1 succeeded at 472 (offset 13 lines). patching file src/nsStreamConverter.h Hunk #1 FAILED at 101. 1 out of 1 hunk FAILED -- saving rejects to file src/nsStreamConverter.h.rej patching file src/nsStreamConverter.cpp Hunk #1 FAILED at 69. Hunk #2 FAILED at 621. Hunk #3 FAILED at 1128. 3 out of 3 hunks FAILED -- saving rejects to file src/nsStreamConverter.cpp.rej Already bit-rotted, cancelling sr request. Can an updated patch please be provided?
As part of patchlove, bienvenu, is this 3.5 year old patch still relevant?
Flags: wanted-thunderbird3?
I could not get it to work at the time; since the problem, if not the solution, is specific to SeaMonkey, a SeaMonkey person might be better suited to un-bitrotting the patch and checking if it fixes the problem.
Flags: wanted-thunderbird3?
Flags: wanted-thunderbird3?
wanted‑thunderbird3- as this wouldn't affect thunderbird in any user visible way.
Flags: wanted-thunderbird3? → wanted-thunderbird3-
I don't see this anymore in: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090419 Lightning/1.0pre Shredder/3.1a1pre ID:20090419051533 Mozilla/5.0 (Windows; U; Windows NT 6.0; el; rv:1.9.1b3pre) Gecko/20090223 Thunderbird/3.0b2 ID:20090223175111 or: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3pre) Gecko/20090227 Lightning/1.0pre Shredder/3.0b2 Fixed then?
Klonos, this seems to be a SeaMonkey issue (see comment #49 ), please test in SeaMonkey to be sure.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b5pre) Gecko/20090425 SeaMonkey/2.0b1pre Bug is still there in current SeaMonkey trunk build on WinXP. To make it clear: The bug shows only in the *browser* part. If you open an eml file in the mail part it works (in Thunderbird too). This bug is like the others in comment 18 SeaMonkey browser specific, but the responsible code is in MailNews (which is shared between TB & SM). Therefore the product of these bugs (MailNews Core, not SeaMonkey).
Ok then, I thought we needed to set this to SeaMonkey only. Now that OstGote clarified that it is, but responsible code is in MailNews I'm shutting up ;) PS (to comment #52): I am using tb and fx at the moment + trying to setup some extra VMs to test things more and in non-production environment. As soon as I have them ready I'll try to install latest SeaMonkey as well and test things. Till then, others can confirm/test on sm.
Whiteboard: [patchlove] → [patchlove][only affects seamonkey]
Opening .eml files in the browser is not intended behaviour.
Assignee: dbienvenu → nobody
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: