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

VERIFIED FIXED in Firefox 26, Firefox OS v1.2

Status

()

Firefox for Android
General
VERIFIED FIXED
4 years ago
11 months ago

People

(Reporter: flod, Assigned: rnewman)

Tracking

unspecified
Firefox 28
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox26+ verified, firefox27+ verified, firefox28+ verified, b2g-v1.2 fixed, fennec+)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
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.
(Reporter)

Comment 1

4 years ago
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
Blocks: 792077

Updated

4 years ago
tracking-fennec: --- → ?
(Reporter)

Updated

4 years ago
Summary: 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 build
Assignee: nobody → rnewman
tracking-fennec: ? → 25+
(Assignee)

Comment 2

4 years ago
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?
Flags: needinfo?(francesco.lodolo)
(Reporter)

Comment 3

4 years ago
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.
Flags: needinfo?(francesco.lodolo)
(Assignee)

Comment 4

4 years ago
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?
Attachment #825605 - Flags: review?(mark.finkle)
Attachment #825605 - Flags: feedback?(francesco.lodolo)
(Reporter)

Comment 5

4 years ago
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.
Attachment #825605 - Flags: feedback?(francesco.lodolo) → feedback+
(Assignee)

Comment 6

4 years ago
(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!
Status: NEW → ASSIGNED
Comment on attachment 825605 [details] [diff] [review]
Cargo-culted patch. v1

This seems correct to me.
Attachment #825605 - Flags: review?(mark.finkle) → review+

Comment 8

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

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

Yep, this should work.
Attachment #825605 - Flags: feedback+
(Assignee)

Updated

4 years ago
Depends on: 933290
(Assignee)

Comment 9

4 years ago
Waiting to land this until I can verify it, which requires some build fixes (Bug 933290).
tracking-fennec: 25+ → +
Flags: needinfo?(rnewman)
(Assignee)

Comment 10

4 years ago
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…
Flags: needinfo?(rnewman)
Target Milestone: --- → Firefox 28
(Reporter)

Comment 11

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

As far as possible :-)
(Assignee)

Updated

4 years ago
tracking-firefox26: --- → ?
tracking-firefox27: --- → ?
Yes please, nominate with risk assessment asap to get into the second-to-last 26 beta
status-firefox26: --- → affected
status-firefox27: --- → affected
status-firefox28: --- → affected
tracking-firefox26: ? → +
tracking-firefox27: ? → +
tracking-firefox28: --- → +
(Assignee)

Comment 13

4 years ago
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:
Attachment #825605 - Flags: approval-mozilla-beta?
Attachment #825605 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/4dc1ce530241
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox28: affected → fixed
Resolution: --- → FIXED
Attachment #825605 - Flags: approval-mozilla-beta?
Attachment #825605 - Flags: approval-mozilla-beta+
Attachment #825605 - Flags: approval-mozilla-aurora?
Attachment #825605 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/a1db4e79b1e2
https://hg.mozilla.org/releases/mozilla-beta/rev/03560edd4494
status-firefox26: affected → fixed
status-firefox27: affected → fixed
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/03560edd4494
status-b2g-v1.2: --- → fixed

Comment 17

4 years ago
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.
Status: RESOLVED → VERIFIED
status-firefox26: fixed → verified
status-firefox27: fixed → verified
status-firefox28: fixed → verified
You need to log in before you can comment on or make changes to this bug.