933272 - Click to play "Tap here to activate plugin." is not localized on multi-locale build

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Francesco Lodolo [:flod]]

Attachment: 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

[Francesco Lodolo [:flod]]

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

[Richard Newman [:rnewman]]

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

[Francesco Lodolo [:flod]]

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

[Richard Newman [:rnewman]]

Attachment: 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

[Francesco Lodolo [:flod]]

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

[Richard Newman [:rnewman]]

(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

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 825605 [details] [diff] [review]
Cargo-culted patch. v1

This seems correct to me.

Comment 8

[Axel Hecht [:Pike]]

Comment on attachment 825605 [details] [diff] [review]
Cargo-culted patch. v1

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

Yep, this should work.

Comment 9

[Richard Newman [:rnewman]]

Waiting to land this until I can verify it, which requires some build fixes (Bug 933290).

Comment 10

[Richard Newman [:rnewman]]

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

[Francesco Lodolo [:flod]]

(In reply to Richard Newman [:rnewman] from comment #10)
> We should probably uplift this…

As far as possible :-)

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

Yes please, nominate with risk assessment asap to get into the second-to-last 26 beta

Comment 13

[Richard Newman [:rnewman]]

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

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4dc1ce530241

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a1db4e79b1e2
https://hg.mozilla.org/releases/mozilla-beta/rev/03560edd4494

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/03560edd4494

Comment 17

[Ioana Chiorean]

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.