Closed Bug 106507 Opened 19 years ago Closed 19 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)

1.71 MB, image/jpeg
Details
19.64 KB, patch
bugzilla
: review+
mscott
: superreview+
Details | Diff | Splinter Review
89.83 KB, patch
mscott
: review+
sspitzer
: superreview+
Details | Diff | Splinter Review
95.09 KB, patch
bugzilla
: review+
mscott
: superreview+
Details | Diff | Splinter Review
4.28 KB, patch
bugzilla
: review+
Details | Diff | Splinter Review
4.29 KB, text/plain
Details
62.78 KB, 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) — Splinter Review
Attached patch New patch. (obsolete) — 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
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+
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: 19 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.