1497565 - Rename locales to locales-src to avoid accidentally exporting l10n-exposed strings

Status: RESOLVED FIXED

(Firefox :: New Tab Page, enhancement, P1)

Description

[Ed Lee :Mardak]

In bug 1496246, we accidentally did a normal export / buildmc that copies over our browser/locales/…/newtab.properties file that then gets exposed to localization. In the past, I've just manually excluded the string file (while keeping the browser/components/newtab/{,prerendered/}locales/… changes), but I failed to catch it in the review process.

The string changes don't need to be exported because l10n-central already has a unified view of strings across central, beta, etc., so just having the string in mozilla-central is sufficient.

However, the string changes also lead to undesired l10n signoffs as noted in bug 1496246 comment 20.

Comment 1

[Ed Lee :Mardak]

In our 5-whys, we realized that mercurial hook was requiring l10n= on uplifts for both the browser/locales and browser/components/newtab/locales directories when the latter one was rubber-stamped but the former should actually be prevented.

https://github.com/mozilla/version-control-tools/blob/master/hghooks/mozhghooks/prevent_string_changes.py#L41

We can avoid this by renaming our locales directory to something else: locales-src

With this fix, we won't need l10n= and sherrifs should know to not add it for activity stream uplifts.

Comment 2

[Ed Lee :Mardak]

Here's the locales/en-US files under browser/components/newtab:

browser/components/newtab/locales/en-US/strings.properties
browser/components/newtab/prerendered/locales/en-US/activity-stream-noscripts.html
browser/components/newtab/prerendered/locales/en-US/activity-stream-prerendered-noscripts.html
browser/components/newtab/prerendered/locales/en-US/activity-stream-strings.js
browser/components/newtab/prerendered/locales/en-US/activity-stream.html
browser/components/newtab/prerendered/locales/en-US/activity-stream-prerendered.html

So only the first line should be caught by mercurial hook although we should be careful that we don't end up exporting files that end in '.dtd', '.ini', '.properties'

We could rename prerendered/locales/en-US to something else.. e.g., locales-prerendered (NB: prerendered-locales/en-US would still match RegEx(locales/en-US/) ;), but we can skip that for now…

Comment 3

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4506

Comment 4

[[github robot]]

Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/0fb3f8c2671c37356716880b27389d81354b1ab1
Fix Bug 1497565 - Rename locales to locales-src to avoid accidentally exporting l10n-exposed strings (#4506)

Comment 5

[Ed Lee :Mardak]

The changes here just prevents accidentally triggering hghook for undesired files while bug 1499901 is actually preventing uplift changes to a desired file (browser/locales/en-US/chrome/browser/activity-stream/newtab.properties).

Comment 6

[Ed Lee :Mardak]

https://hg.mozilla.org/mozilla-central/rev/826cd78b94cb