mozilla-vpn-client icon indicating copy to clipboard operation
mozilla-vpn-client copied to clipboard

VPN-5478: Use shared strings for addons

Open mcleinman opened this issue 6 months ago • 4 comments

Description

This adds a new translation file - ./addons/strings.yaml - where all future message addon strings will live. Bringing all (future) strings to one file allows us to reuse strings across messages, to reduce work on our translation team. My expectation is that the translation file will have this shared section (which may be added to), as well as sections specific to a single addon message (when appropriate). Additionally, there is one argument that can be substituted in - the version number. (I did this instead of something that could support a range of arguments to make the build.py changes simpler. I'm having trouble imaging anything other than a version number we'd want to drop in, and we can make this more generic in the future if it seems like that would be helpful.

All future message addons should include the usesSharedStrings boolean (set to true), as well as the shortVersion (used to insert into common strings). Documentation has been updated.

After this is merged, we'll need to merge https://github.com/mozilla-l10n/mozilla-vpn-client-l10n/pull/466 so that translations are pulled into the i18n repo.

The bulk of the changes are in build.py. (In order to support older clients, all the changes need to be in the addons build system - as a solution that required the clients to do part of the workflow would not work on older clients.) This changes nothing for older clients, but when usesSharedStrings is present and true, it changes the flow:

  1. The retrieve_strings_message function is changed to do some checks and then return the results of retrieve_legacy_strings_message or retrieve_shared_strings_message, depending if usesSharedStrings is present and true.
    1. retrieve_legacy_strings_message has the logic that was moved out of retreive_strings_message.
    2. retrieve_shared_strings_message returns strings in a similar fashion to the legacy function, but pulls the value and comments from the appropriate places (and look at the strings.yaml file to help construct this). It also includes a legacy_id to swap in during the final step.
    3. retrieve_strings_blocks is updated to do similar things as retrieve_shared_strings_message for messages that use shared strings.
  2. If useSharedStrings is present and true, the script confirms shortString is present and a string (and errors out if it is not present).
  3. If useSharedStrings is present and true, it changes all the input xliff files before calling lconvert. Typically, it finds the shared strings.xliff file for the given language (this is the file that will exist in the i18n submodule), and calls transform_shared_strings on it. It creates an output xliff in the temp directory, and this output xliff file is what is used by the convert.
    1. transform_shared_strings does 3 things before writing the temp xliff file: It filters the shared strings.xliff file (the input) to only include the strings relevant for this specific message addon. It substitutes the string ids for the legacy string IDs (saved much earlier in the process), so that the addon displays as expected on all clients. And finally, if it encounters %1 in any string, it substitutes in the shortVersion.

generate_shared_addon_xliff.py is used by the translation repo to pull these strings upstream. (Will be activated with a PR on that repo: https://github.com/mozilla-l10n/mozilla-vpn-client-l10n/pull/466). It should be well commented enough to easily understand what it does; if not please let me know and I'll improve the comments.

Additionally, I've pulled some shared python code into ./utils/shared.py and removed some duplicated code around the repo. (We couldn't just import write_en_language from build.py because of the required arguments for that file.)

Testing

I tested this with the 2.24 update message addon (message_update_v2.24). To do this:

  • version.txt: change the current version to 2.23.0 (so that it shows the update message)
  • addons/strings.yaml: Uncomment out the tester string (as well as the 224updateMessage if you want). This is done to show us ignoring unneeded strings when creating the addon translation files, and still reporting accurate translation percentages.
  • addons/message_update_v2.24/manifest.json: In the message section, add the usesSharedString and shortVersion keys, and update the title, subtitle, and all content values to shared strings. That is, change the message section to:
     "date": 1722476590,
     "id": "message_update_v2.24",
     "usesSharedStrings": true,
     "shortVersion": "2.24",
     "title": "vpn.commonStrings.updateTitle",
     "subtitle": "vpn.commonStrings.subtitle",
     "badge": "new_update",
     "blocks": [
       {
         "id": "c_1",
         "type": "text",
         "content": "vpn.commonStrings.generalUpdateContent"
       },
       {
         "id": "c_3",
         "type": "button",
         "style": "primary",
         "content": "vpn.commonStrings.updateButton",
         "javascript": "update.js"
       },
       {
         "id": "c_4",
         "type": "button",
         "style": "primary",
         "content": "vpn.commonStrings.downloadButton",
         "javascript": "updateWeb.js"
       },
       {
         "id": "c_5",
         "type": "button",
         "style": "link",
         "content": "vpn.commonStrings.getHelpButton",
         "javascript": "getHelp.js"
       },
       {
         "id": "extra_1",
         "type": "text",
         "content": "vpn.commonStrings.downloadTitle"
       }
     ]
   }

  • After this is merged, we’ll get updated files from the translation repo. For now, I manually created two files in the i18n submodule to show how this works. Create a file ./3rdparty/i18n/en/addons/strings.xliff, and insert this text:
<?xml version='1.0' encoding='UTF-8'?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" version="1.2">
  <file original="addon.qml" datatype="plaintext" source-language="en" target-language="en-US">
    <body>
      <trans-unit id="vpn.commonStrings.updateTitle">
        <note annotates="source" from="developer">Standard text in a composer block</note>
        <source>Update title %1</source>
      </trans-unit>
      <trans-unit id="vpn.commonStrings.downloadTitle">
        <note annotates="source" from="developer">Standard text in a composer block</note>
        <source>Download title</source>
      </trans-unit>
      <trans-unit id="vpn.commonStrings.subtitle">
        <note annotates="source" from="developer">Subtitle for a message view</note>
        <source>Subtitle</source>
      </trans-unit>
      <trans-unit id="vpn.commonStrings.generalUpdateContent">
        <note annotates="source" from="developer">Title for a message view</note>
        <source>The typical content text</source>
      </trans-unit>
      <trans-unit id="vpn.commonStrings.updateButton">
        <note annotates="source" from="developer">Title for a message view</note>
        <source>Update button</source>
      </trans-unit>
      <trans-unit id="vpn.commonStrings.downloadButton">
        <note annotates="source" from="developer">Title for a message view</note>
        <source>Download button</source>
      </trans-unit>
      <trans-unit id="vpn.commonStrings.getHelpButton">
        <note annotates="source" from="developer">Title for a message view</note>
        <source>Get help button</source>
      </trans-unit>
      <trans-unit id="vpn.commonStrings.tester">
        <note annotates="source" from="developer">Title for a message view</note>
        <source>TESTER: This should never show up</source>
      </trans-unit>
    </body>
  </file>
</xliff>
  • Additionally, create ./3rdparty/i18n/fr/addons/strings.xliff:
<?xml version='1.0' encoding='UTF-8'?>
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" version="1.2">
  <file original="addon.qml" datatype="plaintext" source-language="en" target-language="fr">
    <body>
      <trans-unit id="vpn.commonStrings.updateTitle">
        <note annotates="source" from="developer">Standard text in a composer block</note>
        <target>French: Update title %1</target>
        <source>Update title %1</source>
      </trans-unit>
      <trans-unit id="vpn.commonStrings.downloadTitle">
        <note annotates="source" from="developer">Standard text in a composer block</note>
        <target>French: Download title</target>
        <source>Download title</source>
      </trans-unit>
      <trans-unit id="vpn.commonStrings.subtitle">
        <note annotates="source" from="developer">Subtitle for a message view</note>
        <target>French: Subtitle</target>
        <source>Subtitle</source>
      </trans-unit>
      <trans-unit id="vpn.commonStrings.generalUpdateContent">
        <note annotates="source" from="developer">Title for a message view</note>
        <target>French: The typical content text</target>
        <source>The typical content text</source>
      </trans-unit>
      <trans-unit id="vpn.commonStrings.updateButton">
        <note annotates="source" from="developer">Title for a message view</note>
        <target>French: Update button</target>
        <source>Update button</source>
      </trans-unit>
      <trans-unit id="vpn.commonStrings.downloadButton">
        <note annotates="source" from="developer">Title for a message view</note>
        <target>French: Download button</target>
        <source>Download button</source>
      </trans-unit>
      <trans-unit id="vpn.commonStrings.getHelpButton">
        <note annotates="source" from="developer">Title for a message view</note>
        <target>French: Get help button</target>
        <source>Get help button</source>
      </trans-unit>
      <trans-unit id="vpn.commonStrings.tester">
        <note annotates="source" from="developer">Title for a message view</note>
        <source>TESTER: This should never show up</source>
      </trans-unit>
    </body>
  </file>
</xliff>

Then follow the instructions in the How to implement and test add-ons section of ./docs/Components/Addons/index.md, and compile a new version of the app to view the addons. (We need a new version of the app so that the version number is reported as 2.23, showing the update addon.)

Non-comprehensive list of things to do In testing:

  • Looked at the % translated (still shows as 100% for French, even though it doesn’t include a translation for commonStrings.tester)
  • French should show its translation
  • All Englishes should show English (demonstrating that language fallback works)
  • No other languages should show this addon, as it doesn’t have translations.
  • If shortVersion is missing, the build script will error out.
  • If the addon is changed to a string ID that is not in strings.yaml 9say, change commonStrings.title to commonStrings.fakeTitle, the build script will error out.

Reference

VPN-5478

Checklist

  • [x] My code follows the style guidelines for this project
  • [x] I have not added any packages that contain high risk or unknown licenses (GPL, LGPL, MPL, etc. consult with DevOps if in question)
  • [x] I have performed a self review of my own code
  • [x] I have commented my code PARTICULARLY in hard to understand areas
  • [x] I have added thorough tests where needed

mcleinman avatar Aug 02 '24 18:08 mcleinman