[Legal]: text for sponsored tiles needs to be localized for Firefox 33

VERIFIED FIXED in Firefox 33

Status

()

defect
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: CocoMo, Assigned: Mardak)

Tracking

33 Branch
Firefox 33
Points:
5
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(firefox33+ verified, firefox34 unaffected, firefox35 unaffected)

Details

Attachments

(7 attachments, 5 obsolete attachments)

- Language: de, es, fr, ja, pl, pt-BR, ru
- File location: https://hg.mozilla.org/mozilla-central/file/tip/browser/locales/en-US/chrome/browser/newTab.properties In case you can't, see the attachment (when you deliver files, please add locale code at the end of each file).
- Deadline: Friday, Aug 29, or as soon as you can.
- Mozilla Contact: Bryan Clark (I will add him to the bug.  If the files return after this Friday, he will take care of them from the bug, and Corneliu will be cc'd).
NOTE: Localize the string with string ID prefix "newtab." and after "=".  Example: newtab.enhanced.explain=A Mozilla partner has visually enhanced this tile, replacing the screenshot. You can turn off enhanced tiles by clicking the %1$S button for your preferences. %2$S
Context Referenece: See images attached.
Posted image Sponsor tiles
Assignee: nobody → corneliu
clarkbw, is this only for the legal text? Or also for the related menu items:

https://hg.mozilla.org/mozilla-central/file/tip/browser/locales/en-US/chrome/browser/newTab.dtd
     7 <!ENTITY newtab.customize.title "Customize your New Tab page">
     8 <!ENTITY newtab.customize.enhanced "Enhanced">
     9 <!ENTITY newtab.customize.classic "Classic">
    10 <!ENTITY newtab.customize.blank "Blank">
    11 <!ENTITY newtab.customize.what "What is this page?">
    12 <!ENTITY newtab.intro.header "What is this page?">
Hi Ed,

We will include these strings in the delivery as well.  Thank you.

Corneliu, 

As communicated through email, please also include this file.

In the final deliveries: 1). save file as UTF-8 2). Name the file for each language with language code.
I'm quite confused by this bug. These strings are exposed to the localization process, so people will localize them on their own unless we ask them not to (and it will be extremely awkward nonetheless).

For example these strings are already localized in at least 2 locales (it, sk) on mozilla-central, and I'd expect pl, ru and es-ES to be localized within the week.

Also: how do you plan to land this?
I will let Ed and Bryan answer the questions.  They came to me because they are "considered "legal related" text.  The agency can't upload the files without the help of Ed or Bryan.
flod, I believe clarkbw filed this bug because of timing requirements and the legal aspects. If we check in string changes to l10n-central such as the ones you've checked in for it:

http://hg.mozilla.org/l10n-central/it/file/tip/browser/chrome/browser/newTab.dtd
http://hg.mozilla.org/l10n-central/it/file/tip/browser/chrome/browser/newTab.properties

Will localizers see them as already localized, so there wouldn't be duplicate translations?
Thanks Pei (CCing also Chris).

Since the beginning we asked for legal advice since the entire concept of "sponsorship" might be tricky across languages and law systems, not sure how we got from that to "ask legal council to localize UI elements".

