feat: web-ext build now uses the default locale (if applicable) when generating a file name (#487)

shubheksha · kumar303 · commit aeff9ba3a9e6 · 2016-10-07T17:51:44.000-05:00
* fix: Building an XPI with only localizations makes an ugly file name

* fix: Building an XPI with only localizations makes an ugly file name

* fix: Building an XPI with only localizations makes an ugly file name -- works with extensions which aren't localized

* fix: Building an XPI with only localizations makes an ugly file name -- resolved merge conflicts

* test: Building an XPI with only localizations makes an ugly file name

* test: removed unnecessary locales

* refactor: isolated the method to retrive the name of the extension

* fix: Building an XPI with only localizations makes an ugly file name

* test: Building an XPI with only localizations makes an ugly file name--added test for malformed JSON

* test: Building an XPI with only localizations makes an ugly file name--added test for malformed JSON

* refactor: Building an XPI with only localizations makes an ugly file name

* refactor: getDefaultLocalizedName() accepts a file instead of a JSON string -- modified the test checking for malformed JSON accordingly

* test: name of extension which is not localized

* test: check if locale file does not exist

* test: check locale file for incorrect format

* fix: style nits and removed unnecessary test

* test: fixed error messages for malformed json and incorrect locale file format tests

* test: check gives correct name to a localized extension with repeating pattern in manifest

* test: made error messages clearer, fixed style nits

* test: cleaned up code, fixed style nits, made error message clearer
diff --git a/src/cmd/build.js b/src/cmd/build.js
@@ -3,12 +3,14 @@ import path from 'path';
 import minimatch from 'minimatch';
 import {createWriteStream} from 'fs';
 import streamToPromise from 'stream-to-promise';
+import {fs} from 'mz';
 
 import defaultSourceWatcher from '../watcher';
 import {zipDir} from '../util/zip-dir';
 import getValidatedManifest, {getManifestId} from '../util/manifest';
 import {prepareArtifactsDir} from '../util/artifacts';
 import {createLogger} from '../util/logger';
+import {UsageError} from '../errors';
 
 
 const log = createLogger(__filename);
@@ -37,14 +39,48 @@ export type PackageCreatorParams = {
   artifactsDir: string,
 };
 
+export type LocalizedNameParams = {
+  messageFile: string,
+  manifestData: ExtensionManifest,
+}
+
+export async function getDefaultLocalizedName(
+  {messageFile, manifestData}: LocalizedNameParams
+): Promise<string> {
+
+  let messageData: string|Buffer;
+  let extensionName: string = manifestData.name;
+
+  try {
+    messageData = JSON.parse(await fs.readFile(messageFile));
+  }
+  catch (error) {
+    throw new UsageError(
+      `Error reading or parsing file ${messageFile}: ${error}`);
+  }
+  extensionName = manifestData.name.replace(/__MSG_([A-Za-z0-9@_]+?)__/g,
+    (match, messageName) => {
+      if (!(messageData[messageName]
+            && messageData[messageName].message)) {
+        const error = new UsageError(
+          `The locale file ${messageFile} ` +
+            `is missing key: ${messageName}`);
+        throw error;
+      }
+      else {
+        return messageData[messageName].message;
+      }
+    });
+  return Promise.resolve(extensionName);
+}
+
 export type PackageCreatorFn =
     (params: PackageCreatorParams) => Promise<ExtensionBuildResult>;
 
 async function defaultPackageCreator(
   {manifestData, sourceDir, fileFilter, artifactsDir}: PackageCreatorParams
 ): Promise<ExtensionBuildResult> {
   let id;
-
   if (manifestData) {
     id = getManifestId(manifestData);
     log.debug(`Using manifest id=${id || '[not specified]'}`);
@@ -56,8 +92,17 @@ async function defaultPackageCreator(
     filter: (...args) => fileFilter.wantFile(...args),
   });
 
+  let extensionName: string = manifestData.name;
+
+  if (manifestData.default_locale) {
+    let messageFile = path.join(sourceDir, '_locales',
+      manifestData.default_locale, 'messages.json');
+    log.debug('Manifest declared default_locale, localizing extension name');
+    extensionName = await getDefaultLocalizedName(
+      {messageFile, manifestData});
+  }
   let packageName = safeFileName(
-    `${manifestData.name}-${manifestData.version}.zip`);
+    `${extensionName}-${manifestData.version}.zip`);
   let extensionPath = path.join(artifactsDir, packageName);
   let stream = createWriteStream(extensionPath);
 
diff --git a/src/util/manifest.js b/src/util/manifest.js
@@ -14,6 +14,7 @@ const log = createLogger(__filename);
 export type ExtensionManifest = {
   name: string,
   version: string,
+  default_locale?: string,
 };
 
 export default async function getValidatedManifest(
diff --git a/tests/fixtures/minimal-localizable-web-ext/_locales/en/messages.json b/tests/fixtures/minimal-localizable-web-ext/_locales/en/messages.json
@@ -0,0 +1,6 @@
+{
+  "extensionName": {
+    "message": "Name of the extension",
+    "description": "This is an example of a minimal extension that does nothing"
+  }
+}
diff --git a/tests/fixtures/minimal-localizable-web-ext/background-script.js b/tests/fixtures/minimal-localizable-web-ext/background-script.js
@@ -0,0 +1,3 @@
+/* eslint no-console:0 */
+
+console.log('background script loaded');
diff --git a/tests/fixtures/minimal-localizable-web-ext/manifest.json b/tests/fixtures/minimal-localizable-web-ext/manifest.json
@@ -0,0 +1,7 @@
+{
+  "manifest_version": 2,
+  "name": "__MSG_extensionName__",
+  "description": "__MSG_extensionDescription__",
+  "version": "1.0",
+  "default_locale": "en"
+}
diff --git a/tests/unit/test-cmd/test.build.js b/tests/unit/test-cmd/test.build.js
@@ -5,11 +5,21 @@ import {it, describe} from 'mocha';
 import {assert} from 'chai';
 import sinon from 'sinon';
 
-import build, {safeFileName, FileFilter} from '../../../src/cmd/build';
+import build, {
+  safeFileName,
+  FileFilter,
+  getDefaultLocalizedName,
+} from '../../../src/cmd/build';
 import {withTempDir} from '../../../src/util/temp-dir';
 import {fixturePath, makeSureItFails, ZipFile} from '../helpers';
-import {basicManifest, manifestWithoutApps} from '../test-util/test.manifest';
+import {
+  basicManifest,
+  manifestWithoutApps,
+} from '../test-util/test.manifest';
+import {UsageError} from '../../../src/errors';
+import {createLogger} from '../../../src/util/logger';
 
+const log = createLogger(__filename);
 
 describe('build', () => {
 
@@ -37,6 +47,113 @@ describe('build', () => {
     );
   });
 
+  it('gives the correct name to a localized extension', () => {
+    return withTempDir(
+      (tmpDir) =>
+        build({
+          sourceDir: fixturePath('minimal-localizable-web-ext'),
+          artifactsDir: tmpDir.path(),
+        })
+        .then((buildResult) => {
+          assert.match(buildResult.extensionPath,
+                       /name_of_the_extension-1\.0\.zip$/);
+          return buildResult.extensionPath;
+        })
+    );
+  });
+
+  it('handles repeating localization keys', () => {
+    return withTempDir(
+      (tmpDir) => {
+        const messageFileName = path.join(tmpDir.path(), 'messages.json');
+        fs.writeFileSync(messageFileName,
+          `{"extensionName": {
+              "message": "example extension",
+              "description": "example description"
+            }
+          }`);
+
+        const manifestWithRepeatingPattern = {
+          name: '__MSG_extensionName__ __MSG_extensionName__',
+          version: '0.0.1',
+        };
+
+        return getDefaultLocalizedName({
+          messageFile: messageFileName,
+          manifestData: manifestWithRepeatingPattern,
+        })
+          .then((result) => {
+            assert.match(result, /example extension example extension/);
+          });
+      }
+    );
+  });
+
+  it('checks locale file for malformed json', () => {
+    return withTempDir(
+      (tmpDir) => {
+        const messageFileName = path.join(tmpDir.path(), 'messages.json');
+        fs.writeFileSync(messageFileName,
+          '{"simulated:" "json syntax error"');
+        return getDefaultLocalizedName({
+          messageFile: messageFileName,
+          manifestData: manifestWithoutApps,
+        })
+          .then(makeSureItFails())
+          .catch((error) => {
+            assert.instanceOf(error, UsageError);
+            assert.match(
+              error.message,
+              /Error .* file .*messages\.json: SyntaxError: Unexpected string/);
+          });
+      }
+    );
+  });
+
+  it('checks locale file for incorrect format', () => {
+    return withTempDir(
+      (tmpDir) => {
+        const messageFileName = path.join(tmpDir.path(), 'messages.json');
+        //This is missing the 'message' key
+        fs.writeFileSync(messageFileName,
+          `{"extensionName": {
+              "description": "example extension"
+              }
+          }`);
+        const basicLocalizedManifest = {
+          name: '__MSG_extensionName__',
+          version: '0.0.1',
+        };
+        return getDefaultLocalizedName({
+          messageFile: messageFileName,
+          manifestData: basicLocalizedManifest,
+        })
+          .then(makeSureItFails())
+          .catch((error) => {
+            assert.instanceOf(error, UsageError);
+            assert.match(
+              error.message,
+              /The locale file .*messages\.json is missing key: extensionName/);
+          });
+      }
+    );
+  });
+
+  it('throws an error if the locale file does not exist', () => {
+    return getDefaultLocalizedName({
+      messageFile: '/path/to/non-existent-dir/messages.json',
+      manifestData: manifestWithoutApps,
+    })
+      .then(makeSureItFails())
+      .catch((error) => {
+        log.info(error);
+        assert.instanceOf(error, UsageError);
+        assert.match(
+          error.message,
+          /Error .* file .*messages\.json: .*: no such file or directory/);
+      });
+  });
+
   it('can build an extension without an ID', () => {
     return withTempDir(
       (tmpDir) => {

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+/* eslint no-console:0 */`
	`2`	`+`
	`3`	`+console.log('background script loaded');`