[freeside-devel] Multiple Patches, localization of Freeside

[Ivan Kohler]

On Thu, Aug 25, 2016 at 09:43:28PM -0300, Fernando M. Kiernan wrote:
> Please be careful on the order when applying because some patches depends on
> other lower numbered to apply cleanly.
> 
> They are many small patches so you can decide on applying or not with more
> granularity.

Hi Fernando,

I started by reviewing "0001-Localize-preferences-page"

The changes to pref-process.html look good.

The changes to pref.html need some updating.


In all of the places where you substitute a translation directly into 
HTML, you need to escape it.  For example, you do this:

  <% mt("Current password:") %>

but you need to instead do:

  <% emt("Current password:") %>

or

  <% mt("Current password:") |h %>

(This doesn't apply to the argument to header.html or the labels for 
select.html.  It does apply to %customer_views as those are dropped 
directly into the page as HTML.)


Also, when you're including a variable (like a username or number) along 
with a translated string, you shouldn't just concatenate them together.  
For example, you do this:

  mt('Preferences for '). $FS::CurrentUser::CurrentUser->username

There's no guarantee that in all languages, the translated phrase ends 
with the name.  So instead, you should do this:

  mt('Preferences for [_1]', $FS::CurrentUser::CurrentUser->username)

(See the Locale::MakeText documentation for details, especially if you 
need to substitute numbers.)


As an illustration, I made the necessary fixes to pref.html and 
committed the changes to git, so you can see them.  The commit is 
611a7c3b4b717a595a530402d15c1960ba3424bd


Would you fix this in the rest of your patchset?  If you like, I can 
review/apply things as you update them, no need to wait until they're 
all done.


Thanks for the patches!


-- 
Ivan Kohler
President and Head Geek, Freeside Internet Services, Inc.  http://freeside.biz/
Debian GNU/Linux developer  |  CPAN author  |  cat person  |  ski addict