(In reply to Francesco Lodolo [:flod] from comment #4)
> I'm quite confused by this bug. These strings are exposed to the
> localization process, so people will localize them on their own unless we
> ask them not to (and it will be extremely awkward nonetheless).

Adding that, as a localizer, you will have a hard-time in convincing me to add strings coming from a third party, especially for UI elements like the new tab settings menu (which has been localized for a long time as of today). 

I'd be perfectly fine with a legal page on mozilla.org linked from this UI, translated by legal council and revised by community. But in-product strings it's a completely different matter.

(In reply to Peiying Mo [:CocoMo] from comment #5)
> The agency can't upload the files without the help of Ed or Bryan.
 
I don't think that Ed or Brian have access to l10n repositories. And if they have, they shouldn't land strings anyway.
(In reply to Ed Lee :Mardak from comment #6)
> flod, I believe clarkbw filed this bug because of timing requirements and
> the legal aspects. 

See also previous comment. "Timing requirements": which version are we targeting? If this is supposed to ride the trains, I'm not exactly sure what kind of timing issue we're looking at.

> If we check in string changes to l10n-central such as the
> ones you've checked in for it:
> Will localizers see them as already localized, so there wouldn't be
> duplicate translations?

We simply don't check-in strings in localizers' repositories. 

Some teams work on l10n-central, some others work on mozilla-aurora. Some work directly on Mercurial, others use external tools. Adding external files in repos is not going to solve problems, is going to create more.
(In reply to Francesco Lodolo [:flod] from comment #8)
> "Timing requirements": which version are we targeting?
I'm not entirely sure myself, but I would assume given the requested string deadline in comment 0 of August 29, that would point to getting new tab strings for the listed locales for Beta 33 (September 2nd).
Then we have another problem, because this requires to break string freeze either on Aurora this cycle, or on Beta next one, or to work around string freeze.
Oh! We don't need to break string freeze as the aurora code being uplifted explicitly does not touch locales files. We put the strings in code, so ignore the previous comments of landing on l10n-central.

https://bugzilla.mozilla.org/attachment.cgi?id=8477762&action=diff
https://bugzilla.mozilla.org/attachment.cgi?id=8477761&action=diff
https://bugzilla.mozilla.org/attachment.cgi?id=8477760&action=diff
We'll probably want to make sure whatever strings we uplift into aurora -> Beta 33 somehow make it to Nightly as well to avoid strings differing between 33 and Nightly 34.

flod, how should we work with the localizers for de, es, fr, ja, pl, pt-BR, ru to get the strings from this bug into nightly?
I confess that I completely lost you.

Bug 1037091 introduced new strings, those patches linked in comment 11 were made to land the same patch on Aurora and then restore the strings in localization files (process that I still find confusing). BTW, did that code land on Aurora yet?

So, English strings are hard-coded in Firefox 33. Do you plan to hardcode all locales like that?

es-ES, fr, pl, ru usually works on l10n-central: they're responsible to keep l10n-central and mozilla-aurora in sync. As I said, I'd expect them to localize those strings before the end of next week (before or right after merge day).

ja: works on mozilla-aurora, doesn't touch l10n-central
de: works on mozilla-aurora, merge mozilla-aurora into l10n-central at the end of the cycle.
pt-BR: I think they work on mozilla-aurora these days

Looking at the dashboard, both ja and pt-BR are a bit behind, so you might want to file bugs to make sure those strings get localized in time.
(In reply to Francesco Lodolo [:flod] from comment #13)
> Bug 1037091 introduced new strings, those patches linked in comment 11 were
> made to land the same patch on Aurora and then restore the strings in
> localization files (process that I still find confusing). BTW, did that code
> land on Aurora yet?
I am about to land them on mozilla-aurora. Here's how the 3 patches I linked in 11 will land with no locales file changes:
https://hg.mozilla.org/try/rev/ec043dac15da
https://hg.mozilla.org/try/rev/d8073de9838a
https://hg.mozilla.org/try/rev/ba7f43e4427a

> So, English strings are hard-coded in Firefox 33. Do you plan to hardcode
> all locales like that?
If the requirement is to not break string freeze on aurora/beta, then I believe this is how we would get strings for the select locales in for Beta 33. We're open to suggestions too for uplifting in a way that doesn't cause unnecessary localization churn.

> es-ES, fr, pl, ru usually works on l10n-central: they're responsible to keep
> l10n-central and mozilla-aurora in sync. As I said, I'd expect them to
> localize those strings before the end of next week (before or right after
> merge day).
So we should reach out to those localizers and provide the strings we get in this bug by the end of this week for them to land on l10n-central?
(In reply to Ed Lee :Mardak from comment #14)
> > So, English strings are hard-coded in Firefox 33. Do you plan to hardcode
> > all locales like that?
> If the requirement is to not break string freeze on aurora/beta, then I
> believe this is how we would get strings for the select locales in for Beta
> 33. We're open to suggestions too for uplifting in a way that doesn't cause
> unnecessary localization churn.

Pike, any alternative suggestion to hard-coded strings? 

It's the second time in a row we rely on this for a new feature (last time it was translations), I really think we should do better than this.

> So we should reach out to those localizers and provide the strings we get in
> this bug by the end of this week for them to land on l10n-central?

No, I think we should wait for them to localize those strings, or file bugs if they're urgent and need to be localized out of cycle. Unless someone decides that these strings need to be localized by legal council.
TBH, I don't know why I should try to understand what's going on here. Reverse engineering the intent by spots in various disconnected conversations doesn't sound promising.

It seems that there's some driver somewhere that wants to have a conversation about this feature with chofmann.
We'll need input on what the legal strings should be for the locales for which we don't have coverage by contractors, including locales that come in through language packs, which are add-ons adding languages to Firefox beyond our control. Sean, can you help on that?

Depending on what that is, we'll need to chose our technical implementation.
Flags: needinfo?(sbohan)
(In reply to Axel Hecht [:Pike] from comment #17)
> We'll need input on what the legal strings should be for the locales for
> which we don't have coverage by contractors, including locales that come in
> through language packs, which are add-ons adding languages to Firefox beyond
> our control. Sean, can you help on that?

We can request help with other locales, the list given in comment 0 is only the priority languages we need expedited.

I think our agreed process was this:
* Translations come back from the agency
* We present those translations to the proper groups / people in the l10n community for the language
* Those contributors can make comments and suggestions we will send back to the agency
* Repeat until no comments / edits
(In reply to Francesco Lodolo [:flod] (offline from Aug 31 to Sep 7) from comment #15)
> any alternative suggestion to hard-coded strings? It's the second time in
> a row we rely on this for a new feature (last time it was translations)
For reference, this is what was done for translations for 32:
https://hg.mozilla.org/releases/mozilla-aurora/rev/089bd4c351c0#l5.12

It's basically what we've done for the new tab related strings for uplift:
http://hg.mozilla.org/releases/mozilla-aurora/file/tip/browser/base/content/newtab/newTab.js#l38

Except now we'll add 14 strings for 7 locales (in addition to what we've already hardcoded for en-US).

CocoMo, just making sure that matches up with your counts on the number of strings?
I have one more question about this: for other locales the feature will be completely disabled, so they won't even see the Settings menu (Enhanced, Classic, etc.) in about:newtab?
(In reply to Francesco Lodolo [:flod] (offline from Aug 31 to Sep 7) from comment #20)
> I have one more question about this: for other locales the feature will be
> completely disabled, so they won't even see the Settings menu (Enhanced,
> Classic, etc.) in about:newtab?

I realized that I could have tried Aurora in Italian, and it works as expected (no English text displayed).
(In reply to Peiying Mo [:CocoMo](offline from Aug 30 - Sept 13) from comment #0)
> - Language: de, es, fr, ja, pl, pt-BR, ru
clarkbw, are we showing de strings to any de-*?

http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/latest-beta/mac/

There's just de listed, but I see add-on pings with de, de-de, de-at, de-ch.

es: es-es, es-mx, es-ar, es-cl

fr: fr

ja: ja, ja-jp-mac

pl: pl

pt: pt-br, pt-pt

ru: ru

I see that you explicitly listed pt-BR. I'm not sure what's expected for ja.
Posted patch wip (obsolete) — Splinter Review
Started using some strings that localizers have committed to l10n-central.
Posted image wip screenshot
(In reply to Ed Lee :Mardak from comment #24)
> Created attachment 8480934 [details]
> wip screenshot
maksik says the russian translations aren't that great with "tile": a construction tile and a little elictrical stove

flod, is there a good way to give feedback? CocoMo will probably get back with differently translated ru strings.

Also, I saw that pl has SPONSORED translated to only capitalizing the first letter:
http://hg.mozilla.org/l10n-central/pl/file/tip/browser/chrome/browser/newTab.properties#l8
newtab.sponsored.button=Sponsorowana
(In reply to Ed Lee :Mardak from comment #25)
> flod, is there a good way to give feedback? CocoMo will probably get back
> with differently translated ru strings.

File bugs in Mozilla Localizations::Locale, localizers should be watching their components, and ask for feedback. I'd wait to have the legal text translated to do it only once though.

See for example what was done for Translation
https://bugzilla.mozilla.org/show_bug.cgi?id=1034626

> Also, I saw that pl has SPONSORED translated to only capitalizing the first
> letter:
> http://hg.mozilla.org/l10n-central/pl/file/tip/browser/chrome/browser/newTab.
> properties#l8
> newtab.sponsored.button=Sponsorowana

Same thing. And be ready for an answer like "I don't like uppercase text".
Hi, Peiying,

I attached localized files in 7 locales.

Please, let me know if you have questions.

Thank you,
Corneliu
Assignee: corneliu → pmo
Status: NEW → ASSIGNED
Thank you Corneliu.

Bryan and Ed, let's figure out what the right process is moving forward.
Assignee: pmo → edilee
Posted patch v1 (obsolete) — Splinter Review
Copied the strings over as is. I'll note that there's differences between existing strings and these, e.g., the new tab title "Nueva pestaña" vs "Personaliza tu página de Pestaña nueva"
Attachment #8480929 - Attachment is obsolete: true
Posted image v1 screenshot
"what's this page?" de wraps; not all titles are ?questions?
css for German needs to be fixed.  The design allows only one liners.
If we change the size of the panel, we'll do that for all locales. Probably something we want to backport to mozilla-central.
(In reply to Ed Lee :Mardak from comment #33)
> Created attachment 8481725 [details]
> de wider 550px vs shorter title
Oh, and I don't know German, so that title might not make any sense. ;)
(In reply to Ed Lee :Mardak from comment #33)
> If we change the size of the panel, we'll do that for all locales. Probably
> something we want to backport to mozilla-central.

At some point I did suggest to have a max-width and then wrap the text on two lines, not sure where or when at this point.
aryx, the exact strings are in Nightly, so you can test there. The strings were moved to newTab.js for Aurora to avoid string freeze.
(In reply to Francesco Lodolo [:flod] (offline from Aug 31 to Sep 7) from comment #35)
> At some point I did suggest to have a max-width and then wrap the text on
> two lines, not sure where or when at this point.
Ah probably bug 1053530 comment 12. Is there an issue with a long menu item?
It could become heavily unbalanced: all other items are one word, this is a sentence.
Imagine putting a setting 1.5 times longer than the German one. It doesn't look great.
If we want the panel's header/title to be on one line, we could change the styling to white-space: nowrap and set a min-width/width on the intro's panel and not have a max-width to let it stretch wider.
(In reply to Ed Lee :Mardak from comment #40)
> If we want the panel's header/title to be on one line, we could change the
> styling to white-space: nowrap and set a min-width/width on the intro's
> panel and not have a max-width to let it stretch wider.

On the contrary, I don't think that long text should be displayed on one line but should wrap in the menu.
(In reply to Bryan Clark (Firefox PM) [:clarkbw] from comment #18)
> (In reply to Axel Hecht [:Pike] from comment #17)
> > We'll need input on what the legal strings should be for the locales for
> > which we don't have coverage by contractors, including locales that come in
> > through language packs, which are add-ons adding languages to Firefox beyond
> > our control. Sean, can you help on that?
> 
> We can request help with other locales, the list given in comment 0 is only
> the priority languages we need expedited.
> 
> I think our agreed process was this:
> * Translations come back from the agency
> * We present those translations to the proper groups / people in the l10n
> community for the language
> * Those contributors can make comments and suggestions we will send back to
> the agency
> * Repeat until no comments / edits

Note, this doesn't answer my question. We will have localizations for which there's no agency, and there are also translations that happen via add-ons, i.e., don't go through our processes at all.
There are basically two options for dealing with that case.

1) Let the localization teams have a crack at the best job possible for legal text, and going with that.  Some localizers are reluctant to do that, others are more bold and adventurous. 

2) Fall back to english where we don't have at least some legal review.

I believe pei told me that fall back to english is more the policy/practice these days but we should confirm.
Posted patch v2 (obsolete) — Splinter Review
clarkbw, do we want to use these strings (and turn on enhanced tiles functionality) for all of en*, de*, es*, fr*, ja*, pl*, pt*, ru* ? Even without geo, the current server can provide links based on locale. Except I'm not entirely sure which locales we'll actually get.

The downside is potentially we show the wrong flavour of en-US to en-GB, etc.; but those clients will then start fetching links and potentially showing content once the server has the data file for that locale.


Unrelated, I've included in the patch comments for the script to fetch the appropriate properties/dtd files to generate the hardcoded strings.
Attachment #8481685 - Attachment is obsolete: true
Attachment #8483241 - Flags: feedback?(clarkbw)
Comment on attachment 8483241 [details] [diff] [review]
v2

If we don't move forward with these strings we won't have those languages for production release.  We're assuming geo comes on board in the next couple weeks.
Attachment #8483241 - Flags: feedback?(clarkbw) → feedback+
Posted patch v3 (obsolete) — Splinter Review
Updated a couple strings that changed since the last patch.
Attachment #8483241 - Attachment is obsolete: true
Attachment #8493945 - Flags: review?(adw)
Comment on attachment 8493945 [details] [diff] [review]
v3

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

r+ with at least the #ifdef part addressed.

This is intended for beta/33, right?  Are we going to make these changes on 34 and 35?  There was another bug a while ago where we hardcoded strings on beta or aurora -- same question there.

::: browser/base/content/newtab/newTab.js
@@ +35,5 @@
>      createBundle("chrome://browser/locale/newTab.properties");
>  });
>  
> +/*
> +let strings = {};

Please `#ifdef 0` this out instead of commenting it out so that it's not included in the final source, like how all the #included JS files in newtab (unnecessarily) ifdef out their license headers.

@@ +71,5 @@
> +  "customize.title", "customize.enhanced", "customize.classic", "customize.blank", "customize.what", "intro.header",
> +  "intro.paragraph1", "intro.paragraph2", "intro.paragraph3", "sponsored.button", "sponsored.explain", "enhanced.explain", "learn.link", "privacy.link",
> +];
> +let enhancedStrings = enhancedStringNames.map(name => locales.map(locale => strings[locale]["newtab." + name]));
> +console.log("const enhancedStrings = [\n  [" + enhancedStrings.map(l => l.map(s => JSON.stringify(s)).join(",\n   ")).join(",\n  ],\n  [") + ",\n  ],\n];")

JSON.stringify(enhancedStrings, undefined, 2)?

@@ +77,2 @@
>  
> +const enhancedStringNames = [

This is the same thing that's in the commented out setup code above, right?  Why include it in the commented out part?  I'd be fine with moving the commented/ifdef'ed out part below this definition if necessary.

@@ +223,4 @@
>  
> +function newTabString(name, args = []) {
> +  let stringIndex = enhancedStringNames.indexOf(name);
> +  if (stringIndex != -1) {

Why not make enhancedStringNames a Map instead of an array?
Attachment #8493945 - Flags: review?(adw) → review+
[Tracking Requested - why for this release]: We want to be able to show tiles for 7 additional locales to satisfy business agreements.

The strings are already localized in central/aurora through the normal process, so this is uplifting specific locales to 33.
OS: Mac OS X → All
Hardware: x86 → All
Summary: [Legal]: text for sponsored tiles needs to be localized → [Legal]: text for sponsored tiles needs to be localized for Firefox 33
Target Milestone: --- → mozilla33
Blocks: 1050643
Ed, it would be nice if this could land before beta 9 (next Thursday). Thanks
Posted patch for uplift (obsolete) — Splinter Review
(In reply to Drew Willcoxon :adw (Away 9/29–10/14) from comment #47)
> This is intended for beta/33, right?  Are we going to make these changes on
> 34 and 35?
Won't need to as the changes here are only to uplift certain strings that already exist in mozilla-aurora.

> Please `#ifdef 0` this out instead of commenting it out so that it's not
> included in the final source, like how all the #included JS files in newtab
> (unnecessarily) ifdef out their license headers.
Done.

> JSON.stringify(enhancedStrings, undefined, 2)?
Duhh. Some reason I was trying too hard to recreate the same format that I originally used.

> > +function newTabString(name, args = []) {
> > +  let stringIndex = enhancedStringNames.indexOf(name);
> > +  if (stringIndex != -1) {
> Why not make enhancedStringNames a Map instead of an array?
We explicitly use the stringIndex later to figure out which string to pick in that JSONified enhancedStrings array. We could use a Map/Set/object to look up strings, but that would require some extra data transformations.
Attachment #8493945 - Attachment is obsolete: true
Posted patch for upliftSplinter Review
Attachment #8496245 - Attachment is obsolete: true
Component: Other → New Tab Page
Product: Mozilla Localizations → Firefox
Target Milestone: mozilla33 → Firefox 33
Version: unspecified → 33 Branch
Comment on attachment 8496356 [details] [diff] [review]
for uplift

Approval Request Comment
[Feature/regressing bug #]: Bug 1030832 / Enhanced Tiles
[User impact if declined]: Users of desired locales will not be able to see enhanced/directory tiles
[Describe test coverage new/current, TBPL]: Existing tests updated to allow for the desired locales.
[Risks and why]: Not the usual way to land strings (but has been done before for translation feature)
[String/UUID change made/needed]: Strings added for the specific locale in code and not localizable dtd/properties
Attachment #8496356 - Flags: approval-mozilla-beta?
Depends on: 1073823
Sorry for the l10n churn, but per bug 1073823, we won't need to specially uplift strings to Firefox 33, and they can ride the trains normally for Firefox 34.

Thanks for the extra effort in getting the strings early.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
Attachment #8496356 - Flags: approval-mozilla-beta?
Maybe I jumped the gun..
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Comment on attachment 8496356 [details] [diff] [review]
for uplift

See comment 52
Attachment #8496356 - Flags: approval-mozilla-beta?
Attachment #8496356 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
https://hg.mozilla.org/releases/mozilla-beta/rev/deaa75a553ac
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
I'm having a hard time tracking what is going on with this bug and where, and when testing an feedback can happen.  Can anyone clearify what the status is, and what plan is?
Points: --- → 5
Iteration: --- → 35.3
QA Contact: cornel.ionce
Mozilla/5.0 (X11; Linux i686; rv:33.0) Gecko/20100101 Firefox/33.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:33.0) Gecko/20100101 Firefox/33.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:33.0) Gecko/20100101 Firefox/33.0

Verified on Firefox 33 beta 8 (build ID: 20140929180120) that the text is localized using the following locales: "de", "es-ES", "fr", "ja-JP", "pl", "pt-BR" and "ru".
Status: RESOLVED → VERIFIED
Flags: needinfo?(sbohan)
You need to log in before you can comment on or make changes to this bug.