Bug 1633665 - fix (and code cleanup) for broken list of recipient S/MIME certificates in composer "view / message security info". r=benc DONTBUILD
authorMagnus Melin <mkmelin+mozilla@iki.fi>
Thu, 30 Apr 2020 13:55:59 +0300
changeset 38081 e1fe33ee1a8be4ede29cc6119e9554f5991267fd
parent 38080 351f3b3d81d9dcdc38dfda194199e194cd2e7e89
child 38082 5e44cc4ebd09802359b93c425fe2ebe6a245d128
push id2595
push userclokep@gmail.com
push dateMon, 04 May 2020 19:02:04 +0000
treeherdercomm-beta@f53913797371 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbenc
bugs1633665
Bug 1633665 - fix (and code cleanup) for broken list of recipient S/MIME certificates in composer "view / message security info". r=benc DONTBUILD
mail/components/compose/content/MsgComposeCommands.js
mail/components/compose/content/messengercompose.xhtml
mail/extensions/openpgp/content/ui/enigmailMsgComposeOverlay.js
mail/extensions/smime/content/msgCompSMIMEOverlay.js
mail/extensions/smime/jar.mn
mailnews/extensions/smime/content/msgCompSecurityInfo.js
mailnews/extensions/smime/public/nsISMimeJSHelper.idl
mailnews/extensions/smime/src/nsSMimeJSHelper.cpp
suite/extensions/smime/content/msgCompSMIMEOverlay.js
--- a/mail/components/compose/content/MsgComposeCommands.js
+++ b/mail/components/compose/content/MsgComposeCommands.js
@@ -4014,48 +4014,50 @@ function GetCharsetUIString() {
 function onSendSMIME() {
   let emailAddresses = [];
 
   try {
     if (!gMsgCompose.compFields.composeSecure.requireEncryptMessage) {
       return;
     }
 
-    Cc["@mozilla.org/messenger-smime/smimejshelper;1"]
+    emailAddresses = Cc["@mozilla.org/messenger-smime/smimejshelper;1"]
       .createInstance(Ci.nsISMimeJSHelper)
-      .getNoCertAddresses(gMsgCompose.compFields, emailAddresses);
+      .getNoCertAddresses(gMsgCompose.compFields);
   } catch (e) {
     return;
   }
 
-  if (emailAddresses.length > 0) {
-    // The rules here: If the current identity has a directoryServer set, then
-    // use that, otherwise, try the global preference instead.
-
-    let autocompleteDirectory;
-
-    // Does the current identity override the global preference?
-    if (gCurrentIdentity.overrideGlobalPref) {
-      autocompleteDirectory = gCurrentIdentity.directoryServer;
-    } else if (Services.prefs.getBoolPref("ldap_2.autoComplete.useDirectory")) {
-      // Try the global one
-      autocompleteDirectory = Services.prefs.getCharPref(
-        "ldap_2.autoComplete.directoryServer"
-      );
-    }
-
-    if (autocompleteDirectory) {
-      window.openDialog(
-        "chrome://messenger-smime/content/certFetchingStatus.xhtml",
-        "",
-        "chrome,modal,resizable,centerscreen",
-        autocompleteDirectory,
-        emailAddresses.value
-      );
-    }
+  if (emailAddresses.length == 0) {
+    return;
+  }
+
+  // The rules here: If the current identity has a directoryServer set, then
+  // use that, otherwise, try the global preference instead.
+
+  let autocompleteDirectory;
+
+  // Does the current identity override the global preference?
+  if (gCurrentIdentity.overrideGlobalPref) {
+    autocompleteDirectory = gCurrentIdentity.directoryServer;
+  } else if (Services.prefs.getBoolPref("ldap_2.autoComplete.useDirectory")) {
+    // Try the global one
+    autocompleteDirectory = Services.prefs.getCharPref(
+      "ldap_2.autoComplete.directoryServer"
+    );
+  }
+
+  if (autocompleteDirectory) {
+    window.openDialog(
+      "chrome://messenger-smime/content/certFetchingStatus.xhtml",
+      "",
+      "chrome,modal,resizable,centerscreen",
+      autocompleteDirectory,
+      emailAddresses
+    );
   }
 }
 
 // Add-ons can override this to customize the behavior.
 function DoSpellCheckBeforeSend() {
   return Services.prefs.getBoolPref("mail.SpellCheckBeforeSend");
 }
 
--- a/mail/components/compose/content/messengercompose.xhtml
+++ b/mail/components/compose/content/messengercompose.xhtml
@@ -88,17 +88,16 @@
 <script src="chrome://messenger/content/messengercompose/addressingWidgetOverlay.js"/>
 <script src="chrome://global/content/contentAreaUtils.js"/>
 <script src="chrome://messenger/content/viewZoomOverlay.js"/>
 #ifdef XP_MACOSX
 <script src="chrome://global/content/macWindowMenu.js"/>
 #endif
 <script src="chrome://communicator/content/utilityOverlay.js"/>
 <script src="chrome://messenger/content/toolbarIconColor.js"/>
-<script src="chrome://messenger-smime/content/msgCompSMIMEOverlay.js"/>
 #ifdef MOZ_OPENPGP
 <script src="chrome://openpgp/content/BondOpenPGP.jsm"/>
 <script src="chrome://openpgp/content/ui/enigmailMsgComposeOverlay.js"/>
 <script src="chrome://openpgp/content/ui/enigmailMsgComposeHelper.js"/>
 #endif
 
 <commandset id="composeCommands">
   <commandset id="msgComposeCommandUpdate"
--- a/mail/extensions/openpgp/content/ui/enigmailMsgComposeOverlay.js
+++ b/mail/extensions/openpgp/content/ui/enigmailMsgComposeOverlay.js
@@ -1869,46 +1869,32 @@ Enigmail.msg = {
 
   /**
    * check if S/MIME encryption can be enabled
    *
    * @return: Boolean - true: keys for all recipients are available
    */
   isSmimeEncryptionPossible() {
     let id = getCurrentIdentity();
-
     if (id.getUnicharAttribute("encryption_cert_name") === "") {
       return false;
     }
 
-    // enable encryption if keys for all recipients are available
-
-    let missingCount = {};
-    let emailAddresses = {};
-
+    // Enable encryption if keys for all recipients are available.
     try {
       if (!gMsgCompose.compFields.hasRecipients) {
         return false;
       }
-      Cc["@mozilla.org/messenger-smime/smimejshelper;1"]
+      let addresses = Cc["@mozilla.org/messenger-smime/smimejshelper;1"]
         .createInstance(Ci.nsISMimeJSHelper)
-        .getNoCertAddresses(
-          gMsgCompose.compFields,
-          missingCount,
-          emailAddresses
-        );
+        .getNoCertAddresses(gMsgCompose.compFields);
+      return addresses.length == 0;
     } catch (e) {
       return false;
     }
-
-    if (missingCount.value === 0) {
-      return true;
-    }
-
-    return false;
   },
 
   /* Manage the wrapping of inline signed mails
    *
    * @wrapresultObj: Result:
    * @wrapresultObj.cancelled, true if send operation is to be cancelled, else false
    * @wrapresultObj.usePpgMime, true if message send option was changed to PGP/MIME, else false
    */
deleted file mode 100644
--- a/mail/extensions/smime/content/msgCompSMIMEOverlay.js
+++ /dev/null
@@ -1,4 +0,0 @@
-/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
--- a/mail/extensions/smime/jar.mn
+++ b/mail/extensions/smime/jar.mn
@@ -1,15 +1,14 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 messenger.jar:
 % content messenger-smime %content/messenger-smime/
-   content/messenger-smime/msgCompSMIMEOverlay.js                   (content/msgCompSMIMEOverlay.js)
    content/messenger-smime/msgHdrViewSMIMEOverlay.js                (content/msgHdrViewSMIMEOverlay.js)
    content/messenger/certpicker.js                                  (../../../mailnews/extensions/smime/content/certpicker.js)
    content/messenger/certpicker.xhtml                               (../../../mailnews/extensions/smime/content/certpicker.xhtml)
    content/messenger-smime/msgReadSMIMEOverlay.js                   (../../../mailnews/extensions/smime/content/msgReadSMIMEOverlay.js)
    content/messenger-smime/msgCompSecurityInfo.js                   (../../../mailnews/extensions/smime/content/msgCompSecurityInfo.js)
    content/messenger-smime/msgCompSecurityInfo.xhtml                (../../../mailnews/extensions/smime/content/msgCompSecurityInfo.xhtml)
    content/messenger-smime/msgReadSecurityInfo.xhtml                (../../../mailnews/extensions/smime/content/msgReadSecurityInfo.xhtml)
    content/messenger-smime/msgReadSecurityInfo.js                   (../../../mailnews/extensions/smime/content/msgReadSecurityInfo.js)
--- a/mailnews/extensions/smime/content/msgCompSecurityInfo.js
+++ b/mailnews/extensions/smime/content/msgCompSecurityInfo.js
@@ -3,79 +3,76 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 var gListBox;
 var gViewButton;
 var gBundle;
 
-var gEmailAddresses;
-var gCertIssuedInfos;
-var gCertExpiresInfos;
 var gCerts;
-var gCount;
-
-var gSMimeContractID = "@mozilla.org/messenger-smime/smimejshelper;1";
-var gISMimeJSHelper = Ci.nsISMimeJSHelper;
-var gIX509Cert = Ci.nsIX509Cert;
-var nsICertificateDialogs = Ci.nsICertificateDialogs;
-var nsCertificateDialogs = "@mozilla.org/nsCertificateDialogs;1";
 
 function onLoad() {
-  var params = window.arguments[0];
+  let params = window.arguments[0];
   if (!params) {
     return;
   }
 
-  var helper = Cc[gSMimeContractID].createInstance(gISMimeJSHelper);
-
-  if (!helper) {
-    return;
-  }
+  let helper = Cc[
+    "@mozilla.org/messenger-smime/smimejshelper;1"
+  ].createInstance(Ci.nsISMimeJSHelper);
 
   gListBox = document.getElementById("infolist");
   gViewButton = document.getElementById("viewCertButton");
   gBundle = document.getElementById("bundle_smime_comp_info");
 
-  gEmailAddresses = {};
-  gCertIssuedInfos = {};
-  gCertExpiresInfos = {};
-  gCerts = {};
-  gCount = {};
-  var canEncrypt = {};
+  let allow_ldap_cert_fetching = params.smFields.requireEncryptMessage;
 
-  var allow_ldap_cert_fetching = params.smFields.requireEncryptMessage;
+  let emailAddresses = [];
+  let certIssuedInfos = [];
+  let certExpiresInfos = [];
+  let certs = [];
+  let canEncrypt = false;
 
   while (true) {
     try {
+      // Out parameters - must be objects.
+      let outEmailAddresses = {};
+      let outCertIssuedInfos = {};
+      let outCertExpiresInfos = {};
+      let outCerts = {};
+      let outCanEncrypt = {};
       helper.getRecipientCertsInfo(
         params.compFields,
-        gEmailAddresses,
-        gCertIssuedInfos,
-        gCertExpiresInfos,
-        gCerts,
-        canEncrypt
+        outEmailAddresses,
+        outCertIssuedInfos,
+        outCertExpiresInfos,
+        outCerts,
+        outCanEncrypt
       );
+      // Unwrap to the actual values.
+      emailAddresses = outEmailAddresses.value;
+      certIssuedInfos = outCertIssuedInfos.value;
+      certExpiresInfos = outCertExpiresInfos.value;
+      gCerts = certs = outCerts.value;
+      canEncrypt = outCanEncrypt.value;
     } catch (e) {
       dump(e);
       return;
     }
 
     if (!allow_ldap_cert_fetching) {
       break;
     }
-
     allow_ldap_cert_fetching = false;
 
-    var missing = [];
-
-    for (let j = gCount.value - 1; j >= 0; --j) {
-      if (!gCerts.value[j]) {
-        missing[missing.length] = gEmailAddresses.value[j];
+    let missing = [];
+    for (let i = 0; i < emailAddresses.length; i++) {
+      if (!certs[i]) {
+        missing.push(emailAddresses[i]);
       }
     }
 
     if (missing.length > 0) {
       var autocompleteLdap = Services.prefs.getBoolPref(
         "ldap_2.autoComplete.useDirectory"
       );
 
@@ -97,81 +94,72 @@ function onLoad() {
             autocompleteDirectory,
             missing
           );
         }
       }
     }
   }
 
-  if (gBundle) {
-    var yes_string = gBundle.getString("StatusYes");
-    var no_string = gBundle.getString("StatusNo");
-    var not_possible_string = gBundle.getString("StatusNotPossible");
-
-    var signed_element = document.getElementById("signed");
-    var encrypted_element = document.getElementById("encrypted");
-
-    if (params.smFields.requireEncryptMessage) {
-      if (params.isEncryptionCertAvailable && canEncrypt.value) {
-        encrypted_element.value = yes_string;
-      } else {
-        encrypted_element.value = not_possible_string;
-      }
+  let signedElement = document.getElementById("signed");
+  let encryptedElement = document.getElementById("encrypted");
+  if (params.smFields.requireEncryptMessage) {
+    if (params.isEncryptionCertAvailable && canEncrypt) {
+      encryptedElement.value = gBundle.getString("StatusYes");
     } else {
-      encrypted_element.value = no_string;
+      encryptedElement.value = gBundle.getString("StatusNotPossible");
     }
-
-    if (params.smFields.signMessage) {
-      if (params.isSigningCertAvailable) {
-        signed_element.value = yes_string;
-      } else {
-        signed_element.value = not_possible_string;
-      }
-    } else {
-      signed_element.value = no_string;
-    }
+  } else {
+    encryptedElement.value = gBundle.getString("StatusNo");
   }
 
-  var imax = gCount.value;
+  if (params.smFields.signMessage) {
+    if (params.isSigningCertAvailable) {
+      signedElement.value = gBundle.getString("StatusYes");
+    } else {
+      signedElement.value = gBundle.getString("StatusNotPossible");
+    }
+  } else {
+    signedElement.value = gBundle.getString("StatusNo");
+  }
 
-  for (let i = 0; i < imax; ++i) {
+  for (let i = 0; i < emailAddresses.length; ++i) {
     let email = document.createXULElement("label");
-    email.setAttribute("value", gEmailAddresses.value[i]);
+    email.setAttribute("value", emailAddresses[i]);
     email.setAttribute("crop", "end");
     email.setAttribute("style", "width: var(--recipientWidth)");
 
     let listitem = document.createXULElement("richlistitem");
     listitem.appendChild(email);
 
-    if (!gCerts.value[i]) {
+    if (!certs[i]) {
       let notFound = document.createXULElement("label");
       notFound.setAttribute("value", gBundle.getString("StatusNotFound"));
       notFound.setAttribute("style", "width: var(--statusWidth)");
-
       listitem.appendChild(notFound);
     } else {
       let status = document.createXULElement("label");
       status.setAttribute("value", "?"); // temporary placeholder
       status.setAttribute("crop", "end");
       status.setAttribute("style", "width: var(--statusWidth)");
+      listitem.appendChild(status);
+
       let issued = document.createXULElement("label");
-      issued.setAttribute("value", gCertIssuedInfos.value[i]);
+      issued.setAttribute("value", certIssuedInfos[i]);
       issued.setAttribute("crop", "end");
       issued.setAttribute("style", "width: var(--issuedWidth)");
+      listitem.appendChild(issued);
+
       let expire = document.createXULElement("label");
-      expire.setAttribute("value", gCertExpiresInfos.value[i]);
+      expire.setAttribute("value", certExpiresInfos[i]);
       expire.setAttribute("crop", "end");
       expire.setAttribute("style", "width: var(--expireWidth)");
-
-      listitem.appendChild(status);
-      listitem.appendChild(issued);
       listitem.appendChild(expire);
 
-      asyncDetermineUsages(gCerts.value[i]).then(results => {
+      asyncDetermineUsages(certs[i]).then(results => {
         let someError = results.some(
           result => result.errorCode !== PRErrorCodeSuccess
         );
         if (!someError) {
           status.setAttribute("value", gBundle.getString("StatusValid"));
           return;
         }
 
@@ -284,17 +272,17 @@ function viewCertHelper(parent, cert) {
     "chrome://pippki/content/certViewer.xhtml",
     "_blank",
     "centerscreen,chrome,titlebar",
     cert
   );
 }
 
 function certForRow(aRowIndex) {
-  return gCerts.value[aRowIndex];
+  return gCerts[aRowIndex];
 }
 
 function viewSelectedCert() {
   if (!gViewButton.disabled) {
     viewCertHelper(window, certForRow(gListBox.selectedIndex));
   }
 }
 
--- a/mailnews/extensions/smime/public/nsISMimeJSHelper.idl
+++ b/mailnews/extensions/smime/public/nsISMimeJSHelper.idl
@@ -15,49 +15,31 @@ interface nsIX509Cert;
 
 [scriptable, uuid(a54e3c8f-a000-4901-898f-fafb297b1546)]
 interface nsISMimeJSHelper : nsISupports
 {
   /**
    * Obtains detailed information about the certificate availability
    * status of email recipients.
    *
-   * @param compFields - Attributes of the composed message
-   *
+   * @param compFields - Attributes of the composed message.
    * @param emailAddresses - The list of all recipient email addresses
-   *
    * @param certIssuedInfos - If a recipient cert was found, when has it been issued?
-   *
    * @param certExpiredInfos - If a recipient cert was found, when will it expire?
-   *
    * @param certs - The recipient certificates, which can contain null for not found
-   *
    * @param canEncrypt - whether valid certificates have been found for all recipients
-   *
-   * @exception NS_ERROR_FAILURE - unexptected failure
-   *
-   * @exception NS_ERROR_OUT_OF_MEMORY - could not create the out list
-   *
-   * @exception NS_ERROR_INVALID_ARG
    */
   void getRecipientCertsInfo(in nsIMsgCompFields compFields,
                              out Array<AString> emailAddresses,
                              out Array<AString> certIssuedInfos,
                              out Array<AString> certExpiresInfos,
                              out Array<nsIX509Cert> certs,
                              out boolean canEncrypt);
 
   /**
    * Obtains a list of email addresses where valid email recipient certificates
    * are not yet available.
    *
    * @param compFields - Attributes of the composed message
-   *
    * @returns The list of email addresses without valid certs
-   *
-   * @exception NS_ERROR_FAILURE - unexptected failure
-   *
-   * @exception NS_ERROR_OUT_OF_MEMORY - could not create the out list
-   *
-   * @exception NS_ERROR_INVALID_ARG
    */
   Array<AString> getNoCertAddresses(in nsIMsgCompFields compFields);
 };
--- a/mailnews/extensions/smime/src/nsSMimeJSHelper.cpp
+++ b/mailnews/extensions/smime/src/nsSMimeJSHelper.cpp
@@ -42,19 +42,18 @@ NS_IMETHODIMP nsSMimeJSHelper::GetRecipi
   certs.ClearAndRetainStorage();
   emailAddresses.SetCapacity(mailbox_count);
   certIssuedInfos.SetCapacity(mailbox_count);
   certExpiresInfos.SetCapacity(mailbox_count);
   certs.SetCapacity(mailbox_count);
 
   nsCOMPtr<nsIX509CertDB> certdb = do_GetService(NS_X509CERTDB_CONTRACTID);
 
-  *canEncrypt = false;
+  *canEncrypt = true;
   rv = NS_OK;
-  bool found_blocker = false;
 
   nsCOMPtr<nsIMsgComposeSecure> composeSecure =
       do_CreateInstance(NS_MSGCOMPOSESECURE_CONTRACTID, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
   for (uint32_t i = 0; i < mailbox_count; ++i) {
     const nsCString &email = mailboxes[i];
     nsCOMPtr<nsIX509Cert> cert;
@@ -68,27 +67,23 @@ NS_IMETHODIMP nsSMimeJSHelper::GetRecipi
             email_lowercase, false, getter_AddRefs(cert)))) {
       nsCOMPtr<nsIX509CertValidity> validity;
       rv = cert->GetValidity(getter_AddRefs(validity));
       if (NS_SUCCEEDED(rv)) {
         validity->GetNotBeforeLocalDay(certIssuedInfo);
         validity->GetNotAfterLocalDay(certExpiresInfo);
       }
     } else {
-      found_blocker = true;
+      *canEncrypt = false;
     }
     emailAddresses.AppendElement(NS_ConvertUTF8toUTF16(email));
     certIssuedInfos.AppendElement(certIssuedInfo);
     certExpiresInfos.AppendElement(certExpiresInfo);
     certs.AppendElement(cert);
   }
-  if (mailbox_count > 0 && !found_blocker) {
-    *canEncrypt = true;
-  }
-
   return NS_OK;
 }
 
 NS_IMETHODIMP nsSMimeJSHelper::GetNoCertAddresses(
     nsIMsgCompFields *compFields, nsTArray<nsString> &emailAddresses) {
   NS_ENSURE_ARG_POINTER(compFields);
   emailAddresses.ClearAndRetainStorage();
 
--- a/suite/extensions/smime/content/msgCompSMIMEOverlay.js
+++ b/suite/extensions/smime/content/msgCompSMIMEOverlay.js
@@ -254,27 +254,26 @@ var SecurityController =
       default:
         return false;
     }
   }
 };
 
 function onComposerSendMessage()
 {
-  let emailAddresses = new Object();
+  let emailAddresses = [];
 
   try
   {
     if (!gMsgCompose.compFields.composeSecure.requireEncryptMessage)
       return;
 
-    Cc["@mozilla.org/messenger-smime/smimejshelper;1"]
+    emailAddresses = Cc["@mozilla.org/messenger-smime/smimejshelper;1"]
       .createInstance(Ci.nsISMimeJSHelper)
-      .getNoCertAddresses(gMsgCompose.compFields,
-                                  emailAddresses);
+      .getNoCertAddresses(gMsgCompose.compFields);
   }
   catch (e)
   {
     return;
   }
 
   if (emailAddresses.length > 0)
   {
@@ -296,17 +295,17 @@ function onComposerSendMessage()
           Services.prefs.getCharPref("ldap_2.autoComplete.directoryServer");
     }
 
     if (autocompleteDirectory)
       window.openDialog("chrome://messenger-smime/content/certFetchingStatus.xul",
                         "",
                         "chrome,modal,resizable,centerscreen",
                         autocompleteDirectory,
-                        emailAddresses.value);
+                        emailAddresses);
   }
 }
 
 function onComposerFromChanged()
 {
   if (!gSMFields)
     return;