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

RESOLVED FIXED

Status

MailNews Core
MIME
RESOLVED FIXED
12 years ago
5 years ago

People

(Reporter: shaver, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.
Created attachment 180648 [details] [diff] [review]
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.
Attachment #180648 - Flags: review?(bienvenu)

Comment 2

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

r=bienvenu with some nits over aim
Attachment #180648 - Flags: review?(bienvenu) → review+

Updated

12 years ago
Depends on: 290239
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!
Status: NEW → ASSIGNED
Flags: blocking1.8b2?
Flags: blocking-aviary1.1?
Created attachment 180814 [details] [diff] [review]
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.
Attachment #180814 - Flags: review?(bienvenu)
Attachment #180814 - Flags: approval1.8b2?
Attachment #180814 - Flags: approval-aviary1.1a?
Attachment #180648 - Attachment is obsolete: true

Updated

12 years ago
Attachment #180814 - Flags: review?(bienvenu) → review+
Comment on attachment 180814 [details] [diff] [review]
As promised.

a=dveditz
Attachment #180814 - Flags: approval1.8b2?
Attachment #180814 - Flags: approval1.8b2+
Attachment #180814 - Flags: approval-aviary1.1a?
Attachment #180814 - Flags: approval-aviary1.1a+
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.
Flags: blocking1.8b2?
Flags: blocking-aviary1.1?

Comment 7

12 years ago
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
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.
Mass abdication.
Assignee: shaver → nobody
Status: ASSIGNED → NEW
QA Contact: mime
(Assignee)

Updated

9 years ago
Product: Core → MailNews Core
joshua, ref comment 7. If this still occurs, would you concur to close this (FIXED) bug and file a new one?
Flags: needinfo?
This interface exists and is in use already, so I see no reason to keep it open.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: needinfo?
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.