Closed Bug 284381 Opened 19 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
Attached patch patch (obsolete) — Splinter Review
Attached patch patch (obsolete) — 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) — 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) — 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) — 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-
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: