1594663 - The number of blocked trackers is not displayed correctly in the Privacy Panel for locales with untranslated strings and ambiguous singular

Status: VERIFIED FIXED

(Firefox :: Protections UI, defect)

Description

[Oana Botisan, Desktop QA]

Affected versions

• Firefox 72.0a1
• Firefox 71.0b7

Affected platforms

• Windows 10x64
• Ubuntu 18.04 x64
• macOS 10.13

Prerequisites

• Set "browser.contentblocking.cfr-milestone.milestones" pref to "[15, 5000, 10000, 25000, 50000, 100000, 500000]"
• Set "browser.contentblocking.cfr-milestone.milestone-achieved" to "1000"
• Set "browser.newtabpage.activity-stream.asrouter.providers.cfr" to {"id":"cfr","enabled":true,"type":"local","localProvider":"CFRMessageProvider","frequency":{"custom":[{"period":"daily","cap":10}]},"categories":["cfrAddons","cfrFeatures"],"updateCycleInMs":3600000}

Steps to reproduce

1. Open an ar build.
2. Go to reddit.com and refresh the page a few times.
3. Click on the shield icon.

Expected result

• At the bottom of the Privacy Panel you can see the number of trackers that are blocked.

Actual result

• The number of blocked trackers is 1 no matter how many times you refresh the page.

Regression range

• This is not a regression.

Additional notes

• In about:protection the blocked trackers are counted.
  *It's possible that issue is reproducing on other RTL builds, but as far as we could tell on he builds the issue is not reproducing.

Comment 1

[Erica Wright [:ewright]]

I'm unable to replicate this issue, I'm able to trigger the milestone message on an Arabic build with the following prefs, and it correctly lists "6 trackers"

- Set "browser.contentblocking.cfr-milestone.milestones" pref to "[6, 5000, 10000, 25000, 50000, 100000, 500000]"
- Set "browser.contentblocking.cfr-milestone.milestone-achieved" to "0"
- Set "browser.newtabpage.activity-stream.asrouter.providers.cfr" to {"id":"cfr","enabled":true,"type":"local","localProvider":"CFRMessageProvider","frequency":{"custom":[{"period":"daily","cap":10}]},"categories":["cfrAddons","cfrFeatures"],"updateCycleInMs":3600000}
- Set "browser.contentblocking.cfr-milestone.update-interval" to 10

visit and refresh "https://senglehardt.com/test/trackingprotection/test_pages/tracking_protection.html"
Oana can you confirm?

Sorry about that, I misread and misunderstood the issue.

Comment 2

[Erica Wright [:ewright]]

Attachment: Screen Shot 2019-11-08 at 4.28.47 PM.png

nihanth can you look into this?

Comment 3

[Oana Botisan, Desktop QA]

I think there isn't anything I can help with in this issue at this moment so I will take the needinfo off of me.
But please don't hesitate to tag me if there is anything I can help with.

Comment 4

[:Gijs (he/him)]

So per comment #0 this is happening in Arabic but not Hebrew. I think this is a translation issue of sorts. Compare Arabic:

https://dxr.mozilla.org/l10n-central/rev/0615f06c127aa872af38c7a845c61227b1114f8f/ar/browser/chrome/browser/browser.properties#682-688

and Hebrew:

https://dxr.mozilla.org/l10n-central/rev/9b3fc7eef9671ea4e2e0fcf6c3d68ae779f8d1ea/he/browser/chrome/browser/browser.properties#713-724

Note that the protections.footer.blockedTrackerCounter.description string is missing in Arabic, and ar doesn't have a row present at https://transvision.mozfr.org/string/?entity=browser/chrome/browser/browser.properties:protections.footer.blockedTrackerCounter.description&repo=gecko_strings

However, I would have expected compare-locales and our langpack production to sub in the en-US string in arabic, which should still have the placeholder.

Any idea what's going on here, :flod?

Comment 5

[Francesco Lodolo [:flod]]

If a string is missing, compare-locales at build time packages the English string. So, the Arabic version will have the English string.
But, this is a plural string, and things get really funky.

English has 2 plural form, Arabic has 6 forms.

The English string is 1 Blocked;#1 Blocked. Note that the singular form (form 0) has 1 hard-coded, not the number.

In Arabic form 1 is only used for the number 2. And any time the code tries to get form 2 and above, since English doesn't have these forms, the code falls back to form 0, which has the number hardcoded.
https://searchfox.org/mozilla-central/rev/6566d92dd46417a2f57e75c515135ebe84c9cef5/intl/locale/PluralForm.jsm#121

