169588 - make nsIMIMEConverter minimally scriptable

Status: RESOLVED FIXED

(MailNews Core :: MIME, defect)

Description

[Dan Mosedale (:dmosedale, :dmose)]

I've got a patch to make header decoding routines in nsIMIMEConverter scriptable.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch, v1

Convert .h to .idl, tweak a few signatures and all callers.

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Peter: does your Bayesian work need this patch?  I'm guessing it doesn't...

Comment 3

[nhottanscp]

+     * deprecated, because the |wstring|s are now deprecated in favor of
+     * |AString|s.

Is this true? Is there a documentation about it?


+    /**
+     * Encode routine (utf-8 input)
+     *
+     * Marked not scriptable because |header| may be non-ASCII, and UTF8 is
+     * being returned.
+     *
+     * XXXdmose is the mailCharset param actually necessary?  I.E. isn't
+     * it _always_ required to be UTF8?
+     */

The mailCharset is the charset for the output encoded word, input header always
has to be UTF-8. So, please change the comment.

Comment 4

[David :Bienvenu]

Yes, it's true. Naoki, check out Alec's string guide:
http://www.mozilla.org/projects/xpcom/string-guide.html

Comment 5

[nhottanscp]

Comment on attachment 99774 [details] [diff] [review]
patch, v1

r=nhotta

please change the comments for encodeMimePartIIStr_UTF8

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch, v2

nhotta: ok, I've removed that XXX comment, and fixed the comment above it to be
more correct.  also converted some tabs to spaces.

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 99863 [details] [diff] [review]
patch, v2

Carrying forward r=nhotta.

Comment 8

[Alec Flett]

Comment on attachment 99863 [details] [diff] [review]
patch, v2

I only count 4 callers of DeprecatedDecodeMimeHeader() - what are the chances
of just eliminating that method entirely?

In fact, I'm looking at the usage, and it looks like the conversion would be
pretty darn easy..

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Assigning bugs that I'm not actively working on back to nobody; use
SearchForThis as a search term if you want to delete all related bugmail at
once.

Comment 10

[Andrew Sutherland [:asuth] (he/him)]

I have forward-ported dmose's patch and things seem to work.  Will post the patch/request review after a sanity checking.

Comment 11

[Andrew Sutherland [:asuth] (he/him)]

Attachment: patch, v3. eliminate deprecated case too.

Here is the updated version of dmose's patch.  Changes are: remove the deprecated wchar variant, propagating the changes through; move the #define constants to interface constants; add some doxygen.

Note that since nsIMimeConverter was separately created by bug 253807, the encodedMimePartIIStr(_UTF8) methods had already been exposed scriptable, and so I haven't changed them, apart from adding some doxygen.

Comment 12

[David :Bienvenu]

Comment on attachment 324680 [details] [diff] [review]
patch, v3.  eliminate deprecated case too.

My one quibble is with this method name:

DecodeMimeHeaderToCString

because it really returns a char ptr, and CString makes it sound like it returns a nsCString, or a ACString

so I might call it DecodeMimeHeaderToCharPtr

- it's just an unfortunate thing that our wrapper classes use CString in their names, but C string already had a meaning. I leave it to your judgement, thx for driving the patch forward.

Comment 13

[Andrew Sutherland [:asuth] (he/him)]

Attachment: patch, v4. rename DecodeMimeHeaderToCString to DecodeMimeHeaderToCharPtr

Make the name change as proposed by bienvenu.  (Hooray for keeping the length the same.)  carrying forward r=bienvenu,sr=bienvenu

Comment 14

[Mark Banner (:standard8)]

