Closed Bug 859035 Opened 9 years ago Closed 8 years ago

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

Categories

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

defect
Not set
normal

Tracking

(blocking-b2g:leo+, b2g18 fixed, b2g-v1.1hd fixed)

RESOLVED FIXED
blocking-b2g leo+
Tracking Status
b2g18 --- fixed
b2g-v1.1hd --- fixed

People

(Reporter: tchevalier, Unassigned)

Details

(Keywords: l12y, Whiteboard: [NO_UPLIFT])

Attachments

(2 files)

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
Oops, garbled comment.

... and appears when you install a WebApp shouldn't be used for Size, Author, etc.
Attachment #779633 - Flags: review?(felash)
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 :)
Attachment #779633 - Flags: review?(felash)
Stas, Pike, do you think that we want this for 1.1 ?
Flags: needinfo?(community)
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?
Flags: needinfo?(community)
Pike> "hard-pressed", you mean you'd take it, or you wouldn't ?
(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ă
(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.
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.
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');
Slovak is affected, for example. Not a hard blocker though. I'd define it as a good-to-have for us
(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.
ah right, I think there is a difference in behavior between the optimized and unoptimized webapp.

Kaze, could you please enlighten us here ?
Flags: needinfo?(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
Flags: needinfo?(kaze)
Comment on attachment 779633 [details]
master: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11111

I fixed the tests :)
Attachment #779633 - Flags: review?(felash)
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?
Attachment #782840 - Flags: review?(felash)
Attachment #782840 - Flags: feedback?(l10n)
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.
(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?
(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
(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 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
Attachment #782840 - Flags: feedback?(l10n) → feedback+
> though I'd remove it on master again afterwards.

that's the plan, we have 2 different patches for 1.1 and master :)
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.
(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".
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
Théo, I let you change this and then I'll review, ok for you ?
(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 :)
Julien, I updated both patches, ready for review
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.
blocking-b2g: --- → leo?
Attachment #782840 - Attachment description: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11232 → v1-train: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11232
Attachment #779633 - Attachment description: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11111 → master: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11111
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).
Attachment #779633 - Flags: review?(felash) → review+
master: 6979539ef837a388b6fc4fc68b36978874f95cc0
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
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.
Attachment #782840 - Flags: review?(felash) → review+
[NO_UPLIFT] because we have an adhoc patch.
Whiteboard: [NO_UPLIFT]
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 .
Flags: needinfo?(l10n)
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.
Flags: needinfo?(l10n)
Comment 36 sounds like you know where you need this, marking leo+ for the landing.
blocking-b2g: leo? → leo+
Can someone please land the v1-train PR? As far as I know, we
…need that before tomorrow.
v1-train: c60e3507d9df160e006d2e25967d26df2dc543cb

sorry for taking so long théo
v1.1.0hd: c60e3507d9df160e006d2e25967d26df2dc543cb
v1.1.0hd: 6d4a2dee95679b9327a0207bff016f25d05a8983
You need to log in before you can comment on or make changes to this bug.