1130405 - Remove unused pippki strings

Status: RESOLVED FIXED

(Core Graveyard :: Security: UI, defect)

Description

[:Cykesiopka]

There are unused pippki strings in the .dtd and .properties files that can be removed.

Comment 1

[:Cykesiopka]

Attachment: bug1130405_rm-unused-pippki-strings_v1.patch

This patch removes the strings in question.

Included in this try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4c280971b9d7

Comment 2

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

Comment on attachment 8560847 [details] [diff] [review]
bug1130405_rm-unused-pippki-strings_v1.patch

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

JC, I'd appreciate it if you would review this patch. Basically, you just need to verify that these strings aren't being used any more (using dxr or mxr should be sufficient). Thanks!

Comment 3

[J.C. Jones [:jcj] (he/they)]

Comment on attachment 8560847 [details] [diff] [review]
bug1130405_rm-unused-pippki-strings_v1.patch

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

I took all the changed lines through DXR; found one hit... I believe, assuming it's case-insensitive. Please take a look. Thanks!

::: security/manager/locales/en-US/chrome/pippki/pippki.properties
@@ -117,5 @@
>  devinfo_stat_ready=Ready
>  enable_fips=Enable FIPS
>  disable_fips=Disable FIPS
>  fips_nonempty_password_required=FIPS mode requires that you have a Master Password set for each security device. Please set the password before trying to enable FIPS mode.
> -unable_to_toggle_fips=Unable to change the FIPS mode for the security device. It is recommended that you exit and restart this application.

This appears to be referenced from security/manager/pki/resources/content/device_manager.js:525

Comment 4

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

(In reply to J.C. Jones [:jcj] from comment #3)
> > -unable_to_toggle_fips=Unable to change the FIPS mode for the security device. It is recommended that you exit and restart this application.
> 
> This appears to be referenced from
> security/manager/pki/resources/content/device_manager.js:525

Good catch - I imagine these strings were intended to be the same:

https://hg.mozilla.org/mozilla-central/rev/c13b8d0a066c

Let's fix device_manager.js to use the same case.

Comment 5

[:Cykesiopka]

(In reply to J.C. Jones [:jcj] from comment #3)
> Comment on attachment 8560847 [details] [diff] [review]
> bug1130405_rm-unused-pippki-strings_v1.patch
> 
> Review of attachment 8560847 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I took all the changed lines through DXR; found one hit... I believe,
> assuming it's case-insensitive. Please take a look. Thanks!
> 
> ::: security/manager/locales/en-US/chrome/pippki/pippki.properties
> @@ -117,5 @@
> >  devinfo_stat_ready=Ready
> >  enable_fips=Enable FIPS
> >  disable_fips=Disable FIPS
> >  fips_nonempty_password_required=FIPS mode requires that you have a Master Password set for each security device. Please set the password before trying to enable FIPS mode.
> > -unable_to_toggle_fips=Unable to change the FIPS mode for the security device. It is recommended that you exit and restart this application.
> 
> This appears to be referenced from
> security/manager/pki/resources/content/device_manager.js:525

Oops, thanks for catching that! (I'll have to make sure case sensitivity is turned off next time...)

(In reply to David Keeler [:keeler] (use needinfo?) from comment #4)
> Good catch - I imagine these strings were intended to be the same:
> 
> https://hg.mozilla.org/mozilla-central/rev/c13b8d0a066c
> 
> Let's fix device_manager.js to use the same case.

OK.

Comment 6

[:Cykesiopka]

(In reply to David Keeler [:keeler] (use needinfo?) from comment #4)
> Good catch - I imagine these strings were intended to be the same:
> 
> https://hg.mozilla.org/mozilla-central/rev/c13b8d0a066c
> 
> Let's fix device_manager.js to use the same case.

Spun off into Bug 1131475: it turns out that case does matter here, and the code in question doesn't actually work.

Comment 7

[:Cykesiopka]

Attachment: bug1130405_rm-unused-pippki-strings_v2.patch

- Revert removal of "unable_to_toggle_fips" string

I also checked all the strings again with case sensitivity off. Hopefully I haven't made any more mistakes.

Comment 8

[J.C. Jones [:jcj] (he/they)]

Comment on attachment 8562763 [details] [diff] [review]
bug1130405_rm-unused-pippki-strings_v2.patch

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

Thanks; looks good to go. r=jcj

Comment 9

[:Cykesiopka]

Thanks for the review!

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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1c5df9f1a2a3

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1c5df9f1a2a3