162440 - Mime parameters are stripped off when setting contentType of nsMsgAttachment

Status: RESOLVED FIXED

(MailNews Core :: MIME, defect)

Description

[ajbu]

build 2002080208
When setting the contentType for an attachment, the mime-parameters are 
stripped off. Mime parameters are part of the mime specification, and are 
needed for inviting over mail using Mozilla Calendar.
Example contenttype of a Calendar meeting request according to the iCalendar 
rfc:
 Content-Type:text/calendar; method=request; charset=UTF-8;

Looking at nsMsgAttachment.cpp#121, the paramaters are stripped out in 
SetContentType, I don't understand the 'need' for cleanup here. This removal 
of parameters will block using Calendar over email.

http://lxr.mozilla.org/mozilla/source/mailnews/compose/src/nsMsgAttachment.cpp#
121
121 NS_IMETHODIMP nsMsgAttachment::SetContentType(const char * aContentType)
122 {
123   mContentType = aContentType;
124   /* a full content type could also contains parameters but we need to
125      keep only the content type alone. Therefore we need to cleanup it.
126   */
127   PRInt32 offset = mContentType.FindChar(';');
128   if (offset >= 0)
129     mContentType.Truncate(offset);
130 
131   return NS_OK;
132 }

Comment 1

[Jean-Francois Ducarroz]

nsIMsgAttachment are used only by message composition (new, forward, template
and draft). This is not used when displaying a received message!

Reporter, can you explain what is the real problem?

Comment 2

[ajbu]

Problem is about sending a message, not displaying one.
Mozilla Calendar *sends* a message with an attachment of text/calendar type 
that needs the method parameter mentioned.

Code that sets the contenttype:
http://lxr.mozilla.org/mozilla/source/calendar/resources/content/calendarMail.j
s#192 (needed parameter removed here because they didn't work)
It adds the calendar event as an attachment to the message and calls 
nsIMsgComposeService.OpenComposeWindowWithParams for sending.

This system of exchanging appointments is specified by RFC 2445

Comment 3

[ajbu]

For correctness: I mean RFC 2447, iCalendar Message-Based Interoperability 
Protoco (iMIP) (ftp://ftp.isi.edu/in-notes/rfc2447.txt)

Comment 4

[Jean-Francois Ducarroz]

Thanks for the clarification. I'll look into it...

Comment 5

[Jean-Francois Ducarroz]

I have a fix which I haven't tested yet. Bassically, I added a new attribute to
nsIMsgAttachment to handle content-type extra params. You will need to add the
following line in calendarMail.js:

  nsIMsgAttachment.contentType = "text/calendar";
  nsIMsgAttachment.contentTypeParam = "method=request";

The charset will be generated during the send. Let me know if that cause a
problem too.

Ajbu, would be nice if you could give a try to this patch...

Comment 6

[Jean-Francois Ducarroz]

Attachment: Proposed fix, v1 (not tested)

Comment 7

[ajbu]

contentTypeParam seems to work fine. Send doesn't set a charset for 
attachments, the generated header for the attachement is:

--------------000007000902020802050009
Content-Type: text/calendar; method=REQUEST;
 name="iCal Event"
Content-Transfer-Encoding: 8bit
Content-Disposition: inline;
 filename="iCal Event"

Comment 8

[Jean-Francois Ducarroz]

no charset has been generated probably because the data is ISO-8859-1 which is
the default. Does the fact the charset is "missing" cause any trouble? Are you
able to  open and view correctly the calendar attachment?

Comment 9

[ajbu]

Actualy, I use hebrew, copied from: http://www.columbia.edu/kermit/utf8.html
when using hebrew in both the messagebody (text/plain) and the attachment, the 
messagebody is readable, but the text/calendar attachment also shown in mail 
is garbage.
Sending a text attachment doesn't seem to detect the charset used and assumes 
ISO-8859-1?

Note that the calendar event is saved to the tempdir and then attached using 
the url property. I copied some text from the utf8.html page for testing.
Also note that nsIMsgAttachment.url is a string, so won't work when using a 
url to a directory with a unicode name?

Comment 10

[ajbu]

There are other bugs about not setting the character set, like bug 71551.
It would be logical if nsMsgAttachment has a property to set the charset like 
nsIMsgCompFields: 74   attribute string  characterSet;

Offcourse the new contentTypeParam can be used for that, but characterSet 
would be more consistent.

Comment 11

[Jean-Francois Ducarroz]

I'll add a charset attribute to the attachment structure..., that will prepare
the ground for bug 71551.

Comment 12

[Jean-Francois Ducarroz]

Attachment: Proposed fix, v2

I added the support for a charset attribute as well a content-type param
(already  in previous patch) in nsIMsgAttachment.

Comment 13

[Jean-Francois Ducarroz]

Ajbu, can you test the new patch to make sure it fit your needs. Just add the
following in your code:

  nsIMsgAttachment.contentType = "text/calendar";
  nsIMsgAttachment.contentTypeParam = "method=request";
  nsIMsgAttachment.charset = "<whatever you charset is>";

Thanks

Comment 14

[ajbu]

Setting contentTypeParam and charset works and generates the desired content-
type

Comment 15

[varada]

Comment on attachment 95277 [details] [diff] [review]
Proposed fix, v2

We should probably rewrite or change the PUSH_STRING stuff sometime later.
Looks good to me. This will help with the internationalisation problem of
handling attachemnts of text files in japanese because now we can pass the
charsets from the front.
r=varada

Comment 16

[David :Bienvenu]

Comment on attachment 95277 [details] [diff] [review]
Proposed fix, v2

sr=bienvenu

Comment 17

[Jean-Francois Ducarroz]

Fixed in the trunk