Checking in mailnews/base/public/nsIMsgHdr.idl;
/cvsroot/mozilla/mailnews/base/public/nsIMsgHdr.idl,v  <--  nsIMsgHdr.idl
new revision: 1.33; previous revision: 1.32
done
Checking in mailnews/base/search/src/nsMsgFilter.cpp;
/cvsroot/mozilla/mailnews/base/search/src/nsMsgFilter.cpp,v  <--  nsMsgFilter.cpp
new revision: 1.125; previous revision: 1.124
done
Checking in mailnews/base/search/src/nsMsgSearchTerm.cpp;
/cvsroot/mozilla/mailnews/base/search/src/nsMsgSearchTerm.cpp,v  <--  nsMsgSearchTerm.cpp
new revision: 1.164; previous revision: 1.163
done
Checking in mailnews/base/src/nsMsgDBView.cpp;
/cvsroot/mozilla/mailnews/base/src/nsMsgDBView.cpp,v  <--  nsMsgDBView.cpp
new revision: 1.317; previous revision: 1.316
done
Checking in mailnews/base/src/nsSpamSettings.cpp;
/cvsroot/mozilla/mailnews/base/src/nsSpamSettings.cpp,v  <--  nsSpamSettings.cpp
new revision: 1.35; previous revision: 1.34
done
Checking in mailnews/base/util/nsMsgI18N.cpp;
/cvsroot/mozilla/mailnews/base/util/nsMsgI18N.cpp,v  <--  nsMsgI18N.cpp
new revision: 1.108; previous revision: 1.107
done
Checking in mailnews/base/util/nsMsgI18N.h;
/cvsroot/mozilla/mailnews/base/util/nsMsgI18N.h,v  <--  nsMsgI18N.h
new revision: 1.33; previous revision: 1.32
done
Checking in mailnews/base/util/nsMsgUtils.cpp;
/cvsroot/mozilla/mailnews/base/util/nsMsgUtils.cpp,v  <--  nsMsgUtils.cpp
new revision: 1.150; previous revision: 1.149
done
Checking in mailnews/compose/src/nsMsgCompose.cpp;
/cvsroot/mozilla/mailnews/compose/src/nsMsgCompose.cpp,v  <--  nsMsgCompose.cpp
new revision: 1.572; previous revision: 1.571
done
Checking in mailnews/compose/src/nsMsgComposeService.cpp;
/cvsroot/mozilla/mailnews/compose/src/nsMsgComposeService.cpp,v  <--  nsMsgComposeService.cpp
new revision: 1.147; previous revision: 1.146
done
Checking in mailnews/compose/src/nsMsgSendLater.cpp;
/cvsroot/mozilla/mailnews/compose/src/nsMsgSendLater.cpp,v  <--  nsMsgSendLater.cpp
new revision: 1.123; previous revision: 1.122
done
Checking in mailnews/compose/src/nsSmtpUrl.cpp;
/cvsroot/mozilla/mailnews/compose/src/nsSmtpUrl.cpp,v  <--  nsSmtpUrl.cpp
new revision: 1.85; previous revision: 1.84
done
Checking in mailnews/db/msgdb/public/nsMsgDatabase.h;
/cvsroot/mozilla/mailnews/db/msgdb/public/nsMsgDatabase.h,v  <--  nsMsgDatabase.h
new revision: 1.134; previous revision: 1.133
done
Checking in mailnews/db/msgdb/src/nsMsgDatabase.cpp;
/cvsroot/mozilla/mailnews/db/msgdb/src/nsMsgDatabase.cpp,v  <--  nsMsgDatabase.cpp
new revision: 1.403; previous revision: 1.402
done
Checking in mailnews/db/msgdb/src/nsMsgHdr.cpp;
/cvsroot/mozilla/mailnews/db/msgdb/src/nsMsgHdr.cpp,v  <--  nsMsgHdr.cpp
new revision: 1.127; previous revision: 1.126
done
Checking in mailnews/local/src/nsPop3Sink.cpp;
/cvsroot/mozilla/mailnews/local/src/nsPop3Sink.cpp,v  <--  nsPop3Sink.cpp
new revision: 1.150; previous revision: 1.149
done
Checking in mailnews/mime/emitters/src/nsMimeBaseEmitter.cpp;
/cvsroot/mozilla/mailnews/mime/emitters/src/nsMimeBaseEmitter.cpp,v  <--  nsMimeBaseEmitter.cpp
new revision: 1.93; previous revision: 1.92
done
Checking in mailnews/mime/emitters/src/nsMimeHtmlEmitter.cpp;
/cvsroot/mozilla/mailnews/mime/emitters/src/nsMimeHtmlEmitter.cpp,v  <--  nsMimeHtmlEmitter.cpp
new revision: 1.95; previous revision: 1.94
done
Checking in mailnews/mime/public/nsIMimeConverter.idl;
/cvsroot/mozilla/mailnews/mime/public/nsIMimeConverter.idl,v  <--  nsIMimeConverter.idl
new revision: 1.4; previous revision: 1.3
done
Checking in mailnews/mime/src/comi18n.cpp;
/cvsroot/mozilla/mailnews/mime/src/comi18n.cpp,v  <--  comi18n.cpp
new revision: 1.137; previous revision: 1.136
done
Checking in mailnews/mime/src/comi18n.h;
/cvsroot/mozilla/mailnews/mime/src/comi18n.h,v  <--  comi18n.h
new revision: 1.31; previous revision: 1.30
done
Checking in mailnews/mime/src/mimeenc.cpp;
/cvsroot/mozilla/mailnews/mime/src/mimeenc.cpp,v  <--  mimeenc.cpp
new revision: 1.26; previous revision: 1.25
done
Checking in mailnews/mime/src/modmimee.h;
/cvsroot/mozilla/mailnews/mime/src/modmimee.h,v  <--  modmimee.h
new revision: 1.11; previous revision: 1.10
done
Checking in mailnews/mime/src/nsMimeConverter.cpp;
/cvsroot/mozilla/mailnews/mime/src/nsMimeConverter.cpp,v  <--  nsMimeConverter.cpp
new revision: 1.44; previous revision: 1.43
done
Checking in mailnews/mime/src/nsMimeConverter.h;
/cvsroot/mozilla/mailnews/mime/src/nsMimeConverter.h,v  <--  nsMimeConverter.h
new revision: 1.21; previous revision: 1.20
done

