Closed Bug 1579705 Opened 2 months ago Closed 2 months ago

Enhanced Tracking Protection is reusing level labels from Content Blocking

Categories

(Firefox :: Preferences, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 71
Tracking Status
firefox70 --- fixed
firefox71 --- fixed

People

(Reporter: flod, Assigned: ewright)

Details

(Whiteboard: [skyline])

Attachments

(1 file)

https://searchfox.org/mozilla-central/rev/4bec213c1a534329bd6fd97f00c0fcfbd60e26ec/browser/locales/en-US/browser/preferences/preferences.ftl#905

The section header is now "Enhanced Tracking Protection", not "Content Blocking", but the strings weren't changed.

Since levels are adjectives, if the reference noun changes gender, the existing string isn't correct anymore. Just changing the localize string would create the opposite problem in any update to 68 ESR.

I've got a question from our Czech localizer a few days ago, and another one today for Italian, which made me start thinking it might be a larger problem.

One possible solution for preferences.ftl would be to create 3 new strings, and use the FTL2FTL migration to avoid losing translation. At that point localizers would be free to change the translation afterwards if they need, and we wouldn't have English content in the meantime.

The comment also mentions browser.properties. Are those strings actually still in use? I can't find them in the UI.

@johann
Anyone with cycles on your side to look into this? It should be quite straightforward, and I can help with the migration if needed.

We should also try to uplift to beta.

Flags: needinfo?(jhofmann)
Summary: Enhanced Tracking Protection is reusing levels from Content Blocking → Enhanced Tracking Protection is reusing level labels from Content Blocking

Erica, do you think you can take this one as well?

The comment also mentions browser.properties. Are those strings actually still in use? I can't find them in the UI.

Nope, seems like we forgot to remove them. Thanks for catching that.

Flags: needinfo?(jhofmann) → needinfo?(ewright)
Priority: -- → P1
Whiteboard: [skyline]
Assignee: nobody → ewright
Status: NEW → ASSIGNED
Flags: needinfo?(ewright)
Pushed by ewright@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7cdfad033225
Update level labels for Enhanced Tracking Protection. r=fluent-reviewers,johannh,flod
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71

Verified on 71.0a1 (2019-09-12) (64 bit)

Status: RESOLVED → VERIFIED

Comment on attachment 9091837 [details]
Bug 1579705 - Update level labels for Enhanced Tracking Protection.

Beta/Release Uplift Approval Request

  • User impact if declined: For some languages it won't be possible to translate the ETP levels in a natural way.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • 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): We have a migration recipe, copying over existing translations, to mitigate the risk of these strings remaining untranslated in 70.
  • String changes made/needed: Yes
Attachment #9091837 - Flags: approval-mozilla-beta?

Comment on attachment 9091837 [details]
Bug 1579705 - Update level labels for Enhanced Tracking Protection.

Fix for l10n issue needed for Skyline. OK for uplift for beta 7.

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

We tried to uplift this and receive the l10n approval error

************************* ERROR ***************************
remote:
remote: File used for localization (browser/locales/en-US/chrome/browser/browser.properties) altered in this changeset
remote: File used for localization (browser/locales/en-US/chrome/browser/browser.dtd) altered in this changeset
remote:
remote: This repository is string frozen. Please request explicit permission from
remote: release managers to break string freeze in your bug.
remote: If you have that explicit permission, denote that by including in
remote: your commit message l10n=...
remote: *************************************************************
remote:
remote: transaction abort!

Since we didn't find that info in the bug comments (tried IRC too) we abandoned uplifting this.
Please specify the l10n approvals so next shift can uplift.
Thank you.

Flags: needinfo?(lhenry)

Same here, add l10n=flod

Flags: needinfo?(lhenry)
QA Whiteboard: [qa-triaged]

Setting the qe-verify flag to - after discussing with flod over slack.

Flags: qe-verify+ → qe-verify-
You need to log in before you can comment on or make changes to this bug.