Closed Bug 1308888 Opened 3 years ago Closed 3 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: Cykesiopka, Assigned: Cykesiopka)

References

()

Details

(Whiteboard: [psm-assigned])

Attachments

(1 file)

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 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?
Attachment #8802529 - Flags: review?(dkeeler) → review+
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.
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7d608304562a
Simplify passing handle to the cert to view in the cert viewer. r=keeler
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7d608304562a
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.