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

RESOLVED DUPLICATE of bug 707631

Status

MailNews Core
MIME
RESOLVED DUPLICATE of bug 707631
13 years ago
6 years ago

People

(Reporter: Nian Liu(n/a in a long time), Unassigned)

Tracking

(Blocks: 1 bug)

Bug Flags:
wanted-thunderbird3 -

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(1 attachment, 6 obsolete attachments)

(Reporter)

Description

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

Comment 1

13 years ago
Created attachment 175998 [details]
test case
(Reporter)

Comment 2

13 years ago
Created attachment 175999 [details] [diff] [review]
patch
(Reporter)

Comment 3

13 years ago
Created attachment 176001 [details] [diff] [review]
patch

file mimeeobj.cpp is missed in last patch
Attachment #175999 - Attachment is obsolete: true
(Reporter)

Comment 4

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

Comment 6

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

Comment 7

13 years ago
Created attachment 176017 [details] [diff] [review]
patch

rename class name in case of static link conflict according to Boris's
suggestion
Attachment #176001 - Attachment is obsolete: true

Comment 8

13 years ago
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

Comment 9

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

Comment 11

13 years ago
*** Bug 270115 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 12

13 years ago
Boris, I see. So after this is done, I'll fix another bug to refactor which
makes the reuse part kept in necko.

(Reporter)

Comment 13

13 years ago
Created attachment 176096 [details] [diff] [review]
patchv2

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

Comment 15

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

Updated

13 years ago
OS: Linux → All
Hardware: PC → All
(Reporter)

Comment 17

13 years ago
*** Bug 264167 has been marked as a duplicate of this bug. ***
(Reporter)

Updated

13 years ago
Attachment #176096 - Flags: review?(bienvenu)

Updated

13 years ago
Summary: open attachment of .eml file in browser → open/view/save attachments of .eml file in browser

Comment 18

13 years ago
Would this patch fixing bug 270772 and bug 174692?

Comment 19

13 years ago
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?

(Reporter)

Comment 21

13 years ago
for comment#18,

no, image display uses different module
(Reporter)

Comment 22

13 years ago
(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_*).
(Reporter)

Comment 24

13 years ago
Created attachment 176543 [details] [diff] [review]
patchv2.1

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

Comment 29

13 years ago
Comment on attachment 176096 [details] [diff] [review]
patchv2

as your wish^_^
Attachment #176096 - Attachment is obsolete: true
(Reporter)

Comment 30

13 years ago
David, can you help me sr this patch or is there someone else you recommend to
sr it?

Comment 31

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

Comment 32

13 years ago
There should be a link for each attachment. With the link, you can open/save the
attachment.

Comment 33

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

Comment 34

13 years ago
not sure what made that. with my src(seamonkey downloaded May 20th) and this
patch , test case works fine.

Comment 35

13 years ago
(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?

Comment 37

13 years ago
(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.

Updated

13 years ago
Flags: blocking-aviary2.0?
(Reporter)

Comment 38

13 years ago
David,

Can you sr this patch or attach your local pathc of related bugs?

Comment 39

13 years ago
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-

Comment 40

13 years ago
Created attachment 193466 [details] [diff] [review]
fix opening of attachments from saved messages

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 41

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

Comment 42

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

Comment 43

13 years ago
works for remote file. doesn't work for local file. related to bug 73757
(Reporter)

Comment 44

12 years ago
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)

Updated

10 years ago
QA Contact: mime
(Assignee)

Updated

10 years ago
Product: Core → MailNews Core

Comment 46

9 years ago
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?

Comment 49

9 years ago
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?

Updated

9 years ago
Flags: wanted-thunderbird3?

Comment 50

9 years ago
wanted‑thunderbird3- as this wouldn't affect thunderbird in any user visible way.
Flags: wanted-thunderbird3? → wanted-thunderbird3-

Comment 51

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

Comment 53

9 years ago
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).

Comment 54

9 years ago
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
Last Resolved: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 707631
You need to log in before you can comment on or make changes to this bug.