1282910 - test_security-info-certificate.js uses issuerOrganizationalUnit when the property being tested is issuerOrganizationUnit

Status: RESOLVED FIXED

(DevTools :: Console, defect, P3)

Description

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

In the beginning, the interface nsIX509Cert was written with a typo: what should have been the "issuerOrganizationalUnit" field was declared the "issuerOrganizationUnit". It would be nice to fix this, but doing so risks silently breaking existing code outside of the tree (e.g. add-ons). Maybe we still will, but in the meantime there's a preexisting issue in that test_security-info-certificate.js refers to the wrong field:

  equal(result.issuer.organizationalUnit, DUMMY_CERT.issuerOrganizationalUnit,
    "Organizational unit of the issuer is correct.");

("issuerOrganizationalUnit" should be "issuerOrganizationUnit")

Comment 1

[mgsudhanva]

hello , i would like to work on this bug .How do i begin ?

Comment 2

[Jarda Snajdr [:jsnajdr]]

(In reply to mgsudhanva from comment #1)
> hello , i would like to work on this bug .How do i begin ?

Thanks for your interest!

The first thing you need to do is to checkout the Firefox sources from Mercurial or Git and build it. Please follow the hacking guide here:

https://wiki.mozilla.org/DevTools/Hacking

Additionally, if you'd prefer working with the Git clone of the Firefox repository on Github instead of setting up Mercurial, then this MDN page has the info on Git setup:

https://developer.mozilla.org/en-US/docs/Tools/Contributing

Then you can start hacking on the actual bug. You want to have a look at these lines:

http://searchfox.org/mozilla-central/source/devtools/shared/webconsole/test/unit/test_security-info-certificate.js#46,54

and fix the property names.

Comment 3

[mgsudhanva]

Attachment: Proposed patch with the property names changed

I have changed the property names , anything more i need to do ?

Comment 4

[Jarda Snajdr [:jsnajdr]]

Comment on attachment 8781065 [details] [diff] [review]
Proposed patch with the property names changed

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

The changes themselves look good, but the patch is wrongly formatted and can't be applied: it shows difference between file "original.js" and "test_security-info-certificate.js", without any path information.

After the patch is fixed, we'll run a try build to see that all tests still pass.

Comment 5

[mgsudhanva]

Attachment: Changed the format of the patch

Created the patch using hg-mercurial

Comment 6

[Jarda Snajdr [:jsnajdr]]

(In reply to mgsudhanva from comment #5)
> Created the patch using hg-mercurial

Thank you, this is much better and can be applied cleanly. Before we can submit a try run and land the patch, please change the commit message. It should be in format:

Bug 1282910 - Fix issuerOrganizationUnit property name in test_security-info-certificate.js r=jsnajdr

That is, it needs to contain:
- the bug number (many automation tools rely on this)
- short description of what's been changed
- the reviewer's name

Also, when posting a new version of patch as an attachment to Bugzilla, mark the previous one as "obsolete" - there is a checkbox for that in the Bugzilla UI.

Comment 7

[mgsudhanva]

Attachment: test_security-info-certificate.js uses issuerOrganizationalUnit when the property being tested is issuerOrganizationUnit

Comment 8

[Jarda Snajdr [:jsnajdr]]

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

Comment 9

[Jarda Snajdr [:jsnajdr]]

Comment on attachment 8782321 [details] [diff] [review]
test_security-info-certificate.js uses issuerOrganizationalUnit when the property being tested is issuerOrganizationUnit

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

The patch is now perfect, including the commit message, and all tests in the try run have successfully passed. Ready to land. Thanks for fixing this bug!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/7db43c31bdb7
Fix issuerOrganizationUnit property name in test_security-info-certificate.js. r=jsnajdr

Comment 11

[Patrick Brosset <:pbro>]

[good first bug] whiteboard -> keyword mass change

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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

Comment 13

[kskamql2007]

We all go through times when desperate measures are our only resort and we must take them… finding a genuine hacker can be a lifeline and I sure used one to save my business, relationship and sanity. If you require the services of an E-C council certified expert you can reach Cyber wizard on - monitorywizard247 @ gmail com You will be glad you did. For the purpose of getting him comfortable with working with you it is advisable you point this post out, it saves you a lot of back and forth while ensuring you aren't just some child playing around. cheers...