Comment 15

[Eric Shepherd [:sheppy]]

Since this interface is not yet documented at all, this change does not seem to require specifically being tagged as doc needed.

Comment 16

[Dan Mosedale (:dmosedale, :dmose)]

Sheppy: this fix has made it possible to do something in extensions that wasn't previously possible.  I think it's worth communicating that change to existing extension developers, presumably as part of a "What's New In Tb3 For Developers" document.  I was assuming that adding dev-doc-needed was a reasonable way to track bugs that need that sort of documentation.  Is there some other way that you would prefer and/or that is likely to work better?

Comment 17

[Eric Shepherd [:sheppy]]

dev-doc-needed is generally intended for use when an existing document will require updating to cover a topic, or for bugs that result in the introduction of a whole new API.  For example, if this bug resulted in the creation of a new interface, that would warrant dev-doc-needed.

In this case, there's a relatively small change being made to an existing API, but that existing API isn't documented, so there should be a separate bug filed against MDC's Documentation Request category requesting that nsIMIMEConverter be documented.

That said, be aware that since my priority is Firefox and other MoCo documentation, unfortunately odds are you guys are going to need to find a volunteer to handle Thunderbird docs, since between maintaining MDC itself and writing Firefox, Gecko, Necko, etc docs, I'm pretty swamped. :)

Comment 18

[Dan Mosedale (:dmosedale, :dmose)]

Sheppy: (In reply to comment #17)
> dev-doc-needed is generally intended for use when an existing document will
> require updating to cover a topic, or for bugs that result in the introduction
> of a whole new API.  For example, if this bug resulted in the creation of a new
> interface, that would warrant dev-doc-needed.

OK, fair enough.

> In this case, there's a relatively small change being made to an existing API,
> but that existing API isn't documented, so there should be a separate bug filed
> against MDC's Documentation Request category requesting that nsIMIMEConverter
> be documented.

I agree; the reason I didn't go file that bug is because it's more work than we can expect to happen in a Tb3 time frame, and because it's really part of a much bigger separate project: lots of Thunderbird interfaces are undocumented or underdocumented.  Because we don't currently have the resources, we're very much not taking on that work right now.  The much tinier unit of work that I'm trying to figure how to track and make happen is "get something added to the 'What's New In Tb3 For Developers' wiki page.  I think shipping Tb3 with such a page is a very tractable problem.

> That said, be aware that since my priority is Firefox and other MoCo
> documentation, unfortunately odds are you guys are going to need to find a
> volunteer to handle Thunderbird docs, since between maintaining MDC itself and
> writing Firefox, Gecko, Necko, etc docs, I'm pretty swamped. :)

Totally understood; I'm just hoping I can get your thoughts on how we (the Thunderbird team) should go about tracking this sort of work, both because you've got lots of good experience here, and because that way we're more likely to end up with a process that's aligned with how the rest of Mozilla does things, and therefore more understandable to the broader contributor base.  How does Firefox track adding an item to "What's New For Devs" page?