Merge pull request #1376 from mozilla/issue-1372-legal-blank-rendering

fix(legal): fix rendering of legal pages when loaded directly via url

grunttasks/build.js

@@ -17,8 +17,6 @@ module.exports = function (grunt) {
     // use error pages from en_US as the static error pages
     'copy:error_pages',
     'useminPrepare',
-    'l10n-create-json',
-    'l10n-generate-tos-pp:dist',
     'requirejs',
     'css',
     'concurrent:dist',

grunttasks/l10n-generate-pages.js

@@ -3,18 +3,34 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 // grunt task to create a copy of each static page for each locale.
-// Any `{{ locale }}` tags will be replaced with the locale. This is
-// used to create a per-locale template with locale specific resources.
+// Three steps are performed to create the pages:
+//  1. po files for each locale are converted to JSON
+//  2. Terms/Privacy markdown documents are converted to HTML
+//  3. Templates are compiled using the JSON strings and legal doc translations, and with URLs for locale
+// specific resources.
+//
+// They compiled templates are placed in the server's compiled template directory to await further processing
+// (requirejs, minification, revving).
 
 module.exports = function (grunt) {
   'use strict';
 
   var path = require('path');
   var Handlebars = require('handlebars');
+  var Promise = require('bluebird');
+  var legalTemplates = require('../server/lib/legal-templates');
 
+  var defaultLang;
   var templateSrc;
   var templateDest;
 
+  // Legal templates for each locale, key'ed by languages, e.g.
+  // templates['en-US'] = { terms: ..., privacy: ... }
+  var templates = {
+    // The debug language does not have template files, so use an empty object
+    'db-LB': {}
+  };
+
   // Make the 'gettext' function available in the templates.
   Handlebars.registerHelper('t', function (string) {
     if (string.fn) {
@@ -25,20 +41,47 @@ module.exports = function (grunt) {
     return string;
   });
 
-  grunt.registerTask('l10n-generate-pages',
+  grunt.registerTask('l10n-generate-pages', ['l10n-create-json', 'l10n-generate-tos-pp', 'l10n-compile-templates']);
+
+
+  grunt.registerTask('l10n-compile-templates',
       'Generate localized versions of the static pages', function () {
 
+    var done = this.async();
+
     var i18n = require('../server/lib/i18n')(grunt.config.get('server.i18n'));
 
     // server config is set in the selectconfig task
     var supportedLanguages = grunt.config.get('server.i18n.supportedLanguages');
+    defaultLang = grunt.config.get('server.i18n.defaultLang');
 
     templateSrc = grunt.config.get('yeoman.page_template_src');
     templateDest = grunt.config.get('yeoman.page_template_dist');
 
-    supportedLanguages.forEach(function (lang) {
-      generatePagesForLanguage(i18n, lang);
-    });
+    // Legal templates have already been generated and placed in the template destination directory.
+    var getTemplate = legalTemplates(i18n, templateDest);
+
+    // Create a cache of the templates so we can reference them synchronously later
+    Promise.settle(supportedLanguages.map(function (lang) {
+
+      return Promise.all([
+          getTemplate('terms', lang, defaultLang),
+          getTemplate('privacy', lang, defaultLang)
+        ])
+        .then(function (temps) {
+          templates[lang] = {
+            terms: temps[0],
+            privacy: temps[1]
+          };
+        });
+
+    })).then(function () {
+      supportedLanguages.forEach(function (lang) {
+        generatePagesForLanguage(i18n, lang);
+      });
+      done();
+    }).then(null, done);
+
   });
 
 
@@ -61,13 +104,17 @@ module.exports = function (grunt) {
 
     grunt.file.copy(srcPath, destPath, {
       process: function (contents, path) {
+        var terms = templates[context.lang].terms || templates[defaultLang].terms;
+        var privacy = templates[context.lang].privacy || templates[defaultLang].privacy;
         var template = Handlebars.compile(contents);
         var out = template({
           l10n: context,
           locale: context.locale,
           lang: context.lang,
           lang_dir: context.lang_dir,
-          fontSupportDisabled: context.fontSupportDisabled
+          fontSupportDisabled: context.fontSupportDisabled,
+          terms: terms,
+          privacy: privacy
         });
         return out;
       }

server/lib/configuration.js

@@ -159,7 +159,7 @@ var conf = module.exports = convict({
       // the big list of locales is specified so the production build script
       // can build all the locales before config/production.json is written.
       default: ['af', 'an', 'ar', 'as', 'ast', 'be', 'bg', 'bn-BD', 'bn-IN', 'br',
-          'bs', 'ca', 'cs', 'cy', 'da', 'de', 'el', 'en', 'en-GB', 'en-US', 'en-ZA',
+          'bs', 'ca', 'cs', 'cy', 'da', 'de', 'el', 'en-GB', 'en-US', 'en-ZA',
           'eo', 'es', 'es-AR', 'es-CL', 'es-MX', 'et', 'eu', 'fa', 'ff', 'fi',
           'fr', 'fy', 'fy-NL', 'ga', 'ga-IE', 'gd', 'gl', 'gu', 'gu-IN', 'he',
           'hi-IN', 'hr', 'ht', 'hu', 'hy-AM', 'id', 'is', 'it', 'it-CH', 'ja',

server/lib/i18n.js

@@ -39,7 +39,8 @@ module.exports = function (config) {
       debug_lang: config.debugLang,
       supported_languages: config.supportedLanguages,
       translation_directory: config.translationDirectory,
-      translation_type: config.translationType
+      translation_type: config.translationType,
+      locale_on_url: true
     }
   );
 
@@ -88,7 +89,7 @@ module.exports = function (config) {
   // This gives us the properties that i18n-abide attaches to the request
   // object, without actually having to be an express app.
   abideObj.localizationContext = function (acceptLang) {
-    var fakeReq = {headers: {}};
+    var fakeReq = {headers: {}, url: ''};
     var fakeResp = {};
     if (acceptLang) {
       fakeReq.headers['accept-language'] = acceptLang;

server/lib/legal-templates.js

@@ -0,0 +1,69 @@
+/* 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/. */
+
+'use strict';
+
+var fs = require('fs');
+var path = require('path');
+var Promise = require('bluebird');
+var logger = require('intel').getLogger('legal-templates');
+
+module.exports = function (i18n, root) {
+
+  var TOS_ROOT_PATH = path.join(root, 'terms');
+  var PP_ROOT_PATH = path.join(root, 'privacy');
+
+  function getRoot(type) {
+    return type === 'terms' ? TOS_ROOT_PATH : PP_ROOT_PATH;
+  }
+
+  var templateCache = {};
+  function getTemplate(type, lang, defaultLang) {
+    var DEFAULT_LOCALE = i18n.localeFrom(defaultLang);
+
+    // Filenames are normalized to locale, not language.
+    var locale = i18n.localeFrom(lang);
+    var templatePath = path.join(getRoot(type), locale + '.html');
+    var resolver = Promise.defer();
+
+    // cache the promises to avoid multiple concurrent checks for
+    // the same template due to async calls to the file system.
+    if (templateCache[templatePath]) {
+      resolver.resolve(templateCache[templatePath]);
+      return resolver.promise;
+    }
+
+    fs.exists(templatePath, function (exists) {
+      if (! exists) {
+        var bestLang = i18n.bestLanguage(i18n.parseAcceptLanguage(lang));
+
+        if (locale === DEFAULT_LOCALE) {
+          var err = new Error(type + ' missing `' + DEFAULT_LOCALE + '` template: ' + templatePath);
+          return resolver.reject(err);
+        } else if (lang !== bestLang) {
+          logger.warn('`%s` does not exist, trying next best `%s`', lang, bestLang);
+          return resolver.resolve(getTemplate(type, bestLang, defaultLang));
+        }
+
+
+        templateCache[templatePath] = null;
+        return resolver.resolve(null);
+      }
+
+      fs.readFile(templatePath, 'utf8', function(err, data) {
+        if (err) {
+          return resolver.reject(err);
+        }
+
+        templateCache[templatePath] = data;
+        resolver.resolve(data);
+      });
+    });
+
+    return resolver.promise;
+  }
+
+  return getTemplate;
+
+};

server/lib/routes/get-terms-privacy.js

@@ -25,14 +25,13 @@ var Promise = require('bluebird');
 var config = require('../configuration');
 
 var PAGE_TEMPLATE_DIRECTORY = path.join(config.get('page_template_root'), 'dist');
-var TOS_ROOT_PATH = path.join(PAGE_TEMPLATE_DIRECTORY, 'terms');
-var PP_ROOT_PATH = path.join(PAGE_TEMPLATE_DIRECTORY, 'privacy');
 
+var templates = require('../legal-templates');
 
 module.exports = function verRoute (i18n) {
-
   var DEFAULT_LANG = config.get('i18n.defaultLang');
-  var DEFAULT_LOCALE = i18n.localeFrom(DEFAULT_LANG);
+
+  var getTemplate = templates(i18n, PAGE_TEMPLATE_DIRECTORY);
 
   var route = {};
   route.method = 'get';
@@ -44,63 +43,10 @@ module.exports = function verRoute (i18n) {
   // * /<locale>/legal/privacy
   route.path = /^\/(?:([a-zA-Z-\_]*)\/)?legal\/(terms|privacy)(?:\/)?$/;
 
-  function getRoot(type) {
-    return type === 'terms' ? TOS_ROOT_PATH : PP_ROOT_PATH;
-  }
-
-  var templateCache = {};
-  function getTemplate(type, lang) {
-    // Filenames are normalized to locale, not language.
-    var locale = i18n.localeFrom(lang);
-    var templatePath = path.join(getRoot(type), locale + '.html');
-    var resolver = Promise.defer();
-
-    // cache the promises to avoid multiple concurrent checks for
-    // the same template due to async calls to the file system.
-    if (templateCache[templatePath]) {
-      resolver.resolve(templateCache[templatePath]);
-      return resolver.promise;
-    }
-
-    fs.exists(templatePath, function (exists) {
-      if (! exists) {
-        var bestLang = i18n.bestLanguage(i18n.parseAcceptLanguage(lang));
-
-        if (locale === DEFAULT_LOCALE) {
-          var err = new Error(type + ' missing `' + DEFAULT_LOCALE + '` template: ' + templatePath);
-          return resolver.reject(err);
-        } else if (lang !== bestLang) {
-          logger.warn('`%s` does not exist, trying next best `%s`', lang, bestLang);
-          return resolver.resolve(getTemplate(type, bestLang));
-        }
-
-
-        templateCache[templatePath] = null;
-        return resolver.resolve(null);
-      }
-
-      fs.readFile(templatePath, 'utf8', function(err, data) {
-        if (err) {
-          return resolver.reject(err);
-        }
-
-        templateCache[templatePath] = data;
-        resolver.resolve(data);
-      });
-    });
-
-    return resolver.promise;
-  }
-
   route.process = function (req, res) {
-    var lang = req.params[0];
+    var lang = req.params[0] || req.lang;
     var page = req.params[1];
 
-    if (! lang) {
-      // abide should put a lang on the request, if not, use the default.
-      return res.redirect(getRedirectURL(req.lang || DEFAULT_LANG, page));
-    }
-
     getTemplate(page, lang)
       .then(function (template) {
         if (! template) {
@@ -114,9 +60,12 @@ module.exports = function verRoute (i18n) {
             res.send(template);
           },
           'text/html': function () {
+            var context = {};
+            context[page] = template;
+
             // the HTML page removes the header to allow embedding.
             res.removeHeader('X-FRAME-OPTIONS');
-            res.render(page, { body: template });
+            res.render(page, context);
           }
         });
       }, function(err) {

server/templates/pages/src/privacy.html

@@ -34,7 +34,7 @@ <h1 id="fxa-pp-header">{{#t}}Privacy Notice{{/t}}</h1>
 
           <section>
             <article id="legal-copy">
-              {{{ body }}}
+              {{{ privacy }}}
             </article>
           </section>
         </div>

server/templates/pages/src/terms.html

@@ -34,7 +34,7 @@ <h1 id="fxa-tos-header">{{#t}}Terms of Service{{/t}}</h1>
 
           <section>
             <article id="legal-copy">
-              {{{ body }}}
+              {{{ terms }}}
             </article>
           </section>
         </div>

tests/functional/fonts.js

@@ -11,6 +11,7 @@ define([
   'use strict';
 
   var url = intern.config.fxaContentRoot + 'signin';
+  var nonFiraUrl = intern.config.fxaContentRoot + 'zh-CN/legal/privacy';
 
   registerSuite({
     name: 'fonts',
@@ -37,6 +38,27 @@ define([
             assert.ok(value.indexOf('Clear Sans') > -1);
           })
         .end();
+    },
+
+    'Does not use Fira for non-supported locale': function () {
+
+      return this.get('remote')
+        .get(require.toUrl(nonFiraUrl))
+        .waitForElementById('fxa-pp-header')
+
+        .elementByCssSelector('#fxa-pp-header')
+          .getComputedCss('font-family')
+          .then(function (value) {
+            assert.ok(value.indexOf('Fira Sans') === -1);
+          })
+        .end()
+
+        .elementByCssSelector('body')
+          .getComputedCss('font-family')
+          .then(function (value) {
+            assert.ok(value.indexOf('Clear Sans') === -1);
+          })
+        .end();
     }
   });
 });