1308888 - Simplify passing handle to the cert to view in the cert viewer

Status: RESOLVED FIXED

(Core :: Security: PSM, defect, P1)

Description

[:Cykesiopka]

The cert viewer currently supports two ways to pass a handle to the cert:
1. Passing the nickname of the cert via window.name.
2. Via an nsIDialogParamBlock, which is itself accessed through window.arguments.

Method 1 is unused and unnecessary.
Method 2 is overly complex: the relevant nsIX509Cert can just be passed directly.

We should change this so that there is only a single, straightforward way to pass a handle to the cert.

Comment 1

[:Cykesiopka]

Attachment: Bug 1308888 - Simplify passing handle to the cert to view in the cert viewer.

The cert viewer currently supports two ways to pass a handle to the cert:
1. Passing the nickname of the cert via window.name.
2. Via an nsIDialogParamBlock, which is itself accessed through window.arguments.

Method 1 is unused and unnecessary.
Method 2 is overly complex: the relevant nsIX509Cert can just be passed directly.

This patch does the following:
1. Makes it so that there is only a single, straightforward way to pass a handle
to the cert.
2. Makes the cert viewer title localisable while we're nearby.
3. Renames viewCertDetails.js to better reflect the current use of the file.

Review commit: https://reviewboard.mozilla.org/r/86910/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/86910/

Comment 2

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8802529 [details]
Bug 1308888 - Simplify passing handle to the cert to view in the cert viewer.

https://reviewboard.mozilla.org/r/86910/#review86094

Cool - r=me. Just some vague suggestions.

::: security/manager/pki/nsNSSDialogHelper.h:14
(Diff revision 1)
>  
>  class mozIDOMWindowProxy;
>  class nsISupports;
>  
>  /**
> - * Common class that uses the window watcher service to open a
> + * Common class that uses the window watcher service to open a standard dialog,

Maybe "Helper class"?

::: security/manager/pki/nsNSSDialogs.cpp:332
(Diff revision 1)
>    }
>  
>    return NS_OK;
>  }
>  
>  NS_IMETHODIMP 

Maybe take out this trailing whitespace at the same time? (not a big deal either way)

::: security/manager/pki/nsNSSDialogs.cpp
(Diff revision 1)
> -  }
> -  nsresult rv = dlgArray->AppendElement(cert, false);
> -  if (NS_FAILED(rv)) {
> -    return rv;
> -  }
> -  nsCOMPtr<nsIDialogParamBlock> dlgParamBlock(

I didn't really look, but maybe we can cull some #includes as a result of removing this?

Comment 3

[:Cykesiopka]

Comment on attachment 8802529 [details]
Bug 1308888 - Simplify passing handle to the cert to view in the cert viewer.

https://reviewboard.mozilla.org/r/86910/#review86094

Thanks!

> Maybe "Helper class"?

Sounds good.

> Maybe take out this trailing whitespace at the same time? (not a big deal either way)

Done.

> I didn't really look, but maybe we can cull some #includes as a result of removing this?

No, but good suggestion anyways - turns out there are #includes that we don't need that weren't removed in previous changes.

Comment 4

[:Cykesiopka]

Comment on attachment 8802529 [details]
Bug 1308888 - Simplify passing handle to the cert to view in the cert viewer.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/86910/diff/1-2/

Comment 5

[:Cykesiopka]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b19235e03710282a919d80ac85d5731eb147f1d2

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7d608304562a
Simplify passing handle to the cert to view in the cert viewer. r=keeler

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7d608304562a