Bug 1468225 - remove method nsICertificateDialogs.viewCert and its uses. r=Gijs,keeler
authorDipen Patel <bugzilla@pansara.org>
Wed, 22 Aug 2018 17:38:33 +0000
changeset 474688 3dd6ba40f3bf51dc3513c0ad88c86f62473330b6
parent 474687 e2cedc15272d164e81a4f48cb7115ff012430745
child 474689 e65592761f4cb4562a194cd2bc5c9c907e13f898
push idunknown
push userunknown
push dateunknown
reviewersGijs, keeler
bugs1468225
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1468225 - remove method nsICertificateDialogs.viewCert and its uses. r=Gijs,keeler - Remove the viewCert method from nsICertificateDialogs - Remove all associated C++ code - Directly invoke UI window where it was previous called. - Update tests MozReview-Commit-ID: 9b62Go0DjE9 Differential Revision: https://phabricator.services.mozilla.com/D3358
browser/base/content/pageinfo/security.js
security/manager/pki/nsNSSDialogs.cpp
security/manager/pki/resources/content/pippki.js
security/manager/ssl/nsICertificateDialogs.idl
security/manager/ssl/tests/mochitest/browser/browser_downloadCert_ui.js
security/manager/ssl/tests/unit/test_certDB_import.js
security/manager/ssl/tests/unit/test_certDB_import_with_master_password.js
--- a/browser/base/content/pageinfo/security.js
+++ b/browser/base/content/pageinfo/security.js
@@ -326,18 +326,18 @@ function setText(id, value) {
     element.textContent = value;
   }
 }
 
 function viewCertHelper(parent, cert) {
   if (!cert)
     return;
 
-  var cd = Cc[CERTIFICATEDIALOGS_CONTRACTID].getService(nsICertificateDialogs);
-  cd.viewCert(parent, cert);
+  Services.ww.openWindow(parent, "chrome://pippki/content/certViewer.xul",
+                         "_blank", "centerscreen,chrome", cert);
 }
 
 /**
  * Return true iff realm (proto://host:port) (extracted from uri) has
  * saved passwords
  */
 function realmHasPasswords(uri) {
   return Services.logins.countLogins(uri.prePath, "", "") > 0;
--- a/security/manager/pki/nsNSSDialogs.cpp
+++ b/security/manager/pki/nsNSSDialogs.cpp
@@ -343,30 +343,16 @@ nsNSSDialogs::GetPKCS12FilePassword(nsII
     _password.Assign(pwTemp);
     free(pwTemp);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsNSSDialogs::ViewCert(nsIInterfaceRequestor* ctx, nsIX509Cert* cert)
-{
-  // |ctx| is allowed to be null.
-  NS_ENSURE_ARG(cert);
-
-  // Get the parent window for the dialog
-  nsCOMPtr<mozIDOMWindowProxy> parent = do_GetInterface(ctx);
-  return nsNSSDialogHelper::openDialog(parent,
-                                       "chrome://pippki/content/certViewer.xul",
-                                       cert,
-                                       false /*modal*/);
-}
-
-NS_IMETHODIMP
 nsNSSDialogs::DisplayGeneratingKeypairInfo(nsIInterfaceRequestor *aCtx, nsIKeygenThread *runnable)
 {
   nsresult rv;
 
   // Get the parent window for the dialog
   nsCOMPtr<mozIDOMWindowProxy> parent = do_GetInterface(aCtx);
 
   rv = nsNSSDialogHelper::openDialog(parent,
--- a/security/manager/pki/resources/content/pippki.js
+++ b/security/manager/pki/resources/content/pippki.js
@@ -26,18 +26,18 @@ function setText(id, value) {
 const nsICertificateDialogs = Ci.nsICertificateDialogs;
 const nsCertificateDialogs = "@mozilla.org/nsCertificateDialogs;1";
 
 function viewCertHelper(parent, cert) {
   if (!cert) {
     return;
   }
 
-  var cd = Cc[nsCertificateDialogs].getService(nsICertificateDialogs);
-  cd.viewCert(parent, cert);
+  Services.ww.openWindow(parent, "chrome://pippki/content/certViewer.xul",
+                         "_blank", "centerscreen,chrome", cert);
 }
 
 function getDERString(cert) {
   var length = {};
   var derArray = cert.getRawDER(length);
   var derString = "";
   for (var i = 0; i < derArray.length; i++) {
     derString += String.fromCharCode(derArray[i]);
--- a/security/manager/ssl/nsICertificateDialogs.idl
+++ b/security/manager/ssl/nsICertificateDialogs.idl
@@ -56,24 +56,13 @@ interface nsICertificateDialogs : nsISup
    *  @param ctx A user interface context.
    *  @param password The password provided by the user.
    *
    *  @return false if the user requests to cancel.
    */
   [must_use]
   boolean getPKCS12FilePassword(in nsIInterfaceRequestor ctx,
                                 out AString password);
-
-  /**
-   *  UI shown when a certificate needs to be shown to the user.
-   *  The implementation should try to display as many attributes
-   *  as possible.
-   *
-   *  @param ctx A user interface context.
-   *  @param cert The certificate to be shown to the user.
-   */
-  [must_use]
-  void viewCert(in nsIInterfaceRequestor ctx, in nsIX509Cert cert);
 };
 
 %{C++
 #define NS_CERTIFICATEDIALOGS_CONTRACTID "@mozilla.org/nsCertificateDialogs;1"
 %}
--- a/security/manager/ssl/tests/mochitest/browser/browser_downloadCert_ui.js
+++ b/security/manager/ssl/tests/mochitest/browser/browser_downloadCert_ui.js
@@ -50,74 +50,37 @@ function openCertDownloadDialog(cert) {
                               "", cert, returnVals);
   return new Promise((resolve, reject) => {
     win.addEventListener("load", function() {
       executeSoon(() => resolve([win, returnVals]));
     }, {once: true});
   });
 }
 
-// Mock implementation of nsICertificateDialogs.
-const gCertificateDialogs = {
-  expectedCert: null,
-  viewCertCallCount: 0,
-  confirmDownloadCACert(ctx, cert, trust) {
-    Assert.ok(false, "confirmDownloadCACert() should not have been called");
-  },
-  setPKCS12FilePassword(ctx, password) {
-    Assert.ok(false, "setPKCS12FilePassword() should not have been called");
-  },
-  getPKCS12FilePassword(ctx, password) {
-    Assert.ok(false, "getPKCS12FilePassword() should not have been called");
-  },
-  viewCert(ctx, cert) {
-    this.viewCertCallCount++;
-    Assert.notEqual(cert, null, "Cert to view should not be null");
-    Assert.equal(cert, this.expectedCert,
-                 "Actual and expected cert should match");
-  },
-
-  QueryInterface: ChromeUtils.generateQI([Ci.nsICertificateDialogs])
-};
-
 add_task(async function setup() {
   for (let testCase of TEST_CASES) {
     testCase.cert = await readCertificate(testCase.certFilename, ",,");
     Assert.notEqual(testCase.cert, null,
                     `'${testCase.certFilename}' should have been read`);
   }
-
-  let certificateDialogsCID =
-    MockRegistrar.register("@mozilla.org/nsCertificateDialogs;1",
-                           gCertificateDialogs);
-  registerCleanupFunction(() => {
-    MockRegistrar.unregister(certificateDialogsCID);
-  });
 });
 
 // Test that the trust header message corresponds to the provided cert, and that
 // the View Cert button launches the cert viewer for the provided cert.
 add_task(async function testTrustHeaderAndViewCertButton() {
   for (let testCase of TEST_CASES) {
     let [win] = await openCertDownloadDialog(testCase.cert);
     let expectedTrustHeaderString =
       `Do you want to trust \u201C${testCase.expectedDisplayString}\u201D ` +
       "for the following purposes?";
     Assert.equal(win.document.getElementById("trustHeader").textContent,
                  expectedTrustHeaderString,
                  "Actual and expected trust header text should match for " +
                  `${testCase.certFilename}`);
 
-    gCertificateDialogs.viewCertCallCount = 0;
-    gCertificateDialogs.expectedCert = testCase.cert;
-    info("Pressing View Cert button");
-    win.document.getElementById("viewC-button").doCommand();
-    Assert.equal(gCertificateDialogs.viewCertCallCount, 1,
-                 "viewCert() should've been called once");
-
     await BrowserTestUtils.closeWindow(win);
   }
 });
 
 // Test that the right values are returned when the dialog is accepted.
 add_task(async function testAcceptDialogReturnValues() {
   let [win, retVals] = await openCertDownloadDialog(TEST_CASES[0].cert);
   win.document.getElementById("trustSSL").checked = true;
--- a/security/manager/ssl/tests/unit/test_certDB_import.js
+++ b/security/manager/ssl/tests/unit/test_certDB_import.js
@@ -27,20 +27,16 @@ const gCertificateDialogs = {
   setPKCS12FilePassword: (ctx, password) => {
     // This is only relevant to exporting.
     ok(false, "setPKCS12FilePassword() should not have been called");
   },
   getPKCS12FilePassword: (ctx, password) => {
     // We don't test anything that calls this method yet.
     ok(false, "getPKCS12FilePassword() should not have been called");
   },
-  viewCert: (ctx, cert) => {
-    // This shouldn't be called for import methods.
-    ok(false, "viewCert() should not have been called");
-  },
 
   QueryInterface: ChromeUtils.generateQI([Ci.nsICertificateDialogs])
 };
 
 // Implements nsIInterfaceRequestor. Mostly serves to mock nsIPrompt.
 const gInterfaceRequestor = {
   alert: (title, text) => {
     // We don't test anything that calls this method yet.
--- a/security/manager/ssl/tests/unit/test_certDB_import_with_master_password.js
+++ b/security/manager/ssl/tests/unit/test_certDB_import_with_master_password.js
@@ -27,20 +27,16 @@ const gCertificateDialogs = {
   setPKCS12FilePassword: (ctx, password) => {
     // This is only relevant to exporting.
     ok(false, "setPKCS12FilePassword() should not have been called");
   },
   getPKCS12FilePassword: (ctx, password) => {
     // We don't test anything that calls this method yet.
     ok(false, "getPKCS12FilePassword() should not have been called");
   },
-  viewCert: (ctx, cert) => {
-    // This shouldn't be called for import methods.
-    ok(false, "viewCert() should not have been called");
-  },
 
   QueryInterface: ChromeUtils.generateQI([Ci.nsICertificateDialogs])
 };
 
 var gMockPrompter = {
   passwordToTry: "password",
   numPrompts: 0,