583530 - Typo in the restartToEnable entity in extensions.properties breaks the details pane

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

There's a typo in the first formatting token in the restartToEnable entity in extension.properties - which, when used, breaks the details pane. 

Specifically, %1$% should be %1$S

Comment 1

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch

Comment 2

[Robert Strong (they/them - no direct email)]

Comment on attachment 461842 [details] [diff] [review]
Patch

># HG changeset patch
># User Blair McBride <[e-mail]>
># Date 1280623364 -43200
># Node ID 05004702a96b70996a79647fa8dee8b7e33bc5c1
># Parent  823e3f0ac5470efae9616237ecf72c4c1abea120
>Bug 583530 - Typo in the restartToEnable entity in extensions.properties breaks the details pane
>
>diff --git a/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties b/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties
>--- a/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties
>+++ b/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties
>@@ -45,7 +45,7 @@ installFailed=Error installing
> installCancelled=Install cancelled
> 
> #LOCALIZATION NOTE: %1$S is the addon name, %2$S is brand name
>-restartToEnable=%1$% will be enabled after you restart %2$S
>+restartToEnable=%1$S will be enabled after you restart %2$S
For l10n to pick up this change you need to change restartToEnable to a new name. :(

Comment 3

[Søren Munk Skrøder (extended hiatus)]

Remember that this will probably have l10n effect... 

There could be localization teams that have already translated the string, but have not noticed the faulty token...

They will not catch on to this change unless
1) you change the key of the string (restartToEnable -> restartToEnable2) 
  or
2) you announce this change in mozilla.dev.l10n

Comment 4

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1.1

Grr, stupid typos.

Comment 5

[Søren Munk Skrøder (extended hiatus)]

even if you change restartToEnable to restartToEnable2, an announcement of the change to moz.dev.l10n is good practise - especially when it's a technical change and not a re-wording...

Comment 6

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #3)
> 1) you change the key of the string (restartToEnable -> restartToEnable2) 

I did this.

Surprised no one (including myself) picked up on this until now. Thanks go to
Søren for finding it.

(In reply to comment #5)
> even if you change restartToEnable to restartToEnable2, an announcement of the
> change to moz.dev.l10n is good practise - especially when it's a technical
> change and not a re-wording...

Ok, will do that after this lands.

Comment 7

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Given this breaks the details pane in a certain situation, would like to see this blocking (I know how late this is!). Note that it is kinda edge-casey though:

* Disable an enabled extension (not a restartless extension)
* Go to details pane of that extension (before restarting)

Comment 8

[Søren Munk Skrøder (extended hiatus)]

(In reply to comment #6)
> (In reply to comment #3)
> > 1) you change the key of the string (restartToEnable -> restartToEnable2) 
> 
> I did this.
> 
> Surprised no one (including myself) picked up on this until now. Thanks go to
> Søren for finding it.

in fairness; kudos should go to Jesper Kristensen/dikrib, fellow member of the
Danish Localization team, who alerted me of this earlier this evening...

credit, where credit is due...  ;-D

Comment 9

[Dave Townsend [:mossop]]

Landed: http://hg.mozilla.org/mozilla-central/rev/b91faba6e48f

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed by landing.