Closed Bug 106507 Opened 23 years ago Closed 23 years ago

Mail/News S/MIME initial landing tracking bug.

Categories

(MailNews Core :: Backend, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: ssaux, Assigned: ddrinan0264)

References

Details

Attachments

(7 files, 4 obsolete files)

(deleted), image/jpeg
Details
(deleted), patch
bugzilla
: review+
mscott
: superreview+
Details | Diff | Splinter Review
(deleted), patch
mscott
: review+
sspitzer
: superreview+
Details | Diff | Splinter Review
(deleted), patch
bugzilla
: review+
mscott
: superreview+
Details | Diff | Splinter Review
(deleted), patch
bugzilla
: review+
Details | Diff | Splinter Review
(deleted), text/plain
Details
(deleted), patch
bugzilla
: review+
Details | Diff | Splinter Review
This bug created to attach diff for the initial landing of s/mime features.
Blocks: 105526
Attached patch Mailnews diffs for S/MIME (obsolete) (deleted) — Splinter Review
Attached patch New patch. (obsolete) (deleted) — Splinter Review
Attachment #54978 - Attachment is obsolete: true
I'm still in the middle of making changes to make smime an extension in mailnews. However, while I'm still doing that, I have some problems with the current look of the cert picker dialog which I'd like us to fix before we land smime. This is the dialog that comes up from the account manager when you pick a cert to use for encryption or to sign messages. I'll post a picture, but here are the current problems: 1) The Window title says "X" 2) The dialog name says "Y" 3) The window isn't wide enough causing the basic form elements in the dialog to be clipped. 4) The window is not resizeable David, can you tag the right person to fix these issues? (I don't know if your the right person or if someone else owns that particular dialog).
btw, I'm almost done with the mailnews/extensions/smime work. However, David mentioned that at some point this weekened release is spinning test builds of s/mime. We don't know when this is going to happen to the branch is effectively "closed" to me since I don't want to check something in and have it break your test builds that are getting built. I've got everything moved over except for a few lines of code in nsMsgSend and nsMsgCompFields. I'll try to finish those fragments up today. On Monday, I'll check this stuff in and then I'll need help with the following: 1) Mac build changes to match my changes and to remove compose\src\nsMsgComposeSecure.cpp from the mac build. I just installed OSX on my machine and have not set up a build environment yet. 2) Linux makefile changes for the new smime directory. My linux machine is broken right now so I can't do this. I had to rewrite most of the smime account wizard stuff to play nice with the account manager and to leverage the generic attribute changes Seth came up with which made it possible for us to dynamically bring in the security panel and to dynamically set attributes on the msg identity. Many thanks to Seth for working so hard over the weekend to give me patches to make this possible.
cc javi for the mac project. This is great, and a thousand thanks for your and Seth's work. I'll make sure that the next builds on the test branch has all the fixes to date.
I've figured out the X and Y and I've checked in a fix into the branch.
Here's the email I just sent Javi and David summarizing what I just checked into the branch and what we still need to do: I just checked in the first phase of shifting the smime code into a mozilla/mailnews/extensions/smime directory. I had one problem. I was unable to check in a change to nsIMsgIncomingServer.idl. I just sent David email with a diff seeing if he can check it in. It almost appears like the entry in CVS is corrupt for this file on the branch. What I did: 1) identities, accounts and servers now all support "generic" attributes so we don't have to hard code methods for storing the s/mime prefs on a user's identity. 2) Moved nsMsgComposeSecure.cpp/.h out of mailnews/compose and into mailnews/extensions/smime/src 3) Moved am-security.xul & am-security.js into the extensions directory. I had to re-write the xul to fit in with the account manager & to support generic attributes. So we'll need to retest the behavior of this panel. CURRENTLY: your changes in this dialog aren't saved to prefs yet. I have to debug this some more. I also have to move David's checkin this morning for the cert dialog over to these new files. 4) Created overlays for the smime menu items and placed them in mailnews/extensions/smime/resources/content. Now, when the smime extension is built, these overlays will insert the menu items (and the associated JS) into the compose window dynamically. 5) created lots of new .dtd and .properties files for the smime string changes inside the extensions/smime module and took those strings out of compose and mailnews/base. 6) added default values for a new identity for the security settings in a new smime.js file in extensions/smime/resources/content. We can use this JS file to change the default security settings we want for new identities. If we add more smime specific prefs in the future they can go here too. 7) new jar.mn file for extensions/smime What still needs to happen: 1) ME: I need to figure out why changes via the security account manager panel aren't getting saved to disk 2) ME: I now need to start looking into abstracting out the SMIME C++ changes in mailnews compose (nsMsgSend, nsMsgCompFields, etc) and figuring out how to move that into extensions/smime. 3) Mac project changes: * For starters, we need to remove nsMsgComposeSecure.h/.cpp from the msgcompose mac project. They are no longer in the tree. * We need to create a new macbuild directory in extensions/smime and create a new project which builds nsMsgComposeSecure.cpp and nsMsgSMIMEFactory.cpp , creating a new shared library: msgsmime * There's a new JAR file in mailnews/extensions/smime. The mac build scripts will need to be modified to look for this new JAR file. 4) Linux Makefile.in changes: Well I checked in Makefile.in files for my new directory structure but my linux machine is broken right now. I'm sure they don't work. But at least the structure is there. We need to add mailnews/extensions/smime to mailnews/makefiles. Then we need to make sure the Makefile.in files I created actually work and create a new shared library for msgsmime. In addition, I have some special REGISTER_CHROME stuff in makefile.win in extensions/smime/resources/content and /locale/en-US. We need to make sure that same magic happens in the Linux makefiles.
I already talked about the first part of the changes I checked in earlier today with regards to breaking out the UI into a smime/extensions directory and making the account manager, msg identities and servers support generic attributes so our extension/smime module can poke stuff in them. Phase II involved breaking out the remaining smime dependencies in mailnews/compose . First, I took out the smime changes to nsIMsgComposeField. nsIMsgComposeField now holds on to a nsISupports securityInfo object. This object is blindly passed around in mailnews/compose. Nowhere do we make any assumptions about what's inside of it. It gets passed into the implementor of nsIMsgComposeSecure. So other encryption modules other than S/MIME can store whatever information they want in this object. Now, in msgSMIMEComposeOverlay.xul/.js, we create a new object which supports a smime private interface called nsIMsgSMIMECompFields. It then gets the msg compose fields object and opaquely stores this object as the security info object slot of nsIMsgCompFields. Changes to the smime security overlay menu (changing the encryption, or signature state) are reflected back in the nsIMsgSMIMECompFields object. When the user sends a message, nsMsgSend, now attemps to create a nsIMsgComposeSecure object. If it is there (right now smime is the only provider implementing this interface), we ask the secure compose if for this identity and this opaque security info object, we need to perform some kind of encryption. Our SMIME code QIs the security object for our private MSIME Comp Field interface, reads the settings and determines if we need to encrypt or send this ougoing message. If compose secure says we don't need to do any kind of encryption on the outgoing message then nsMsgSend detroys the compose secure object. Otherwise it calls apis for being encryption encapsulation writeData and end encryption encapsulation. So we are calling through a very clean set of abstract methods for encryption. I believe this leaves us in very good shape for making smime work completely as an extension to mailnews with the exception of the mime changes which we can't separate out anyway. I'll check in phase II right now.
I just checked in phase II. This means more work for Javi =(. In order to pull this off I had to create a new smime specific interface which means a new IDL mac project for extensions/smime. You'll notice we now have: extensions/smime/public/nsIMsgSMIMECompFields.idl That's going to require adding a mac IDL project change. What's Left: 1) project changes for the mac 2) state of the linux build? 3) I still have to figure out why changes to the security panel in the account wizard aren't persisting. I'm probably doing something stupid. I just haven't figured out what yet. 4) I also need to generate a new set of mailnews diffs between the trunk and the branch. This way we can tell if I've missed some dependencies on smime in core mailnews. Thanks for the help everyone!
A couple of things I'm worried about which may effect a potential landing date: 1) Currently PSM2=1 is required in your build environment in order to build s/mime. For some reason building PSM is not a requirement when building seamonkey. So many people currently aren't building with PSM enabled. If we checkin these libmime changes, a lot of folks are going to have broken builds. In fact I'm not sure all the tinderbox machines build with PSM2=1 defined either! What can we do about this? We could wrap some #ifdefs in libmime around this environment variable. If PSM isn't built, we won't try to include the PSM libraries. Furthermore, it would be really nice if the old smime stub was present if PSM isn't getting built. That way we'll have the old behavior of saying "this message is signed/encrypted and you currently don't have the capability to view it". I'm most worried about making mailnes/mime build without PSM2 defined though. In addition to that we should only build the extensions/smime directory if PSM2 is defined as well. 2) We'll need to make changes to the packaging lists for the new msgsmime dll and .xpt file for the 3 platforms.
I've checked in the projects and added the jar.mn file to the mac build scripts. I don't see the UI for selecting a certificate on the mac anymore, though. I'm not sure if it's because I added the jar.mn file incorrectly or not. The branch builds on Mac right now, but I can't seem to get at the S/MIME features :(
Javi, When you say you can't see the dialog that let's you pick certs anymore, do you mean in the account manager, the security panel is not showing up? Or just the button inside the security panel? When you bring up a compose window, are the security options listed under the "Options" menu? Or is that not loading either.
Javi, I sent you mail but I'm guessing part of the problem is that smime-service.js is probably not gettin put in the components directory on the Mac. I added a MANIFEST file to extensions/smime/src. You'll want to add that to the InstallComponentFiles in MozillaBuildList.pm. All of this should probably be under a smime option. I sent email to Simon and cc'ed you on it talking about some of these issues. Let me know if something else comes up I can help with.
Tomorrow might be a good time to generate a new set of test builds for windows. I checked in some packaging file changes into the branch for windows to export the new DLL and XPT file. That would give us a chance to test things out after all my changes. (I also checked in a fix for the security settings not being saved). I'm not sure if the mac is still having run time problems or not. Javi, can you let us know if my suggestion from this afternoon fixed your problem?
1) I have checked in updated makefiles for Linux. It builds, and I can successfully open the preferences panel, it includes the security section, and the preferences works, options get saved. 2) I will now look at how to add the resources (currently, the smime menu items are not available). 3) When viewing an encrypted or signed message, the smime code is currently not triggered, no decryption, no "is signed" feedback. I have to figure out how to enable this.
kai, The reason the mime code is not triggered is because you must define ENABLE_SMIME when you build mime/src. This is necessary in order to allow people to build mailnews who do not build PSM by default. I got this working for windows, so update mailnews/mime to see what I did.
I added the InstallResources and CreateJarFromManifest calls to the Mac build script and I am still not able to see the "Security" section under the Mailnews Account settings. I'm gonna be working today with David to mimic the build behavior he did on Win32 for deciding when to build smime.
David, thanks, this helped. I just checked in more Linux makefile changes. Reading signed and encrypted makefiles now works. I made the makefiles dependent on whether PSM is built or not, as in Windows. The menu overlay is not yet visible in Linux. I just compiled Windows, and while the menu is visible there, you can't check it, an exception shows up.
Kai, what exception do you get on windows? That seems to be working for me. I'll make sure everything I have is checked in.
The menu works for me as well on windows. Have you tried a clean pull and build?
Attachment #55701 - Attachment is obsolete: true
Attached patch mime module changes (deleted) — Splinter Review
All 3 sets of diffs which I posted today will need reviewed and sr reviewed before we can land this on the trunk. Javi/Kai/David, if you make any changes to mailnews on the otis test branch can you send me email so I can move your change onto the trunk build in my tree which has all of this stuff on a trunk tree? Things blocking us from landing: 1) reviews on these 3 patches 2) Kai says linux doesn't have the security overlays in the compose window. 3) Last I heard from Javi on Tuesday, mac wasn't showing the security panel in the account wizard 4) Verify that mac and linux still build mailnews when PSM isn't built. I verified this was true on windows and things behaved nicely. 5) test builds on linux, windows and mac which all pass go after all these changes. When these are all done, 6) approval from drivers
My windows problem was caused by not having the certificates set... Regarding Linux: I checked whether the functionality to send encrypted and/or signed messages is working - it is. When I set the default to always encrypt and sign using the preferences panel, sending such a message works. But still, the Options/Security menu does not show up on Linux.
Comment on attachment 55965 [details] [diff] [review] mailnews/compose changes synched against the trunk (needs review) looks good. R=ducarroz
Attachment #55965 - Flags: review+
the compose patch has been checked into the trunk. That leaves for tomorrow of Friday the last 2 patches: mailnews/extensions/smime and mailnews/mime/src
I've checked in that patch. without it, sending mail will fail, as the S/MIME extension is not built yet. we fail to create an instance of NS_MSGCOMPOSESECURE_CONTRACTID, which causes BeginCryptoEncapsulation() to return failure, which calls nsMsgSendPart::Write () to return failure. we should evaluate BeginCryptoEncapsulation() and make sure that when it returns error, it's really an error.
I've checked in more fixes to Linux makefiles. There is one problem left however. While I was able to hook the smime extensions's chrome into the registry, and make the overlay menu appear in the compose menu, it didn't display any text. The menu was there, but it just displayed empty space. You can open that submenu and see the three submenu items, you can control them by selecting or unselecting them, but again for those, no text is displayed, just empty space. I made sure that the locale chrome files are registered, too. After digging around I came up with the following fix that makes it work. For some reason the runtime environment does not find the file specified by <!DOCTYPE overlay SYSTEM "chrome://messenger-smime/locale/msgCompSMIMEOverlay.dtd"> By moving this address to chrome://messenger/ the strings appear in the menu. Is it possible, that some global registry needs to be updated if we want to use the new messenger-smime toplevel path in chrome? This seems to be a Linux problem only, as we have seen the menu already worked in Windows. But I guess my fix should work on Windows, too. Here it is: Index: mozilla/mailnews/extensions/smime/jar.mn =================================================================== RCS file: /cvsroot/mozilla/mailnews/extensions/smime/Attic/jar.mn,v retrieving revision 1.1.2.2 diff -u -d -r1.1.2.2 jar.mn --- mozilla/mailnews/extensions/smime/jar.mn 2001/11/01 15:46:11 1.1.2.2 +++ mozilla/mailnews/extensions/smime/jar.mn 2001/11/01 15:50:27 @@ -9,4 +9,4 @@ locale/en-US/messenger/am-smime.dtd (resources/locale/en-US/am-smime.dtd) locale/en-US/messenger/am-smime.properties (resources/locale/en-US/am-smime.properties) locale/en-US/messenger-smime/contents.rdf (resources/locale/en-US/contents.rdf) - locale/en-US/messenger-smime/msgCompSMIMEOverlay.dtd (resources/locale/en-US/msgCompSMIMEOverlay.dtd) + locale/en-US/messenger/msgCompSMIMEOverlay.dtd (resources/locale/en-US/msgCompSMIMEOverlay.dtd) Index: mozilla/mailnews/extensions/smime/resources/content/msgCompSMIMEOverlay.xul =================================================================== RCS file: /cvsroot/mozilla/mailnews/extensions/smime/resources/content/Attic/msgCompSMIMEOverlay.xul,v retrieving revision 1.1.2.1 diff -u -d -r1.1.2.1 msgCompSMIMEOverlay.xul --- mozilla/mailnews/extensions/smime/resources/content/msgCompSMIMEOverlay.xul 2001/10/29 21:20:42 1.1.2.1 +++ mozilla/mailnews/extensions/smime/resources/content/msgCompSMIMEOverlay.xul 2001/11/01 15:50:27 @@ -24,7 +24,7 @@ --> -<!DOCTYPE overlay SYSTEM "chrome://messenger-smime/locale/msgCompSMIMEOverlay.dtd"> +<!DOCTYPE overlay SYSTEM "chrome://messenger/locale/msgCompSMIMEOverlay.dtd"> <overlay xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
As Scott suggested, I started two test builds to see whether mailnews and psm as optional components works nicely on Linux, too. I started a build with PSM, but without mailnews. I started another without PSM, but including mailnews. When the builds have finished and I have done some basic testing, I will another comment.
Both combinations build and work fine.
CC'ing sspitzer. Seth: You said in this bug, you were unable to send mail and have created and checked in a patch. I can confirm what you say - without your patch I'm unable to send mail in the build without PSM. But with your checkin something must have gone wrong - it was not contained in the otis branch. As your patch fixes this for me, I have just checked it in.
Kai, changing the chrome name space of the .dtd file for the overlay is not the right thing to do. Please don't check that patch in. On linux are you sure your gettng the contents.rdf file in smime\resources\locale\en-US? that file defines the new chrome name space that the overlay's dtd file goes in.
>But with your checkin something must have gone wrong - it was not contained in >the otis branch. my checkin was on the trunk only. I'm not on the branch. > As your patch fixes this for me, I have just checked it in. great, thanks!
> Kai, changing the chrome name space of the .dtd file for the overlay is not the > right thing to do. Please don't check that patch in. It is in already, but no problem to change it back if we find the real solution. > On linux are you sure your gettng the contents.rdf file in > smime\resources\locale\en-US? > > that file defines the new chrome name space that the overlay's dtd file goes in. I believe yes. I've added this code to extensions/smime/Makefile.in: install:: $(INSTALL) $(srcdir)/resources/content/smime.js $(DIST)/bin/defaults/pref @$(REGCHROME) content messenger-smime messenger.jar @$(REGCHROME) locale en-US/messenger-smime en-US.jar When I start Mozilla, the output includes the following: *** Chrome Registration of package: Checking for contents.rdf at jar:resource:/chrome/messenger.jar!/content/messenger-smime/ *** Chrome Registration of locale: Checking for contents.rdf at jar:resource:/chrome/en-US.jar!/locale/en-US/messenger-smime/ I compared the contents of file all-locales.rdf from Windows and Linux. Both contain the same bits about smime. There was one difference, in Windows, the description section comes first. I changed that order manually on Linux, but with no effect. I checked the contents of the en-US.jar file using a jar tool: 635 Thu Nov 01 16:32:20 CET 2001 locale/en-US/messenger/am-smime.dtd 90 Mon Oct 29 22:21:50 CET 2001 locale/en-US/messenger/am-smime.properties 823 Mon Oct 29 22:21:50 CET 2001 locale/en-US/messenger-smime/contents.rdf 305 Mon Oct 29 22:21:54 CET 2001 locale/en-US/messenger-smime/msgCompSMIMEOverlay.dtd 1030 Wed Aug 22 02:50:38 CEST 2001 locale/en-US/messenger/smime.properties The interesting thing is, that although I changed that jar file, the file msgCompSMIMEOverlay.dtd still is in subdirectory messenger-smime. I tried, and the change to jar.mn is actually not necessary, regardless of whether messenger or messenger-smime is used as the path there. The change that makes it work is the !DOCTYPE tag in the XUL file. Broken with "messenger-smime", working with "messenger".
Once mscott lands the tip of the OTIS_TEST_BRANCH of mozilla/mailnews onto the trunk of the mozilla builds, then my previous patch is required to enable S/MIME in the Mac builds.
Comment on attachment 55981 [details] [diff] [review] mime module changes >+ >+ PR_ASSERT(match == PR_TRUE); why do we care about this assert if we are just going to force match to be true right >+static char * >+MimeCMS_generate (void *crypto_closure) >+{ >+ MimeCMSdata *data = (MimeCMSdata *) crypto_closure; >+ PRBool self_signed_p = PR_FALSE; >+ PRBool self_encrypted_p = PR_FALSE; >+ PRBool union_encrypted_p = PR_FALSE; >+ PRBool good_p = PR_TRUE; >+ PRBool unverified_p = PR_FALSE; >+ >+ PR_ASSERT(data && data->output_fn); do we need this assert? >+ if (!data || !data->output_fn) return 0; >+ >+ if (data->content_info) >+ { >+ data->content_info->ContentIsSigned(&self_signed_p); >+ data->content_info->ContentIsEncrypted(&self_encrypted_p); >+ union_encrypted_p = (self_encrypted_p || data->parent_is_encrypted_p); >+ >+ if (self_signed_p) >+ { >+ PR_SetError(0, 0); >+ good_p = data->content_info->VerifySignature(); >+#if 0 // XXX Fix this XXX // >+ good_p = SEC_PKCS7VerifySignature(data->content_info, >+ 0, /*certUsageEmailSigner */ /* #### */ >+ PR_TRUE); /* #### keepcerts */ >+#endif >+ if (!good_p) >+ { >+ if (!data->verify_error) >+ data->verify_error = PR_GetError(); >+ PR_ASSERT(data->verify_error < 0); >+ if (data->verify_error >= 0) >+ data->verify_error = -1; >+ } >+ else >+ { >+ good_p = MimeCMSHeadersAndCertsMatch(data->self, >+ data->content_info, >+ &data->sender_addr); >+ if (!good_p && !data->verify_error) >+ // data->verify_error = SEC_ERROR_CERT_ADDR_MISMATCH; XXX Fix later XXX // >+ data->verify_error = -1; >+ } >+ } >+ >+#if 0 >+ if (SEC_PKCS7ContainsCertsOrCrls(data->content_info)) >+ { >+ /* #### call libsec telling it to import the certs */ >+ } >+#endif >+ >+ /* Don't free these yet -- keep them around for the lifetime of the >+ MIME object, so that we can get at the security info of sub-parts >+ of the currently-displayed message. */ >+#if 0 >+ SEC_PKCS7DestroyContentInfo(data->content_info); >+ data->content_info = 0; >+#endif /* 0 */ >+ } >+ else >+ { >+ /* No content info? Something's horked. Guess. */ >+ self_encrypted_p = PR_TRUE; >+ union_encrypted_p = PR_TRUE; >+ good_p = PR_FALSE; >+ if (!data->decode_error && !data->verify_error) >+ data->decode_error = -1; >+ } >+ >+ unverified_p = data->self->options->missing_parts; >+ >+ PR_ASSERT(data->self); >+ if (data->self && data->self->parent) >+ // mime_set_crypto_stamp(data->self->parent, self_signed_p, self_encrypted_p); XXX Fix later XXX // >+ >+ >+ { >+ char *stamp_url = 0, *result = nsnull; >+ if (data->self) >+ { >+ if (unverified_p && data->self->options) >+ // stamp_url = IMAP_CreateReloadAllPartsUrl(data->self->options->url); XXX Fix later XXX // >+ stamp_url = nsnull; >+ else >+ stamp_url = MimeCMS_MakeSAURL(data->self); >+ } >+ >+#if 0 // XXX Fix this later XXX // what do we need to do here for later? do we need a bug for it? >+ result = >+ MimeHeaders_make_crypto_stamp (union_encrypted_p, >+ self_signed_p, >+ good_p, >+ unverified_p, >+ data->parent_holds_stamp_p, >+ stamp_url); >+#endif >+ PR_FREEIF(stamp_url); >+ return result; >+ } >+} >Index: src/mimecms.h >=================================================================== >RCS file: mimecms.h >diff -N mimecms.h >--- /dev/null Wed Apr 26 15:53:02 2000 >+++ mimecms.h Wed Oct 31 14:19:13 2001 >@@ -0,0 +1,37 @@ >+/* Insert copyright and license here 1996 */ i added a license here >Index: src/mimecryp.cpp >=================================================================== >+ PR_ASSERT(!enc->crypto_closure); can we get rid of this assert? >+static int >+MimeEncrypted_parse_end (MimeObject *obj, PRBool abort_p) >+{ >+ MimeEncrypted *enc = (MimeEncrypted *) obj; >+ >+ /* Don't free these yet -- keep them around for the lifetime of the >+ MIME object, so that we can get at the security info of sub-parts >+ of the currently-displayed message. */ >+#if 0 can we get rid of this # if 0 code? >+ if (enc->crypto_closure) >+ { >+ ((MimeEncryptedClass *) obj->class)->crypto_free (enc->crypto_closure); >+ enc->crypto_closure = 0; >+ } >+#endif /* 0 */ >+ >+ return ((MimeObjectClass*)&MIME_SUPERCLASS)->parse_end (obj, abort_p); >+} >+ >+ >+static int >+MimeEncrypted_add_child (MimeObject *parent, MimeObject *child) >+{ >+ MimeContainer *cont = (MimeContainer *) parent; >+ PR_ASSERT(parent && child); >+ if (!parent || !child) return -1; >+ >+ /* Encryption containers can only have one child. */ >+ PR_ASSERT(cont->nchildren == 0); >+ if (cont->nchildren != 0) return -1; >+ >+ return ((MimeContainerClass*)&MIME_SUPERCLASS)->add_child (parent, child); >+} >+ >+ >+static int >+MimeEncrypted_emit_buffered_child(MimeObject *obj) >+{ >+ MimeEncrypted *enc = (MimeEncrypted *) obj; >+ if (enc->crypto_closure && >+ obj->options && >+ obj->options->headers != MimeHeadersCitation && >+ obj->options->write_html_p && >+ obj->options->output_fn) >+ // && !mime_crypto_object_p(enc->hdrs, PR_TRUE)) // XXX fix later XXX // >+ { >+ char *html; >+#if 0 // XXX Fix this later XXX // what do we need to fix? >+ char *html = (((MimeEncryptedClass *) obj->clazz)->crypto_generate_html >+ (enc->crypto_closure)); >+ if (!html) return -1; /* MK_OUT_OF_MEMORY? */ >+ >+ status = MimeObject_write(obj, html, nsCRT::strlen(html), PR_FALSE); >+ PR_FREEIF(html); >+ if (status < 0) return status; >+#endif >+
blast it....most of my review comments didn't show up. I'm still only a 1/4 of the way through on the libmime patch though. Most of my comments are about a lot PR_ASSERT calls when I wasn't sure if we needed them. I seem to remember that PR_ASSERT actually aborts Linux builds. So we try to use them sparingly..... Instead of doing my review in line on the attachment I'll make a separate attachment listing my comments on the mime patch.
Comment on attachment 55965 [details] [diff] [review] mailnews/compose changes synched against the trunk (needs review) this had an sr=sspitzer from yesterday and is already in the trunk.
Attachment #55965 - Flags: superreview+
mime module changes will be reviewed by JS, sr'ed by myself smime extension changes will be reviewed by ? and sr'ed by seth (except for nsMsgComposeSecure which I'll sr)
Comment on attachment 55972 [details] [diff] [review] diffs for mailnews/extensions/smime module sr=sspitzer our first extension!
Attachment #55972 - Flags: superreview+
Attached file review comments on the mime changes (deleted) —
Comment on attachment 55972 [details] [diff] [review] diffs for mailnews/extensions/smime module r=mscott
Attachment #55972 - Flags: review+
Comment on attachment 56187 [details] review comments on the mime changes I have incorporated Scotts comments into the code and checked them into the OTIS_TEST_BRANCH. Someone will have to merge these onto the trunk.
Comment on attachment 55981 [details] [diff] [review] mime module changes sr=mscott after looking at David's changes.
Attachment #55981 - Flags: superreview+
I just merged david's changes from the otis branch onto my trunk build so when I check in we'll get those changes. I also sr'ed the patch after looking at his changes. I'll post a diff showing just his additional changes to the bug for historical purposes. How's the review going JF? I think your the last stop before we land! Yippee....
Comment on attachment 56063 [details] [diff] [review] supplimental patch needed. with out this, build without the S/MIME extension will fail to send mail. this is already in the trunk so i'm just obsoletetin'g this for clarity purposes.
Attachment #56063 - Attachment is obsolete: true
Comment on attachment 54981 [details] [diff] [review] New patch. obsoleting for clarity purposes since we've re-arrange this intial path and broken it into new patches.
Attachment #54981 - Attachment is obsolete: true
Comment on attachment 56127 [details] [diff] [review] Patch required to Mac build scripts to enable S/MIME support R=ducarroz
Attachment #56127 - Flags: review+
Comment on attachment 55981 [details] [diff] [review] mime module changes Make sure every new file has the correct licence and copyright. Also looks like new files use tab instead of 2 chars. Be sure mimehdrs.cpp has a new line at the end. R=ducarroz
Attachment #55981 - Flags: review+
Comment on attachment 56263 [details] [diff] [review] the additional changes David made after my sr (made against the first mime patch you need to review both) R=ducarroz
Attachment #56263 - Flags: review+
Comment on attachment 55981 [details] [diff] [review] mime module changes I've open a separate bug for the tab removal. http://bugzilla.mozilla.org/show_bug.cgi?id=108241
*** Bug 110272 has been marked as a duplicate of this bug. ***
where can us mortals find a nightly or recent build with s/mime support for testing? it isn't in the usual nightly, is it?
It should be included. Look at mailnews preferences, security settings. Configure the certificates you want to use. Compose a message. Look at options security. Send yourself a signed/encrypted message. When you view it, some message boxes should appear indicating the state of the message.
That is wrong, it does not work.(only receiving mails) Take a look at bug 110272.
I just used Netscape 4.79 to send an encrypted mail to myself, but when I click cancel when asked to enter master password to decrypt(in mozilla), it tells me the encryption was succesful!!?! But it does show an empty body afterwards. I also just crashed when trying to edit an encrypted message. (crash data sent to you)
Can this initial S/MIME landing tracking bug be closed now?
Marking fixed.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
This has landed, and there are specific UI/functionality bugs logged separately now. Verified with build 2002-01-30...
Status: RESOLVED → VERIFIED
QA Contact: esther → stephend
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: