Issue 1321 - Truncate extension name and description at build time instead of only truncating descr…

packagerChrome.py

 # coding: utf-8
 
 # This file is part of the Adblock Plus build tools,
 # Copyright (C) 2006-2014 Eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
@@ skipping 270 matching lines @@
       'pt': 'pt_PT',
     }
     for chromeLocale, operaLocale in operaMapping.iteritems():
       chromeFile = '_locales/%s/messages.json' % chromeLocale
       operaFile = '_locales/%s/messages.json' % operaLocale if operaLocale != None else None
       if chromeFile in files:
         if operaFile != None:
           files[operaFile] = files[chromeFile]
         del files[chromeFile]
 
-def fixMissingTranslations(files):
-  # Chrome requires messages used in manifest.json to be given in all languages
+def truncate(text, length_limit):
+  if len(text) <= length_limit:
+    return text
+  return text[:length_limit - 1].rstrip() + u"\u2026"

[Sebastian Noack, 2014/09/03 18:53:37]

I think we should at least generate a warning when truncating the text.

[Wladimir Palant, 2014/09/03 19:05:22]

I considered doing that. However, the main effect of this warning would be
spamming Felix and me (we are the ones receiving cron mail from development
build generation). I'd really prefer to have this covered by
https://issues.adblockplus.org/ticket/104 instead.

[Sebastian Noack, 2014/09/03 19:13:03]

Perfect. Even better would be to have proper monitoring, where everybody can see
if/what went wrong. But for now that should be fine.
It shouldn't take until the next release to realize that a translation were
truncated. Also this issue won't probably be done in the near future.

[Wladimir Palant, 2014/09/03 21:21:31]

Not at all. It should spam you because you are the one who should do something
about it.
A reminder: translations are being updated manually. Whoever updated them should
also check them. And not rely on Felix or me to be annoyed enough to remind him
of it.

[Sebastian Noack, 2014/09/04 10:40:02]

So I feel this is the wrong place to address that issue. Why don't we just make
"build.py gettranslations" ensure the length of those string?

[Wladimir Palant, 2014/09/04 13:27:03]

That's what you did originally. But that means that we have a truncated string
in our repository and need to go to Crowdin to see the real one (e.g. to shorten
it). Also, we will have to truncate it for all browsers, not just Chrome.

gettranslations really shouldn't change the data...

[Sebastian Noack, 2014/09/04 13:29:55]

I agree, but that isn't what I meant to suggest. gettranslations should fail
with an error (or at least show a warning) if the string is too long. So whoever
imports the translation (prior to the release in the future) must take care of
it.

[Wladimir Palant, 2014/09/04 13:47:43]

Take care how exactly? Do you speak Telugu? I don't. And if there is an issue in
the Telugu translation (one that cannot be addressed by removing whitespace),
then the usual approach is to inform the translator. If we are lucky, the
translator will respond on the same day. It's quite likely however that this
issue won't be fixed in time for the release (maybe not even in time for the
next one). I'd prefer to be able to release nevertheless, merely with a
truncated string.

As I said, warnings are something that should be addressed in #104 - that
validation can be integrated into gettranslations then (or maybe not, the
validation on Firefox produces a massive amount of warnings). It shouldn't be
hardcoded that "name_devbuild" can only be 45 characters long, this is something
to be configured in metadata.common. Note also that validating "name" requires
actually importing it from Firefox.

[Sebastian Noack, 2014/09/04 14:00:58]

In the worst case one can still decide to truncate the string manually. But it
dangerous, to silently truncate strings.

As I said yesterday, I think we should update the translations at least one week
before the release, that there is enough time to resolve issues like that an
similar ones.

+
+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.
   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
 
     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)
 
 def signBinary(zipdata, keyFile):
   import M2Crypto
   if not os.path.exists(keyFile):
     M2Crypto.RSA.gen_key(1024, 65537, callback=lambda x: None).save_key(keyFile, cipher=None)
   key = M2Crypto.EVP.load_key(keyFile)
   key.sign_init()
   key.sign_update(zipdata)
   return key.final()
@@ skipping 47 matching lines @@
   if metadata.has_section('preprocess'):
     files.preprocess(
       [f for f, _ in metadata.items('preprocess')],
       {'needsExt': True}
     )
 
   if metadata.has_section('import_locales'):
     importGeckoLocales(params, files)
 
   files['manifest.json'] = createManifest(params, files)
-  fixMissingTranslations(files)
+  if type == 'chrome':
+    fixTranslationsForCWS(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', 'backgroundScripts') and
       'lib/info.js' in re.split(r'\s+', metadata.get('general', 'backgroundScripts')) and
       'lib/info.js' not in files):
     files['lib/info.js'] = createInfoModule(params)
 
   zipdata = files.zipToString()
   signature = None
   pubkey = None
   if keyFile != None:
     signature = signBinary(zipdata, keyFile)
     pubkey = getPublicKey(keyFile)
   writePackage(outFile, pubkey, signature, zipdata)
 
 def createDevEnv(baseDir, type):
   fileBuffer = StringIO()
   createBuild(baseDir, type=type, outFile=fileBuffer, devenv=True, releaseBuild=True)
 
   from zipfile import ZipFile
   zip = ZipFile(StringIO(fileBuffer.getvalue()), 'r')
   zip.extractall(os.path.join(baseDir, 'devenv'))
   zip.close()