Last Comment Bug 933272 - Click to play "Tap here to activate plugin." is not localized on multi-locale build
: Click to play "Tap here to activate plugin." is not localized on multi-locale...
Status: VERIFIED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
-- normal (vote)
: Firefox 28
Assigned To: Richard Newman [:rnewman]
:
: Sebastian Kaspari (:sebastian)
Mentors:
Depends on: 933290
Blocks: 792077
  Show dependency treegraph
 
Reported: 2013-10-31 07:59 PDT by Francesco Lodolo [:flod]
Modified: 2016-07-29 14:36 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
verified
+
verified
+
verified
fixed
+


Attachments
Screenshot of Android phone (161.00 KB, image/png)
2013-10-31 07:59 PDT, Francesco Lodolo [:flod]
no flags Details
Cargo-culted patch. v1 (3.93 KB, patch)
2013-10-31 16:22 PDT, Richard Newman [:rnewman]
mark.finkle: review+
francesco.lodolo: feedback+
l10n: feedback+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description User image Francesco Lodolo [:flod] 2013-10-31 07:59:34 PDT
Created attachment 825312 [details]
Screenshot of Android phone

I'm running a multi-locale build (Firefox 25.0) on a phone set to Italian and this text is displayed in English.
Comment 1 User image Francesco Lodolo [:flod] 2013-10-31 08:13:41 PDT
String is here
http://hg.mozilla.org/mozilla-central/file/default/toolkit/locales/en-US/chrome/mozapps/plugins/plugins.dtd

This should be related to bug 792077.
https://hg.mozilla.org/mozilla-central/file/f0d363d72753/mobile/android/locales/jar.mn

I don't see any reference to mozapps/plugins/plugins.dtd
Comment 2 User image Richard Newman [:rnewman] 2013-10-31 11:37:28 PDT
flod: this is all new to me, but I would guess from your Comment 1 that this is a one-line fix: add that DTD to jar.mn?

Is there anything more to it?

How can I verify a fix?
Comment 3 User image Francesco Lodolo [:flod] 2013-10-31 12:04:01 PDT
With bug 792077 we started shipping as few strings as possible, with time we discovered some missing pieces (e.g. bug 890726), fixed with landings like this one
https://hg.mozilla.org/mozilla-central/rev/c1443b4d7e51

For sure this string is in mozapps/plugins/plugins.dtd, not sure if we're using strings from plugins.properties as well for Click to Play, and we're already importing a plugins.properties from dom

CCing Mark Finkle who reviewed a patch for this file in the past and for sure knows more than me about this.
Comment 4 User image Richard Newman [:rnewman] 2013-10-31 16:22:26 PDT
Created attachment 825605 [details] [diff] [review]
Cargo-culted patch. v1

This is entirely cargo-culted. But it does build (and I verified that screwing up the paths causes the build to fail).

Flod: how do I verify this fix?
Comment 5 User image Francesco Lodolo [:flod] 2013-10-31 21:32:34 PDT
Comment on attachment 825605 [details] [diff] [review]
Cargo-culted patch. v1

Review of attachment 825605 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Richard Newman [:rnewman] from comment #4)
> Flod: how do I verify this fix?

1) Install the multi-locale build you've created
2) Set the phone to a foreign language you can understand (French, Spanish, etc)
3) Open a website with flash and check if the string for click to play is localized or not.

For example, if I open http://www.adobe.com/software/flash/about on my Nexus 7 I get "A plugin is needed to display this content." (another string in plugins.dtd), on Galaxy S II "Tap here to activate plugin." (I guess one has Flash installed, the other doesn't).

For what I can understand this file the patch looks good.
Comment 6 User image Richard Newman [:rnewman] 2013-10-31 21:40:52 PDT
(In reply to Francesco Lodolo [:flod] from comment #5)

> 1) Install the multi-locale build you've created

And to confirm, 'cos I know nothing at all about this -- are these instructions to make a multi-locale build accurate?

https://wiki.mozilla.org/Mobile/Fennec/Android#Multilocale_builds


> For what I can understand this file the patch looks good.

Thanks for the feedback!
Comment 7 User image Mark Finkle (:mfinkle) (use needinfo?) 2013-10-31 22:04:25 PDT
Comment on attachment 825605 [details] [diff] [review]
Cargo-culted patch. v1

This seems correct to me.
Comment 8 User image Axel Hecht [:Pike] 2013-11-01 04:54:18 PDT
Comment on attachment 825605 [details] [diff] [review]
Cargo-culted patch. v1

Review of attachment 825605 [details] [diff] [review]:
-----------------------------------------------------------------

Yep, this should work.
Comment 9 User image Richard Newman [:rnewman] 2013-11-04 10:47:27 PST
Waiting to land this until I can verify it, which requires some build fixes (Bug 933290).
Comment 10 User image Richard Newman [:rnewman] 2013-11-18 13:28:51 PST
Verified patch: once the locale was actually specified (matchOS defaults to false in my locale build, for some reason, and thus despite the OS being in es-ES, Gecko was in en-US!), I correctly see "Se necesita un plugin para mostrar este contenido" (the missingPlugin case).

https://hg.mozilla.org/integration/fx-team/rev/4dc1ce530241

We should probably uplift this…
Comment 11 User image Francesco Lodolo [:flod] 2013-11-18 13:30:32 PST
(In reply to Richard Newman [:rnewman] from comment #10)
> We should probably uplift this…

As far as possible :-)
Comment 12 User image Lukas Blakk [:lsblakk] use ?needinfo 2013-11-18 15:22:00 PST
Yes please, nominate with risk assessment asap to get into the second-to-last 26 beta
Comment 13 User image Richard Newman [:rnewman] 2013-11-18 15:26:45 PST
Comment on attachment 825605 [details] [diff] [review]
Cargo-culted patch. v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  Long-standing omission.

User impact if declined: 
  Plugin-related Gecko content strings will always be presented in English.

Testing completed (on m-c, etc.): 
  Manual testing only.

Risk to taking this patch (and alternatives if risky): 
  Slim to none; adds existing DTD and property files to the manifest, so they're included on Android much as they would be on desktop.
String or IDL/UUID changes made by this patch:
Comment 14 User image Wes Kocher (:KWierso) 2013-11-18 18:28:14 PST
https://hg.mozilla.org/mozilla-central/rev/4dc1ce530241
Comment 16 User image Ryan VanderMeulen [:RyanVM] 2013-11-21 19:46:08 PST
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/03560edd4494
Comment 17 User image Ioana Chiorean 2013-11-27 07:05:12 PST
Tested with Galaxy Nexus (4.3) and Galaxy Tab 2 (4.0) using:
Firefox 26 Beta 8 
Firefox 27.0a2
Firefox 28.0a1

The string for tap to play and other text related  to plugins ("You need Adobe Flash Player to .. ") are localized. I will set this to verify fixed.

Note You need to log in before you can comment on or make changes to this bug.