Unfortunately, this is an expected result. Possible solutions:

• Change the English string #1 Blocked;#1 Blocked.
• Wait for the string to be translated.
• Move to Fluent where the plural form is explicit, and it would be set on [other] for English.

Comment 6

[Axel Hecht [:Pike]]

Yes, this is a problem that doesn't show up in Fluent. The trick is that in PluralForm.jsm, we use the plural form for the app locale, while in Fluent, we use the plural form of the bundle. That is, for strings we have in Arabic, we use the Arabic plural form, and for strings falling back to English, we use the English plural form.

Clearing needinfo on Nihanth, there's nothing he can do about this one.

Comment 7

[:Gijs (he/him)]

(In reply to Axel Hecht [:Pike] from comment #6)

Yes, this is a problem that doesn't show up in Fluent. The trick is that in PluralForm.jsm, we use the plural form for the app locale, while in Fluent, we use the plural form of the bundle. That is, for strings we have in Arabic, we use the Arabic plural form, and for strings falling back to English, we use the English plural form.

Clearing needinfo on Nihanth, there's nothing he can do about this one.

I mean, per comment #5, the short-term workaround would be to have the #1 in the first (number 0) plural form in English, right? And presumably, because that doesn't actually alter the string, this could be uplifted, fixing this in 71, which per comment #0 is the first place we're shipping this?

(To be clear, I'm all for switching to fluent here but I doubt we can do that for 71 anymore.)

Comment 8

[Axel Hecht [:Pike]]

Looking at https://pontoon.mozilla.org/ar/firefox/all-resources/?status=missing&search=Plural, it seems we have this problem in 4 strings right now.

We should also have this problem in Islandic, https://pontoon.mozilla.org/is/firefox/all-resources/?status=missing&search=Plural&string=200871. Resummarizing.

To answer Gijs' question, yes, I think we can switch 1 for #1 w/out an ID change, and that'd limit the impact untranslated strings have in languages with ambiguous singular plural form.

I leave it up to others to decide how widely we should work around this.

Comment 9

[:Gijs (he/him)]

Attachment: Bug 1594663 - ensure first plural form for blocked tracker count in English includes a plural form reference so its contents provide value when used as a fallback in other locales, r?Pike,nhnt11

Comment 10

[:Gijs (he/him)]

Attachment: Bug 1594663 - ensure first plural form in English in devtools includes a plural form reference so its contents provide value when used as a fallback in other locales, r?Pike!,nchevobbe!

This loses the specialcasing for "One message/request", which seems like an acceptable trade-off. We can reinstate that type of distinction when using fluent.

Depends on D52855

Comment 11

[Phabricator Automation]

Comment on attachment 9108434 [details]
Bug 1594663 - ensure first plural form in English in devtools includes a plural form reference so its contents provide value when used as a fallback in other locales, r?Pike!,nchevobbe!

Revision D52856 was moved to bug 1596149. Setting attachment 9108434 [details] to obsolete.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a64b015e2fb9
ensure first plural form for blocked tracker count in English includes a plural form reference so its contents provide value when used as a fallback in other locales, r=ewright

Comment 13

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/a64b015e2fb9

Comment 14

[:Gijs (he/him)]

Comment on attachment 9108433 [details]
Bug 1594663 - ensure first plural form for blocked tracker count in English includes a plural form reference so its contents provide value when used as a fallback in other locales, r?Pike,nhnt11

Beta/Release Uplift Approval Request

• User impact if declined: Missing strings in some locales result in broken UI
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This changes an English singular string so it's usable as a fallback for other locales. Because it doesn't change the meaning/phrasing of the string, there's no string impact. It's a single-line patch and is safe for uplift.
• String changes made/needed: No changes that need translation.

Comment 15

[Pascal Chevrel:pascalc]

Comment on attachment 9108433 [details]
Bug 1594663 - ensure first plural form for blocked tracker count in English includes a plural form reference so its contents provide value when used as a fallback in other locales, r?Pike,nhnt11

Low risk, one line l10n change, uplift approved for 71 beta 11, thanks.

Comment 16

[Oana Pop-Rus]

https://hg.mozilla.org/releases/mozilla-beta/rev/a25e8472d4ff

Comment 17

[Oana Botisan, Desktop QA]

I verified the fix using latest Nightly 72.0a1 and Firefox 71.0b11 on Windows 10 x64, macOS 10.13 and Ubuntu 18.04 x64. The issue is not reproducing anymore.