Created attachment 331194 [details] [diff] [review] patch v1 GetMsgTextFromStream right now uses its own header parsing -- libmime can definitely be used for this instead. There are also several tests I wrote to test this. The in and out files are very bare, containing just the necessary headers to test. One nice side effect of this is that there are now several emails of different kinds that can be used as fodder for tests -- more headers can probably be added without severely breaking this test. Requesting r+sr from bienvenu for main function changes, r from Standard8 for tests. Please make sure that 1. all the test data files have CRLF line endings 2. the out files do not have a newline at the end both are currently necessary for the test to work.
Comment on attachment 331194 [details] [diff] [review] patch v1 >diff --git a/mailnews/base/public/nsIMsgFolder.idl b/mailnews/base/public/nsIMsgFolder.idl This file is missing a uuid change. >+function run_test() >+ var inStream = Cc["@mozilla.org/network/file-input-stream;1"].createInstance(Ci.nsIFileInputStream); nit: wrap these lines please (I suggest putting the .createInstance on the next line, and aligning the . with the [ >+ var outFile = do_get_file(kDataRoot + test.name + ".out"); >+ var outStream = Cc["@mozilla.org/network/file-input-stream;1"].createInstance(Ci.nsIFileInputStream); >+ outStream.init(outFile, -1, -1, Ci.nsIFileInputStream.CLOSE_ON_EOF); This is confusing, you've called it an outStream, but its an input stream. How about resultFile and resultStream? Also, I've just created a function loadFileToStream in mailTestUtils.js to do a very similar function. How about extending that to take a charset parameter, if the param is null, just do what it does currently, otherwise pass it through the converter input stream?
Created attachment 332346 [details] [diff] [review] update v1.1, addressing nits and extending loadFileToString
Attachment #332346 - Flags: review?(bugzilla)
Comment on attachment 332346 [details] [diff] [review] update v1.1, addressing nits and extending loadFileToString r=me for the unit test changes
Attachment #332346 - Flags: review?(bugzilla) → review+
sid0, very cool. one nit - mimehdrpar - can you put some upper case in there, like mimeHdrParam? I haven't had a chance to test this yet, and I would like to make sure it doesn't break Spotlight, so I'll apply it now and test it.
Comment on attachment 332346 [details] [diff] [review] update v1.1, addressing nits and extending loadFileToString TB's mdimporter seems to be functioning w/ this patch
Created attachment 332378 [details] [diff] [review] patch to checkin nit fixed, carrying forward r+sr=bienvenu for main changes, r=Standard8 for unit test changes.
Checked in, changeset id: 57:81120c50a9b3
Target Milestone: --- → mozilla1.9.1a2
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.