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

Bug706572 localization v2 #897

Merged
merged 14 commits into from
Jan 18, 2012
Merged

Bug706572 localization v2 #897

merged 14 commits into from
Jan 18, 2012

Conversation

ozten
Copy link
Contributor

@ozten ozten commented Jan 12, 2012

This is a rebased version of the earlier pull request.

lloyd and others added 5 commits January 12, 2012 11:43
Adding i18n to wsapi. Split strings into client.po and messages.po.

Switched email from mustache to EJS templates.

Created fake locale db-LB for development and debugging.

Updated scripts.
See http://viewvc.svn.mozilla.org/vc?view=revision&revision=99911
res.render('test.ejs', {title: 'BrowserID QUnit Test', layout: false});
}

app.get("/test", QUnit);
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather one of these redirect to the other, so we stop bookmarking both.

@benadida
Copy link
Contributor

overall, this patch looks good, except for one fairly important thing I would strongly suggest tweaking.

The i18n overhead on a request appears to be non-negligible, just from eyeballing the amount of code that's run. But in fact we only need to do all of this stuff on two API calls that relate to sending email. Fortunately, every wsapi call is in its own file with its own metadata now. So we can easily add

exports.i18n = true;

in those wsapi calls that need it, and your middleware can bail early if i18n is not needed for a given call.

@ozten
Copy link
Contributor Author

ozten commented Jan 17, 2012

I'm waiting for r+ before landing this on dev.

I'll merge again to keep the PR merge button green.

…alization_v2

Conflicts:
	lib/configuration.js
	resources/static/dialog/views/pick_email.ejs
	resources/static/shared/renderer.js
…alization_v2

Conflicts:
	resources/views/dialog_layout.ejs
	resources/views/test.ejs
	scripts/compress.sh
…alization_v2

Conflicts:
	lib/configuration.js
@lloyd
Copy link
Contributor

lloyd commented Jan 18, 2012

@ozten and I consulted gerv, and now understand that the final outstanding issue is removing the GPL licensed implementation of gettext(), as it would mean we couldn't release browserid under MPL without violating his license.

Specifically, this library is the issue: https://github.com/mozilla/browserid/blob/a725d667e4ce537404006037d1d72a7c5fd6a1d7/resources/static/shared/gettext.js

Choose your own adventure: rewrite, choose another, or contact the author?

@ozten
Copy link
Contributor Author

ozten commented Jan 18, 2012

Lame. Sorry about that and thanks @lloyd for spotting this legal bug.

There are several client side gettext to choose from. I'll remove and stub out, then replace with a functioning gettext.

@ozten
Copy link
Contributor Author

ozten commented Jan 18, 2012

Following up on other new code:

LGPL libraries:
node-gettext
lib-iconv

ozten added a commit that referenced this pull request Jan 18, 2012
Bug706572 localization v2 r=benadida,stomlinson,lloyd
@ozten ozten merged commit 9e12c06 into dev Jan 18, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants