Andrew Sutherland [:asuth] (he/him) Assignee
	Description • 16 years ago

In my fix for bug 479214 I took the "if (this._curBodyPart)" guard off of "this._curBodyPart.body += aBuf;" for reasons that are not immediately clear.  (I'm assuming I thought the guard was no longer required, but normally I leave a long, boring comment explaing why it is no longer required...)

Brief investigation shows at least one logic hole; startBody calls _beginPayload, which has a case (rfc822) that assumes startBody is going to handle it.  It does not.

At the bare minimum I need to handle the (nested) rfc822 case, and add a unit test for the nested rfc822 case, and put a guard variant back with a warning.  The right thing to do is probably to also quickly investigate the circumstances in which StartBody/WriteBody will be called and document that on the emitter, ensure that we handle those cases, ensure unit tests for those cases, and remove the guard.

	Andrew Sutherland [:asuth] (he/him) Assignee
	Comment 1 • 16 years ago

My analysis was too hasty (and somewhat misled by use of the incorrect word in a comment) and made an incorrect assumption about what was actually causing the error.

This patch:
...in gloda space:
- provides a reasonably comprehensive unit test for the JS mime emitter
- greatly simplifies the JS mime emitter logic
- adds/updates the JS emitter documentation; it's fairly comprehensive now that I actually know more of what libmime is up to (and have modified it to feed us what we want.)
- removes a lot of the convenience attributes/helpers on the MimeMessage object relating to bodyParts and sub-messages.  It was all evolutionary gloda cruft, and I'm not sure what code would actually want what most of the logic does.  (For example, having all your HTML bodies concatenated does not produce particularly legal HTML code.  And the plaintext coercion is more useful than just explicit plaintext.)
- nukes some trailing whitespace from gloda files/unit tests and add some non-required but advisable semicolons.  (js2-mode's warning underlining at work...)

...in libmime space:
- makes libmime's multipart/related guy give an x-jsemitter-part-path hint (if so configured) when it actually outputs the re-swizzled content part of the related part.  This was the actual _curBodyPart problem; the emitter thought it was in an image/png and totally did not expect a bodyWrite to happen in that case.
- adds a write_pure_bodies option to stop MimeInlineImage from writing junk to the body about the image, and from separators in general from being induced.
- forces show_attachment_inline_p to be true for the jsemitter case.  I noticed this while investigating stuff that this is normally a preference.  That would suck for gloda if someone turned off the pref and we no longer got all the parts in a multipart/alternative, etc.

and for the record, the execution traces that I blogged about when hacking on this were phenomenally useful.  of course, I can think of several ways more tool-building is required to make it even more useful...! (actually, I just need one of those firefox extensions that lets you highlight different terms on a page in different colors.  I'm pretty sure a thing exists in multiple forms and I used to have one of them.)

	David :Bienvenu
	Comment 2 • 16 years ago

Comment on attachment 367561 [details] [diff] [review]
v1 provide confidence in the JS mime emitter

those js exceptions were getting painful, so I'm glad they're gone with this patch! And the tests all pass...

	Andrew Sutherland [:asuth] (he/him) Assignee
	Comment 3 • 16 years ago

pushed: http://hg.mozilla.org/comm-central/rev/4eb65f5b363c