Removing a certificate that lacks a CommonName from a trust store shows an empty dialog

RESOLVED FIXED in Firefox 52

Status

()

Core
Security: PSM
P1
normal
RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: Tom Ritter, Assigned: Cykesiopka)

Tracking

(Blocks: 1 bug)

39 Branch
mozilla52
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox52 fixed)

Details

(Whiteboard: [psm-assigned])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Reporter)

Description

3 years ago
Created attachment 8636924 [details]
Sample certificate

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2357.134 Safari/537.36

Steps to reproduce:

1) Add a certificate without a CommonName to the Trust Store
2) Delete it



Actual results:

1) Observe that the dialog list is empty


Expected results:

Dialog List should have contained something, perhaps the full Subject value (C=<data>, OU=<data> etc etc).  If the Subject field is missing, perhaps a serial number, or just "Unnamed Cert".

Comment 1

3 years ago
Thanks, Tom. Moving to security UI.
Component: Untriaged → Security: UI
Product: Firefox → Core
Blocks: 1029832
(Assignee)

Updated

3 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Component: Security: UI → Security: PSM
Priority: -- → P3
Whiteboard: [psm-backlog]
(Assignee)

Comment 2

2 years ago
I want to write a test for deletecert.js, so I might as well fix this along the way.
Assignee: nobody → cykesiopka.bmo
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [psm-backlog] → [psm-assigned]
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Actually, if Mark has time to review this, that would be great.
Attachment #8798358 - Flags: review?(dkeeler) → review?(mgoodwin)
Attachment #8798359 - Flags: review?(dkeeler) → review?(mgoodwin)

Comment 6

2 years ago
mozreview-review
Comment on attachment 8798358 [details]
Bug 1186286 - Move some code around to facilitate code reuse for the subsequent patch.

https://reviewboard.mozilla.org/r/83880/#review82762
Attachment #8798358 - Flags: review?(mgoodwin) → review+

Comment 7

2 years ago
mozreview-review
Comment on attachment 8798359 [details]
Bug 1186286 - Consult more than just the CN when deleting a cert so it's always clear what's being deleted.

https://reviewboard.mozilla.org/r/83882/#review82758

::: security/manager/locales/en-US/chrome/pippki/pippki.properties:12
(Diff revision 1)
>  
> +# LOCALIZATION NOTE(certWithSerial): Used for semi-uniquely representing a cert.
> +# %1$S is the serial number of the cert in AA:BB:CC hex format.
> +certWithSerial=Certificate with serial number: %1$S
> +
>  #These are for dialogs

Not your fault, but this file can't make up its mind whether it wants spaces between # and the comment.

::: security/manager/ssl/tests/mochitest/browser/browser_deleteCert_ui.js:95
(Diff revision 1)
> +      cert = yield readCertificate(testCase.certFilename, ",,", gImportedCerts);
> +    }
> +    let certTreeItem = {
> +      hostPort: FAKE_HOST_PORT,
> +      cert: cert,
> +      QueryInterface(iid) {

I personally find `QueryInterface: function(iid)` more readable. Not a strong preference, though.
Attachment #8798359 - Flags: review?(mgoodwin) → review+
(Assignee)

Comment 8

2 years ago
mozreview-review-reply
Comment on attachment 8798359 [details]
Bug 1186286 - Consult more than just the CN when deleting a cert so it's always clear what's being deleted.

https://reviewboard.mozilla.org/r/83882/#review82758

Thanks!

> Not your fault, but this file can't make up its mind whether it wants spaces between # and the comment.

Fixed - the file now consistently puts a space between the # and the comment.

> I personally find `QueryInterface: function(iid)` more readable. Not a strong preference, though.

Noted. Let's keep this as is for now.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 12

2 years ago
Pushed by kcambridge@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c693c3241fa
Move some code around to facilitate code reuse for the subsequent patch. r=mgoodwin
https://hg.mozilla.org/integration/mozilla-inbound/rev/fcfc065914e5
Consult more than just the CN when deleting a cert so it's always clear what's being deleted. r=mgoodwin
Keywords: checkin-needed

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4c693c3241fa
https://hg.mozilla.org/mozilla-central/rev/fcfc065914e5
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox52: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
(Assignee)

Updated

a year ago
See Also: → bug 269022
You need to log in before you can comment on or make changes to this bug.