915993 - Change string ID for open_file_label to reflect new content (version 0.8.505)

Status: RESOLVED FIXED

(Firefox :: PDF Viewer, defect)

Description

[Francesco Lodolo [:flod]]

https://hg.mozilla.org/mozilla-central/rev/4ede1a360fc6

-open_file_label=Open
+open_file_label=Open File

This must not happen: new string -> new ID, unless there are technical reasons for not doing it.
https://developer.mozilla.org/en-US/docs/Making_String_Changes

We should try to fix this before it slips into Aurora.

Comment 1

[Brendan Dahl [:bdahl]]

Attachment: string-revert.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 914603
User impact if declined: 
Testing completed (on m-c, etc.): none, string only change
Risk to taking this patch (and alternatives if risky): low risk, reverting change
String or IDL/UUID changes made by this patch: String revert

Comment 2

[Francesco Lodolo [:flod]]

Not sure I'm the right person to review this patch.

Since the change has already gone to Aurora, I'd say we leave the string as it is there for en-US (no revert) and use a new ID on trunk. Change will be picked up a cycle later by l10n but at least we don't touch Aurora.

Pike, your opinion?

Comment 3

[Yury Delendik (:yury)]

Is there a need to introduce a new ID? We just reverting a change. Where we can find info about how translation backend and synchronization with aurora works?

Comment 4

[Francesco Lodolo [:flod]]

If you don't need that change anymore, just revert the change on both mozilla-aurora and l10n-central.
If you want that change, for sure you need a new ID on trunk (like you needed the first time it landed).

Not sure what you mean with "translation backend and synchronization with aurora".

Comment 5

[Yury Delendik (:yury)]

> If you don't need that change anymore, just revert the change on both mozilla-aurora and l10n-central.

We don't need that change. We are not touching l10n-central. How patch for l10n-central shall look like?

> Not sure what you mean with "translation backend and synchronization with aurora"

It's probably off-topic. But do you have any documentation about the software that does localization magic for m-c? I'm just trying to understand what are DOs and DONTs for pdf.js to avoid future mistakes and how to correct them.

Comment 6

[Francesco Lodolo [:flod]]

The idea is pretty simple: https://developer.mozilla.org/en-US/docs/Making_String_Changes

You don't change strings already landed on mozilla-central without using a new ID, unless it's a minor fix that involves only en-US. 
Changing the ID is the only sure way to have localizers look at the new string (the new string will be marked as untranslated) and not picking up outdated content.

> We don't need that change. We are not touching l10n-central. How patch for l10n-central shall look like?

The problem is that you already did, and you need to fix it
https://hg.mozilla.org/mozilla-central/rev/4ede1a360fc6

IMO there are two ways out (I'll wait for pike's answer, since he has much more experience than me in dealing with this):
a) we revert the string change on both mozilla-central and mozilla-aurora
b) we leave mozilla-aurora alone and change the ID on trunk

At this point I personally prefer b), I don't like touching stuff on Aurora.

Comment 7

[Yury Delendik (:yury)]

> a) we revert the string change on both mozilla-central and mozilla-aurora

Fixed upstream at https://github.com/mozilla/pdf.js/pull/3698. It will land on m-c with next pdf.js sync.

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Tracking (and waiting for review to approve uplift) so we ensure this gets reverted in Aurora.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8422f9e7e1c9

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Fixed on m-c by bug 922693.