Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

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

Merged
merged 2 commits into from Jul 14, 2014
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions grunttasks/build.js
Expand Up @@ -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',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are now run as part of l10n-generate-pages, since it depends on them.

'concurrent:dist',
Expand Down
61 changes: 54 additions & 7 deletions grunttasks/l10n-generate-pages.js
Expand Up @@ -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) {
Expand All @@ -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);

Choose a reason for hiding this comment

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

Can you add a note here about why templateDest instead of templateSrc? I think it's because we have already copied the templates to templateDest, but need to compile them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the legal templates are sourced from the legal repo, so we convert them markdown to HTML and put it in the template destination directory, at which point they are finished with processing. We're using them here to help process the other page templates. I'll add a note!


// 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);

});


Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion server/lib/configuration.js
Expand Up @@ -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',
Expand Down
5 changes: 3 additions & 2 deletions server/lib/i18n.js
Expand Up @@ -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
}
);

Expand Down Expand Up @@ -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;
Expand Down
69 changes: 69 additions & 0 deletions 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;

};
67 changes: 8 additions & 59 deletions server/lib/routes/get-terms-privacy.js
Expand Up @@ -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';
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion server/templates/pages/src/privacy.html
Expand Up @@ -34,7 +34,7 @@ <h1 id="fxa-pp-header">{{#t}}Privacy Notice{{/t}}</h1>

<section>
<article id="legal-copy">
{{{ body }}}
{{{ privacy }}}
</article>
</section>
</div>
Expand Down
2 changes: 1 addition & 1 deletion server/templates/pages/src/terms.html
Expand Up @@ -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>
Expand Down
22 changes: 22 additions & 0 deletions tests/functional/fonts.js
Expand Up @@ -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',
Expand All @@ -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();
}
});
});