Closed Bug 1594663 Opened 4 years ago Closed 4 years ago

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

Categories

(Firefox :: Protections UI, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 72
Tracking Status
firefox71 --- verified
firefox72 --- verified

People

(Reporter: obotisan, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

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.

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.

Flags: needinfo?(oana.botisan)

nihanth can you look into this?

Flags: needinfo?(nhnt11)

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.

Flags: needinfo?(oana.botisan)

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?

Flags: needinfo?(francesco.lodolo)

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

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.

Flags: needinfo?(nhnt11)

(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.)

Flags: needinfo?(l10n)

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.

Flags: needinfo?(l10n)
Summary: [RTL builds] The number of blocked trackers is not displayed correctly in the Privacy Panel → The number of blocked trackers is not displayed correctly in the Privacy Panel for locales with untranslated strings and ambiguous singular
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED

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

See Also: → 1596149

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.

Attachment #9108434 - Attachment is obsolete: true
Pushed by gijskruitbosch@gmail.com:
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
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 72

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.
Attachment #9108433 - Flags: approval-mozilla-beta?

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.

Attachment #9108433 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.