859035 - [l10n] Do not use the same "unknown" entity for Size & Author when installing a WebApp

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Théo Chevalier]

This unknown entity is located in apps ⊃ system ⊃ system.properties:unknown and appears when you install a 
 shouldn't be used for Size, Author, etc.

For instance in French we have:
unknown = inconnu

But we should have:
unknown-size = inconnue
unknown-author = inconnu

Comment 1

[Théo Chevalier]

Oops, garbled comment.

... and appears when you install a WebApp shouldn't be used for Size, Author, etc.

Comment 2

[Théo Chevalier]

Attachment: master: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11111

Pointer to Github pull-request

Comment 3

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

Comment on attachment 779633 [details]
master: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11111

Hey Théo, thanks for your patch !

You need to update the unit tests as well, see https://travis-ci.org/mozilla-b2g/gaia/builds/9381216 (the first 4 errors, the last one is irrelevant), should be easy enough :)

If you don't want to set up a unit test environement on your computer, each time you push to the pull request you'll trigger a travis job that you can check out. (you may need to reload the PR page a few minutes adter pushing).

Please request a new review from me when you're ready :)

Comment 4

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

Stas, Pike, do you think that we want this for 1.1 ?

Comment 5

[Axel Hecht [:Pike]]

I'd not want this patch on 1.1 as is.

I'd be hard-pressed to take a patch that keeps 'unknown' as a fallback for both 'unknown-size' and 'unknown-author', and we could remove the fallback on master afterwards.

A good argument for that would be a language in the 1.1 bucket that's actually affected. Theo, flod, do you know of one?

Comment 6

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

Pike> "hard-pressed", you mean you'd take it, or you wouldn't ?

Comment 7

[Théo Chevalier]

