Issue 5763 - Target languages supported by Firefox

packagerChrome.py

 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import errno
 import io
 import json
 import os
 import re
 from StringIO import StringIO
@@ skipping 278 matching lines @@
 
             files[targetFile] = toJson(data)
 
 
 def truncate(text, length_limit):
     if len(text) <= length_limit:
         return text
     return text[:length_limit - 1].rstrip() + u'\u2026'
 
 
-def fixTranslationsForCWS(files):
-    # Chrome Web Store requires messages used in manifest.json to be present in
-    # all languages. It also enforces length limits for extension names and
-    # descriptions.
+def fixTranslationsForChrome(files):

[tlucas, 2017/10/02 09:25:43]

nit: when changing a function's name the new name should adhere to PEP-8
(lowercase, underscores).

[Sebastian Noack, 2017/10/02 23:01:33]

Done.

     defaults = {}
     data = json.loads(files['_locales/%s/messages.json' % defaultLocale])
     for match in re.finditer(r'__MSG_(\S+)__', files['manifest.json']):
         name = match.group(1)
         defaults[name] = data[name]
 
     limits = {}
     manifest = json.loads(files['manifest.json'])
     for key, limit in (('name', 45), ('description', 132), ('short_name', 12)):
         match = re.search(r'__MSG_(\S+)__', manifest.get(key, ''))
         if match:
             limits[match.group(1)] = limit
 
-    for filename in files:
-        if not filename.startswith('_locales/') or not filename.endswith('/messages.json'):
-            continue
+    for path in list(files):
+        match = re.search(r'^_locales/(?:es_(AR|CL|(MX))|[^/]+)/(.*)', path)
+        if match:

[Vasily Kuznetsov, 2017/10/02 21:13:32]

This loop with a regexp search and then `if match` creates an annoying
additional level of iteration that doesn't really represent anything useful and
just gets in the way. You could probably get rid of it this way:

    for path in list(files):
        match = ...
        if not match:
            continue
        ...

although I admit that this inverted conditional is not quite satisfactory. What
I would like to have is something like:

    for path, (isLatAm, isMexican, filename) in re.magic_filter(REGEX, files):
        ...

but there doesn't seem to be any way to get to anything resembling this and
still readable and sensible. Well, I guess I leave it up to you to decide if you
like the inverted conditional more than the extraneous indent.

[Sebastian Noack, 2017/10/02 23:01:33]

The reason why I didn't bail out, but went for the extra level of indentation
was that it looked weird to either have `... = match.groups()` surrounded by
blank lines or have it squashed together with the block below. But otherwise, I
would have gone for using continue as well. Anyway, here you go.

+            isLatAm, isMexican, filename = match.groups()

[Vasily Kuznetsov, 2017/10/02 21:13:32]

While PEP8 is annoyingly non-specific with regards to the naming convention for
local variables, I've always assumed it's supposed to be lowercase with
underscores (to be consistent with functions[1] and global variables[2]). What
do you think the convention should be (and maybe we need to add it to our style
guide to augment this missing part of PEP8)?

[1] https://www.python.org/dev/peps/pep-0008/#function-names
[2] https://www.python.org/dev/peps/pep-0008/#global-variable-names

[Sebastian Noack, 2017/10/02 23:01:33]

Yeah, this should be lowercase with underscores. Done.

 
-        data = json.loads(files[filename])
-        for name, info in defaults.iteritems():
-            data.setdefault(name, info)
-        for name, limit in limits.iteritems():
-            if name in data:
-                data[name]['message'] = truncate(data[name]['message'], limit)
-        files[filename] = toJson(data)
+            # The Chrome Web Store requires messages used in manifest.json to be

[tlucas, 2017/10/02 09:25:43]

nit: this line is 1 character too long

[Sebastian Noack, 2017/10/02 23:01:33]

Done.

+            # present in all languages, and enforces length limits on extension
+            # name and description.
+            if filename == 'messages.json':
+                data = json.loads(files[path])
+                for name, info in defaults.iteritems():
+                    data.setdefault(name, info)
+                for name, limit in limits.iteritems():
+                    info = data.get(name)
+                    if info:
+                        info['message'] = truncate(info['message'], limit)
+                files[path] = toJson(data)
+
+            # Chrome combines Latin American dialects of Spanish into es-419.
+            if isLatAm:
+                data = files.pop(path)
+                if isMexican:
+                    files['_locales/es_419/' + filename] = data
 
 
 def signBinary(zipdata, keyFile):
     from Crypto.Hash import SHA
     from Crypto.PublicKey import RSA
     from Crypto.Signature import PKCS1_v1_5
 
     try:
         with open(keyFile, 'rb') as file:
             key = RSA.importKey(file.read())
@@ skipping 59 matching lines @@
         files.preprocess(
             [f for f, _ in metadata.items('preprocess')],
             {'needsExt': True}
         )
 
     if metadata.has_section('import_locales'):
         import_locales(params, files)
 
     files['manifest.json'] = createManifest(params, files)
     if type == 'chrome':
-        fixTranslationsForCWS(files)
+        fixTranslationsForChrome(files)
 
     if devenv:
         import buildtools
         import random
         files.read(os.path.join(buildtools.__path__[0], 'chromeDevenvPoller__.js'), relpath='devenvPoller__.js')
         files['devenvVersion__'] = str(random.random())
 
         if metadata.has_option('general', 'testScripts'):
             files['qunit/index.html'] = createScriptPage(
                 params, 'testIndex.html.tmpl', ('general', 'testScripts')
             )
 
     zipdata = files.zipToString()
     signature = None
     pubkey = None
     if keyFile != None:
         signature = signBinary(zipdata, keyFile)
         pubkey = getPublicKey(keyFile)
     writePackage(outFile, pubkey, signature, zipdata)