898316 - Gaia should interpret correctly unicode character codes in .properties files

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Francesco Lodolo [:flod]]

Attachment: Screenshot of the Calendar app on Fx OS 1.1

Take for example the Italian localization for the Calendar app
single-month-view-header-format = %B\u0020

The unicode character code \u0020 is just displayed as if it was a string, not interpreted.

Comment 1

[Francesco Lodolo [:flod]]

Note: for sure it, lij, and ru have those \u0020 characters in the strings.

Comment 2

[Fabien Cazenave [:kaze]]

Sorry if that’s a silly question, but why don’t you use a space instead?

I understood the point of those \uxxxx codes when the dominant OS wasn't utf8-proof, but nowadays I think it makes little sense to support them. What’s the use case for that?

Comment 3

[Axel Hecht [:Pike]]

In the gecko parser for .properties, trailing whitespace is stripped:

http://mxr.mozilla.org/mozilla-central/source/xpcom/ds/nsPersistentProperties.cpp#93

Which means that if you want a trailing space, you'll need to end your code with \u0020.

Which also has the benefit that editors removing trailing whitespace don't get a chance to do that. I've seen patches fly by more than once in en-US where that happened.

And quite generally, for some characters, it might be easier to type the unicode one.

Comment 4

[Fabien Cazenave [:kaze]]

Hey Pike, thanks for the quick update.

I tend to think that when an l10n string needs a trailing space, it’s because it’s going to be concatenated on the JS side, which is bad imho (rather than concatenating `foo' and `bar', we should rather rely on a third l10n string containing `{{foo}} {{bar}}', right?). I agree it can be useful for readability though (e.g. \u00a0 for no-break spaces).

Taking to keep this bug on my radar, but patches are welcome too. :-)

Comment 5

[Francesco Lodolo [:flod]]

For the record, it's not me, it's the tool :-)

I didn't realize it until today, when someone asked on #l10n why the two months were displayed as one (they removed the trailing space when localizing).

Comment 6

[Fabien Cazenave [:kaze]]

Attachment: link to pull request — l10n fix

https://github.com/mozilla-b2g/gaia/pull/11194

Comment 7

[Fabien Cazenave [:kaze]]

If our current tools create those \uXXXX, then it’s a good reason to support that. :-)
I’ve added the support for multilines while I’m at it (= lines ending with \).

Comment 8

[Francesco Lodolo [:flod]]

Thanks Fabien, much appreciated :-)

Should I set leo?

I've CCed Alexander, the Russian localizer, if we're shipping Russian in 1.1 we should either fix the localization or the code.

Comment 9

[Alexander L. Slovesnik]

(In reply to Francesco Lodolo [:flod] from comment #8)
> I've CCed Alexander, the Russian localizer, if we're shipping Russian in 1.1
> we should either fix the localization or the code.
I've fixed my locale manually for 1.1 - https://hg.mozilla.org/gaia-l10n/ru/rev/5edd942609ff

Comment 10

[Axel Hecht [:Pike]]

Comment on attachment 782165 [details]
link to pull request — l10n fix

I'm afraid this has false positives if someone did %2F in their localized string, etc. Also, one artifact of the gecko parser is that \u020 is the same as \u0020. unescape of '%u020' is just '%u020'. In compare-locales, I use
 self._post = re.compile('\\\\u([0-9a-fA-F]{0,4})')

The lineending code doesn't look like it's handling 

foo = bar \\

which doesn't escape the newline, but is a trailing backslash. That's a totally academic comment though, I guess, as compare-locales doesn't support that stuff either.

Comment 11

[Fabien Cazenave [:kaze]]

Comment on attachment 782165 [details]
link to pull request — l10n fix

Thanks for the review, I’ve just updated the pull request to address your comments.

Comment 12

[Axel Hecht [:Pike]]

Comment on attachment 782165 [details]
link to pull request — l10n fix

r=me, I'd add one more test, though:

'\\u0020\\u020\\u20%2F' == '   %2F'

Comment 13

[Fabien Cazenave [:kaze]]

I’ve added this test, thanks for the suggestion. Merged on master:
https://github.com/mozilla-b2g/gaia/commit/7d6c6732e38bba5d4eb7883b95b03dd5a65bc789

Comment 14

[Wayne Chang [:wchang]]

Triage - blocking for broken UI and missing information for the user.

Comment 15

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 7d6c6732e38bba5d4eb7883b95b03dd5a65bc789 to:
v1-train: fda907660cc5d82e6f131d6729b33ae4fa2ffa58

Comment 16

[Tim Guan-tin Chien [:timdream] (please needinfo)]

v1.1.0hd: fda907660cc5d82e6f131d6729b33ae4fa2ffa58

Comment 17

[dkumar]

Verified fixed, the issue no longer reproduces

Environmental Variables
Build ID: 20130807071207
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/11bb1b0eefff
Gaia: 60ca81600a080dae33058b0692ecaa213556c926
Platform Version: 18.1

None of the combined months show the unicode characters any more.