(In reply to Axel Hecht [:Pike] from comment #5)
> I'd not want this patch on 1.1 as is.
> 
> I'd be hard-pressed to take a patch that keeps 'unknown' as a fallback for
> both 'unknown-size' and 'unknown-author', and we could remove the fallback
> on master afterwards.
> 
> A good argument for that would be a language in the 1.1 bucket that's
> actually affected. Theo, flod, do you know of one?

It seems Romanian could be affected:

Unknown album = Album necunoscut
Unknown error = Eroare necunoscută

Comment 8

[Axel Hecht [:Pike]]

(In reply to Julien Wajsberg [:julienw] (PTO 15th -> 28th July) from comment #6)
> Pike> "hard-pressed", you mean you'd take it, or you wouldn't ?

It means you need a lot of pressure to get it through. Just for my own sanity, I checked that I'm using that phrase right, http://idioms.thefreedictionary.com/be+hard+pressed.

Comment 9

[Francesco Lodolo [:flod]]

I have the feeling that several locales have a different gender for size and author and could be affected (e.g. ca, cs, el, hr, ro, sr). 

Unfortunately the string "author unknown" is never used in Mozilla products like this (probably composed from two strings), so I can't find any real evidence in Transvision.

Comment 10

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

Théo, following comment 5, could you prepare a specific patch for v1-train, where you would do something like :

  var xxx = _('unknown-size') || _('unknown');

Comment 11

[Vlado Valastiak [:wladow]]

Slovak is affected, for example. Not a hard blocker though. I'd define it as a good-to-have for us

Comment 12

[Axel Hecht [:Pike]]

(In reply to Julien Wajsberg [:julienw] (PTO 15th -> 28th July) from comment #10)
> Théo, following comment 5, could you prepare a specific patch for v1-train,
> where you would do something like :
> 
>   var xxx = _('unknown-size') || _('unknown');

isn't _('unknown-size') going to return the en-US string? I think we'll need to dig deeper into the abstraction.

Comment 13

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

ah right, I think there is a difference in behavior between the optimized and unoptimized webapp.

Kaze, could you please enlighten us here ?

Comment 14

[Fabien Cazenave [:kaze]]

(In reply to Axel Hecht [:Pike] from comment #12)
> isn't _('unknown-size') going to return the en-US string?

You’re right: the build system will fallback to the default locale (en-US by default).

(In reply to Axel Hecht [:Pike] from comment #5)
> I'd be hard-pressed to take a patch that keeps 'unknown' as a fallback for
> both 'unknown-size' and 'unknown-author', and we could remove the fallback
> on master afterwards.

We could do that by adding two keys (unknown-author & unknown-size) for on the l10n side and make them point to `unknown' by default:

  [en-US]
  unknown        = unknown
  unknown-author = {{unknown}}
  unknown-size   = {{unknown}}
  
  [fr]
  unknown        = inconnu
  unknown-author = inconnu
  unknown-size   = inconnue

Would that be OK?

> A good argument for that would be a language in the 1.1 bucket that's
> actually affected. Theo, flod, do you know of one?

Romanian uses “Mărime” for “size” in system.properties, which is feminine (⇒ “necunoscută” instead of “necunoscut”).

http://en.bab.la/dictionary/romanian-english/imensitate-m%C4%83rime

Comment 15

[Théo Chevalier]

Comment on attachment 779633 [details]
master: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11111

I fixed the tests :)

Comment 16

[Théo Chevalier]

Attachment: v1-train: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11232

Pointer to Github pull-request

Comment 17

[Théo Chevalier]

Comment on attachment 782840 [details]
v1-train: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11232

Patch with the fix for 1.1 only, as suggested in c#14.

Pike, is that acceptable?

Comment 18

[Francesco Lodolo [:flod]]

That's an interesting approach, but my first reaction as a localizer would be to not touch those variables. 

So, Italian would look like this:

unknown=sconosciuto
unknown-size={{unknown}}
unknown-author={{unknown}} 

If we're going down that road, for sure we need clear localization comments explaining what's happening.

Comment 19

[Théo Chevalier]

(In reply to Francesco Lodolo [:flod] from comment #18)
> If we're going down that road, for sure we need clear localization comments
> explaining what's happening.

You are right. I updated the patch with the following l10n note:

# LOCALIZATION NOTE: these two strings are optional. If you have different
# translations for "Unknown" regarding the subject, you should translate them
# as needed. For instance in French we have:
# unknown-size=inconnue
# unknown-author=inconnu

Does that seems clear enough?

Comment 20

[Francesco Lodolo [:flod]]

(In reply to Théo Chevalier [:tchevalier] from comment #19)
> # LOCALIZATION NOTE: these two strings are optional. If you have different
> # translations for "Unknown" regarding the subject, you should translate them
> # as needed. For instance in French we have:
> # unknown-size=inconnue
> # unknown-author=inconnu

What about something like this?

# LOCALIZATION NOTE: if you leave the original en-US value "{{unknown}}" in
# the following two strings, the localization will be automatically picked
# from the existing string with ID "unknown". If you need to adapt your
# localization to different genders for author and size, you can localize
# these strings. For example in French:
# unknown-size=inconnue
# unknown-author=inconnu

Comment 21

[Théo Chevalier]

(In reply to Francesco Lodolo [:flod] from comment #20)
> What about something like this?
> 
> # LOCALIZATION NOTE: if you leave the original en-US value "{{unknown}}" in
> # the following two strings, the localization will be automatically picked
> # from the existing string with ID "unknown". If you need to adapt your
> # localization to different genders for author and size, you can localize
> # these strings. For example in French:
> # unknown-size=inconnue
> # unknown-author=inconnu

Looks better, thanks!
PR updated

Comment 22

[Axel Hecht [:Pike]]

Comment on attachment 782840 [details]
v1-train: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11232

Interesting, didn't know that webl10n did this. Looks good to me, though I'd remove it on master again afterwards. For that, we should use different IDs now than what we want to do in the end, so I'd go for unknown-size-default or so.

The main reason is that this was overly eager string re-use to begin with, IMHO.

Also, we should make the scope of the localization note explicit, so that tools can pick it up:

# LOCALIZATION NOTE (unknown-size,unknown-author):
# if you leave the original en-US value "{{unknown}}" in
# the following two strings, the localization will be automatically picked
# from the existing string with ID "unknown". If you need to adapt your
# localization to different genders for author and size, you can localize
# these strings. For example in French:
# unknown-size=inconnue
# unknown-author=inconnu

Comment 23

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

> though I'd remove it on master again afterwards.

that's the plan, we have 2 different patches for 1.1 and master :)

Comment 24

[Francesco Lodolo [:flod]]

I think that the idea is to have this situation (not covered by the current patches).

On v1-train:
unknown=Unknown
unknown-size={{unknown}}
unknown-author={{unknown}}

While on Master:
unknown-size-default=Unknown
unknown-author-default=Unknown

Not sure if you'll still need "unknown". The change of the ID is necessary because the same string has a different content.

Comment 25

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

(In reply to Francesco Lodolo [:flod] from comment #24)

> The change of the ID is necessary
> because the same string has a different content.

Oki, I get what you mean.

I'm not fond of the "-default" suffix, I'd rather invert the terms: "size-unknown" and "author-unknown" for the master patch, it makes it clearer (for me at least) that it's for the case "Size: unknown".

Comment 26

[Axel Hecht [:Pike]]

The other way around, actually ;-)

On v1-train:
unknown=Unknown
unknown-size-default={{unknown}}
unknown-author-default={{unknown}}

While on Master:
unknown-size=Unknown
unknown-author=Unknown

Comment 27

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

Théo, I let you change this and then I'll review, ok for you ?

Comment 28

[Théo Chevalier]

(In reply to Julien Wajsberg [:julienw] from comment #27)
> Théo, I let you change this and then I'll review, ok for you ?

Will do it right now :)

Comment 29

[Théo Chevalier]

Julien, I updated both patches, ready for review

Comment 30

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

requesting leo?

see comment 0, comment 7, comment 9, comment 11.

(it's a pity we haven't see the bug before now :( thanks to Théo for providing a patch).

See also comment 22, we worked with the l10n team so that the disruption is minimal for l10n.

Comment 31

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

Comment on attachment 779633 [details]
master: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11111

looks good to me, tried with the twitter app in the marketplace (size is unknown, not the author).

Comment 32

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

master: 6979539ef837a388b6fc4fc68b36978874f95cc0

Comment 33

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

Comment on attachment 782840 [details]
v1-train: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11232

r=me

tried with both french and english language with the twitter app in the marketplace.

Need to get leo+ before landing this.

Comment 34

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

[NO_UPLIFT] because we have an adhoc patch.

Comment 35

[bhavana bajaj [:bajaj]]

needinfo Axel : do you need on 1.1 ? Its unclear if this is a regression and hence we would need it Or the bug is necessarily needed due to new locales in 1.1 .

Comment 36

[Axel Hecht [:Pike]]

Yes, I'd like to land this for 1.1 in Slovak.

There's no impact on existing localization work, and it allows for better strings in the locales where it's needed.

Comment 37

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment 36 sounds like you know where you need this, marking leo+ for the landing.

Comment 38

[Théo Chevalier]

Can someone please land the v1-train PR? As far as I know, we

Comment 39

[Théo Chevalier]

…need that before tomorrow.

Comment 40

[Julien Wajsberg [:julienw] (PTO -> Jul 24)]

v1-train: c60e3507d9df160e006d2e25967d26df2dc543cb

sorry for taking so long théo

Comment 41

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

v1.1.0hd: c60e3507d9df160e006d2e25967d26df2dc543cb
v1.1.0hd: 6d4a2dee95679b9327a0207bff016f25d05a8983