New issue
Advanced search Search tips

Issue 857538 link

Starred by 0 users

Issue metadata

Status: Available
Owner: ----
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

libdot/hterm: improve localization helper

Project Member Reported by vapier@chromium.org, Jun 28 2018

Issue description

see the discussion in this CL:
  https://chromium-review.googlesource.com/1116205

specifically this bit:
thinking a bit more, we could rework hterm.msg.  atm we inline the fallback message everywhere and set up an empty hterm.messageManager in hterm.js.

if we created a new hterm_l10n.js file and moved both those members there, we could also call hterm.messageManager.addMessages on all the messages we care about during init.  then all hterm.msg callers would be forced to only pass a msgId and no fallback string.

this would have the benefit of addMessages automating the $FOO$->$# placeholders so we wouldn't be forced to use $# directly.

this would also have the benefit of improving unittest coverage here.  hterm_preference_manager_tests.js only covers preference messages, but we could cover all of them since the messages would be centrally registered.  i don't think we need to move the user strings from hterm_preference_manager.js to the new hterm_l10n.js though as that'd violate the "keep the docs and codes close to each other", and because none of those help strings have replacements we need to worry about.
 
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 13

The following revision refers to this bug:
  https://chromium.googlesource.com/apps/libapps/+/632e2700745e735be0671ddd873d2ac857dc1f82

commit 632e2700745e735be0671ddd873d2ac857dc1f82
Author: Mike Frysinger <vapier@chromium.org>
Date: Fri Jul 13 14:43:27 2018

libdot: i18n: move replaceReferences helper here

This is a standalone function and makes more sense in this dedicated
i18n module, and we'll use it more in the future here.

We also take the opportunity to fix/improve it and add some tests.
Specifically, we now allow the substitutions to be empty, missing, or
not have enough elements.  This brings the behavior in line with how
Chrome replaces things, and lets us simplify getMessage a bit.

BUG=chromium:857538

Change-Id: Iecc54f7d17c9ab773e64a7fc16fb4151219de6f5
Reviewed-on: https://chromium-review.googlesource.com/1135780
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>

[modify] https://crrev.com/632e2700745e735be0671ddd873d2ac857dc1f82/libdot/js/lib_i18n.js
[modify] https://crrev.com/632e2700745e735be0671ddd873d2ac857dc1f82/libdot/js/lib_message_manager.js
[modify] https://crrev.com/632e2700745e735be0671ddd873d2ac857dc1f82/libdot/js/lib_i18n_tests.js

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 16

The following revision refers to this bug:
  https://chromium.googlesource.com/apps/libapps/+/6cadbf3194f3c9d4e57865bc59853157cad816a9

commit 6cadbf3194f3c9d4e57865bc59853157cad816a9
Author: Mike Frysinger <vapier@chromium.org>
Date: Mon Jul 16 22:54:20 2018

libdot: i18n: move getMessage helper here

Lets move this out of the message manager so it can be more widely
used, and benefit from the chrome/browser logic in lib.i18n.

This changes the behavior of lib.MessageManager.get slightly in that
we let lib.i18n.getMessage do all the argument replacing for us.
This in turn fixes some latent bugs when getting translations that
the browser knows about -- if the replacements aren't specified, the
browser also strips out the $# values.

BUG=chromium:857538

Change-Id: I2db519d3ceb857419aceb26b315095e453e5efa7
Reviewed-on: https://chromium-review.googlesource.com/1135781
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/6cadbf3194f3c9d4e57865bc59853157cad816a9/libdot/js/lib_i18n.js
[modify] https://crrev.com/6cadbf3194f3c9d4e57865bc59853157cad816a9/libdot/js/lib_message_manager.js
[modify] https://crrev.com/6cadbf3194f3c9d4e57865bc59853157cad816a9/nassh/js/nassh.js
[modify] https://crrev.com/6cadbf3194f3c9d4e57865bc59853157cad816a9/libdot/js/lib_i18n_tests.js

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 17

The following revision refers to this bug:
  https://chromium.googlesource.com/apps/libapps/+/2f1ffd816482f53ac31e5618022eca3c34cb0d64

commit 2f1ffd816482f53ac31e5618022eca3c34cb0d64
Author: Mike Frysinger <vapier@chromium.org>
Date: Tue Jul 17 00:15:42 2018

libdot: prefer browser translations over local ones

Since the browser is the only place where we currently host
translations, prefer that over any registered messages.  In
practice, these databases should be in sync, so it shouldn't
cause issues with old messages.

Bug: 857538
Change-Id: I8b87d0712286e85c64775337cb2f440fc582ee11
Reviewed-on: https://chromium-review.googlesource.com/1139079
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/2f1ffd816482f53ac31e5618022eca3c34cb0d64/libdot/js/lib_message_manager.js

Sign in to add a comment