Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VPN-6749: Fix shared addon ids #10065

Merged
merged 6 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 110 additions & 0 deletions .github/l10n/check_l10n_issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,121 @@
import xml.etree.ElementTree as etree
import json
import os
import subprocess
import sys
import tempfile

script_folder = os.path.abspath(os.path.dirname(__file__))
vpn_root_folder = os.path.realpath(os.path.join(script_folder, os.pardir, os.pardir))

def fileContentsAsJSON(filepath):
try:
with open(filepath, 'r') as file:
content = json.load(file)

if not content:
print(f"No content found in: {filepath}")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
print(f"No content found in: {filepath}")
sys.exit(f"No content found in: {filepath}")

You can exit directly with the message (across the file).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had it that way initially, but moved to print combined with sys.exit(1) to match the style in the rest of this file.

sys.exit(1)
return content
except FileNotFoundError:
print(f"File not found: {filepath}")
sys.exit(1)
except Exception as e:
print(f"An error occurred while loading {filepath}: {e}")
sys.exit(1)

# Check if it uses shared strings. If so, pull the value for “title”, “subtitle”, then look within "blocks"
def getSharedStringsInManifest(mainifest_contents):
shared_string_ids = []
# If it includes the key “message” and if within "message", there is a “usesSharedStrings” key that is true...
if manifest_contents.get("message", {}).get("usesSharedStrings", False):
#...collect all the string IDs
if "title" in manifest_contents["message"]:
shared_string_ids.append(manifest_contents["message"]["title"])
else:
print(f"No title found for: {addon}")
sys.exit(1)
if "subtitle" in manifest_contents["message"]:
# It is acceptable if there is no subtitle (but title is required)
shared_string_ids.append(manifest_contents["message"]["subtitle"])
if not "blocks" in manifest_contents["message"]:
print(f"No blocks found for: {addon}")
sys.exit(1)
for block in manifest_contents["message"]["blocks"]:
if not "content" in block:
print(f"No content found in {addon} for {block}")
sys.exit(1)
if isinstance(block["content"], str):
shared_string_ids.append(block["content"])
elif isinstance(block["content"], list):
# This is the contents of a bulleted or ordered list; go one layer deeper.
for list_item in block["content"]:
if isinstance(list_item["content"], str):
shared_string_ids.append(list_item["content"])
else:
print(f"No content found in {addon} for {list_item}")
sys.exit(1)
else:
print(f"Content found in {addon} was unknown type for {block}")
sys.exit(1)
return shared_string_ids

### 1. Check Addons' shared strings - all string IDs should be present in the translation file. ###
# Find list of all addon folders
addon_path = os.path.join(vpn_root_folder, "addons")
try:
addon_list = [item for item in os.listdir(addon_path) if os.path.isdir(os.path.join(addon_path, item))]
except FileNotFoundError:
print(f"Path not found: {addon_path}")
sys.exit(1)
except Exception as e:
print(f"An error occurred when finding all addons: {e}")
sys.exit(1)

if len(addon_list) == 0:
print(f"No addons found")
sys.exit(1)

shared_string_ids = []

# For each, open the manifest file and pull out the shared strings
for addon in addon_list:
manifest_path = os.path.join(addon_path, addon, "manifest.json")
manifest_contents = fileContentsAsJSON(manifest_path)
shared_string_ids = shared_string_ids + getSharedStringsInManifest(manifest_contents)

# Create temporary translation file
tmp_path = tempfile.mkdtemp()
shared_addon_strings = os.path.join(addon_path, "strings.yaml")
shared_addons_tmp_file = os.path.join(tmp_path, "strings.xliff")
generate_addon_python_file = os.path.join(vpn_root_folder, "scripts", "utils", "generate_shared_addon_xliff.py")
p = subprocess.run(['python', generate_addon_python_file, '-i', shared_addon_strings, '-o', shared_addons_tmp_file])

# Check that the shared strings are present in the translation file
if not os.path.isfile(shared_addons_tmp_file):
print(f"Unable to find {shared_addons_tmp_file}")
sys.exit(1)
try:
with open(shared_addons_tmp_file, 'r') as file:
shared_string_content = file.read()
if not shared_string_content:
print(f"No content found in: {shared_addons_tmp_file}")
sys.exit(1)
except FileNotFoundError:
print(f"File not found: {shared_addons_tmp_file}")
sys.exit(1)
except Exception as e:
print(f"An error occurred while loading {shared_addons_tmp_file}: {e}")
sys.exit(1)

for shared_string_id in shared_string_ids:
string_id_unit = "<trans-unit id=\"" + shared_string_id + "\">"
if string_id_unit not in shared_string_content:
print(f"{string_id_unit} not found in shared string file")
sys.exit(1)

### 2. Check all strings for overall issues. ###

# Paths are relative to the root folder
paths = [
"translations.ts",
Expand Down
17 changes: 12 additions & 5 deletions docs/Components/Addons/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,18 +152,25 @@ If you want to implement new add-ons, you need to follow these steps:

1. Create a manifest in a separate folder in the `addons` directory of the mozilla VPN repository.
2. Read and follow the documentation for the add-on type you want to implement.
3. Build the CMake project in the `addons` directory.
3. If new shared strings were added, process them so that strings are shown in the addon:
`python ./scripts/utils/generate_shared_addon_xliff.py -i ./addons/strings.yaml -o ./3rdparty/i18n/en/addons/strings.xliff`
(This step is not necessary, strictly speaking. This script is what will be run as new
strings are ingested into l10n repo, and once the new strings have done a round trip to
the l10n repo and then back to the client repo, the string will show up in the addon as
expected. Manually running this script as part of the build process prevents the brand
new addon from showing "vpn.newStringCategory.newString" until this round trip happens.)
4. Build the CMake project in the `addons` directory.

```
mkdir -p build-addons/
cmake -S <source>/addons -B build-addons -GNinja
cmake --build build-addons
```

4. Expose the generated build directory through a webservice. For example: `python3 -m http.server --directory build-addons/`
5. Open the dev-menu from the get-help view and set a custom add-on URL: `http://localhost:8000/`
6. Scroll down and disable the signature-addon feature from the dev-menu, list of features
7. Be sure you are doing all of this using a staging environment
5. Expose the generated build directory through a webservice. For example: `python3 -m http.server --directory build-addons/`
6. Open the dev-menu from the get-help view and set a custom add-on URL: `http://localhost:8000/`
7. Scroll down and disable the signature-addon feature from the dev-menu, list of features
8. Be sure you are doing all of this using a staging environment

If all has done correctly, you can see the app fetching the manifest.json (and
not! the manifest.json.sig) resource from the webservice.
2 changes: 1 addition & 1 deletion scripts/addon/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ def transform_shared_strings(input_file, output_file, relevant_strings, short_ve
print("Create localization file...")
os.mkdir(os.path.join(tmp_path, "i18n"))
template_ts_file = os.path.join(args.dest, f"{manifest['id']}.ts")
write_en_language(template_ts_file, strings)
write_en_language(template_ts_file, strings, True)

# This will be probably replaced by the en locale if it exists
en_ts_file = os.path.join(tmp_path, "i18n", "locale_en.ts")
Expand Down
7 changes: 5 additions & 2 deletions scripts/shared.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import sys
import xml.etree.ElementTree as ET

def write_en_language(filename, strings):
def write_en_language(filename, strings, key_as_id):
ts = ET.Element("TS")
ts.set("version", "2.1")
ts.set("language", "en")
Expand All @@ -12,7 +12,10 @@ def write_en_language(filename, strings):

for key, value in strings.items():
message = ET.SubElement(context, "message")
message.set("id", key)
if key_as_id:
message.set("id", key)
else:
message.set("id", value["string_id"])

location = ET.SubElement(message, "location")
location.set("filename", "addon.qml")
Expand Down
2 changes: 1 addition & 1 deletion scripts/utils/generate_shared_addon_xliff.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def prune_lists_to_strings(data):
print("Then, write the strings to a .ts file")
tmp_path = tempfile.mkdtemp()
ts_file = os.path.join(tmp_path, "sharedAddonsStrings.ts")
write_en_language(ts_file, translation_strings)
write_en_language(ts_file, translation_strings, False)

print("Finally, convert the ts file into an xliff file")
os.system(f"{lconvert} -if ts -i {ts_file} -of xlf -o {args.outfile}")
Loading