Wrong comment about the function hasMatchingOverride() in nsICertOverrideService.idl

RESOLVED FIXED in Firefox 40

Status

()

RESOLVED FIXED
10 years ago
4 years ago

People

(Reporter: om.brahmana, Assigned: Cykesiopka)

Tracking

Trunk
mozilla40
x86
All
Points:
---

Firefox Tracking Flags

(firefox40 fixed)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 years ago
The function hasMatchingOverride() in nsICertOverrideService.idl has a wrong comment/description. http://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/public/nsICertOverrideService.idl#91

It is an exact replica of the comment for the previous function: http://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/public/nsICertOverrideService.idl#72
Depends on: 466011
There's a patch in bug 466011 that fixes this. It got lost in the mists of time waiting for review, but should still apply pretty cleanly, since the IDL doesn't change much.

Comment 2

6 years ago
reassign bug owner.
mass-update-kaie-20120918
Assignee: kaie → nobody
(Assignee)

Comment 3

4 years ago
Created attachment 8586562 [details] [diff] [review]
bug488480_correct-hasMatchingOverride()-doc_by-Johnathan-Nightingale_v1.patch

This patch contains the relevant bits from johnath's patch in Bug 466011, with very trivial changes.
Attachment #8586562 - Flags: review?(dkeeler)
Comment on attachment 8586562 [details] [diff] [review]
bug488480_correct-hasMatchingOverride()-doc_by-Johnathan-Nightingale_v1.patch

Review of attachment 8586562 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM with nits addressed.

::: security/manager/ssl/public/nsICertOverrideService.idl
@@ +63,3 @@
>     *
>     *  @param aHostName The host (punycode) this mapping belongs to
>     *  @param aPort The port this mapping belongs to, if it is -1 then it 

nit: let's remove trailing whitespace while we're here

@@ +63,5 @@
>     *
>     *  @param aHostName The host (punycode) this mapping belongs to
>     *  @param aPort The port this mapping belongs to, if it is -1 then it 
>     *          is internaly treated as 443
>     *  @param aCert The cert that should always be accepted

I would update this description too. Maybe "The certificate this mapping belongs to".

@@ +72,2 @@
>     */
>    boolean hasMatchingOverride(in ACString aHostName, 

nit: trailing whitespace
Attachment #8586562 - Flags: review?(dkeeler) → review+
(Assignee)

Comment 5

4 years ago
Created attachment 8587370 [details] [diff] [review]
bug488480_correct-hasMatchingOverride()-doc_v2.patch

+ Address feedback from Comment 4
+ Fix typos
Assignee: nobody → cykesiopka.bmo
Attachment #8586562 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8587370 - Flags: review+
(Assignee)

Comment 6

4 years ago
Thanks for the review.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=79e43b4ab4c4
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/755da69810a1
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
(Assignee)

Updated

4 years ago
No longer depends on: 466011
You need to log in before you can comment on or make changes to this bug.