290247 - add simple, scriptable interface for MIME-part conversion/display

Status: RESOLVED FIXED

(MailNews Core :: MIME, defect)

Description

[Mike Shaver (:shaver emeritus)]

Over in Lightning, land, we'd really like to

- handle text/calendar attachments, presenting the user with a nice UI for
  inspecting, accepting, and replying to invitations and events, and
- do it from script.

The patch I will attach shortly allows us to do that, by adding a very simple
conversion interface, and a cthandler stub that proxies the content back and
forth.  I'm sure we want more on this interface for other, similar uses, but for
now this lets us get quite a ways in, and seems like a useful addition for 1.1a
aka 1.8b2.

The build stuff is currently set up to keep the scriptable-stub component in its
own DLL, but I could easily integrate it in directly if desired.

Comment 1

[Mike Shaver (:shaver emeritus)]

Attachment: add scriptable stub/glue, call out to it as appropriate

There are various ways I could refactor this to make the use of two categories
unnecessary, but I'm motivated to leave that until after 1.1a/1.8b2 when we can
get some feedback on what other attachment-presenting folks would need there.

Comment 2

[David :Bienvenu]

Comment on attachment 180648 [details] [diff] [review]
add scriptable stub/glue, call out to it as appropriate

r=bienvenu with some nits over aim

Comment 3

[Mike Shaver (:shaver emeritus)]

So right now, the patch requires that a scriptable-converter impl do the following:

- register the scriptable stub's contractID as handling the content type it wishes
  to handle (by using a category with no other purpose, because you can't just
  tell the component registry to map a given contractID to a given CID without
  also telling it where the factory is),
- register itself in a category inspected by the scriptable stub, again with the 
  content type it wishes to handle.

I would like to refine my patch to do the following:
 - fold the scriptable stub into the core MIME code
 - have mime_locate_external_content_handler check the scriptable-dispatch
   category itself, and create a scriptable stub to wrap it if it finds 
   something appropriate

This would simplify the control flow for the scriptable stub a bit, remove the
factory boilerplate, eliminate the first category mentioned above, and eliminate
the need for a separate scriptable/ directory with the associated build changes.

I'm going to put that patch up in a few hours, because I think it's a better API
to expose to mail extenders, and it's not much work to make that improvement. 
Comments on the plan (and the upcoming patch) encouraged!

Comment 4

[Mike Shaver (:shaver emeritus)]

Attachment: As promised.

I also improved the category naming, and added a C++-visible #define for it. 
This works quite well for me, and I can't think of a good way to improve it
(cache the catman, avoid the duplicate category lookups) that is possible in
the C-object world of libmime.	I'd love to get this in 1.8b2 and Tbird 1.1a,
for lightning and others.

Comment 5

[Daniel Veditz [:dveditz]]

Comment on attachment 180814 [details] [diff] [review]
As promised.

a=dveditz

Comment 6

[Mike Shaver (:shaver emeritus)]

FIXED.  If we want to change the API to expose more control to components
implementing it, I'm very much amenable to bugs filed for that purpose.

Comment 7

[Scott MacGregor]

I'm seeing lots of assertions that look like:

WARNING: NS_ENSURE_TRUE(aEntryName) failed, file c:/build/trees/tbirddbg/mozilla
/xpcom/components/nsCategoryManager.cpp, line 503
Time Spent in mime:    53 ms
WARNING: NS_ENSURE_TRUE(aEntryName) failed, file c:/build/trees/tbirddbg/mozilla
/xpcom/components/nsCategoryManager.cpp, line 503

because we are passing in an empty string for the content type into the category
manager in mime_locate_external_content_handler.

stack trace:

nsCategoryManager::GetCategoryEntry(nsCategoryManager * const 0x00b20c40, const
char * 0x02692ac0, const char * 0x00000000, char * * 0x0012ed00) line 503
mime_locate_external_content_handler(const char * 0x00000000,
contentTypeHandlerInitStruct * 0x0012eeb8) line 244 + 74 bytes
mime_find_class(const char * 0x00000000, MimeHeaders * 0x0341dbf0,
MimeDisplayOptions * 0x034042e8, int 0) line 512 + 13 bytes
mime_create(const char * 0x00000000, MimeHeaders * 0x0341dbf0,
MimeDisplayOptions * 0x034042e8) line 830 + 19 bytes
MimeMessage_close_headers(MimeObject * 0x03ed0c48) line 469 + 23 bytes

Comment 8

[Mike Shaver (:shaver emeritus)]

That stack doesn't look like it's passing an empty string, but rather a NULL
pointer -- I'm a little surprised we don't crash.  Is this correct behaviour for
MimeMessage_close_headers here?  I don't see the assertions with my build, so
far, but I'll play around and try to make it happen.

Comment 9

[Mike Shaver (:shaver emeritus)]

Mass abdication.

Comment 10

[Wayne Mery (:wsmwk)]

joshua, ref comment 7. If this still occurs, would you concur to close this (FIXED) bug and file a new one?

Comment 11

[Joshua Cranmer [:jcranmer]]

This interface exists and is in use already, so I see no reason to keep it open.