1432338 - Introduce sync-brand.ftl

Status: RESOLVED FIXED

(Core :: Internationalization, enhancement, P3)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

On par with bug 1424683 (brand.ftl) we need syncBrand.ftl for bug 1424681.

Migration from: https://searchfox.org/mozilla-central/source/browser/locales/en-US/chrome/browser/syncBrand.dtd

I'm not sure if we should place it in browser or branding/.

Comment 1

[Axel Hecht [:Pike]]

I'd go for branding. Also, we'll need to spell out which of those brands are affected by which rules.

Comment 2

[Axel Hecht [:Pike]]

... should we put this into toolkit for android usage?

Comment 3

[Francesco Lodolo [:flod]]

+1 on branding.

From a quick look, Android only uses syncBrand.shortName.label in the fake DTDs for Java (mobile/android/base/android_strings.dtd), not in the chrome part. Not sure if that affects the decision, personally I would leave Android out and only focus on desktop.

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

Are you guys suggesting `/branding/syncBrand.ftl` or `/browser/branding/syncBrand.ftl`?

Because I thought `/branding/syncBrand.ftl` but then I'm not sure what's the issue with android.

Comment 5

[Francesco Lodolo [:flod]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #4)
> Are you guys suggesting `/branding/syncBrand.ftl` or
> `/browser/branding/syncBrand.ftl`?
> 
> Because I thought `/branding/syncBrand.ftl` but then I'm not sure what's the
> issue with android.

Personally I was thinking browser/branding/syncBrand.ftl

I don't like the idea of a new root folder just for branding, assuming that's your suggestion with /branding/syncBrand.ftl

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

> I don't like the idea of a new root folder just for branding, assuming that's your suggestion with /branding/syncBrand.ftl

It wouldn't be a new folder - we use it in bug 1424683 for brand.ftl (and brand.dtd/properties).

Comment 7

[Francesco Lodolo [:flod]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #6)
> > I don't like the idea of a new root folder just for branding, assuming that's your suggestion with /branding/syncBrand.ftl
> 
> It wouldn't be a new folder - we use it in bug 1424683 for brand.ftl (and
> brand.dtd/properties).

Either we're talking about two different things, or I'm lost. 

Bug 1424683 has the file in 
/browser/branding/official/brand.ftl

I'm suggesting to have Sync brand in
/browser/branding/syncBrand.ftl

For reference, prefs.ftl is in
/browser/browser/preferences/preferences.ftl

With "root folder" I meant the repository's root, a folder at the same level as browser, dom, toolkit, etc. On the other hand, I think you mean "root" as "/browser" in the hg repository.

Comment 8

[Zibi Braniecki [:zbraniecki][:gandalf]]

Several questions for :pike and :flod:

 - do we want brandSync.ftl or brand-sync.ftl?
 - does it make sense to add new FTL file for sync brands, or should we add a new message to brand.ftl (and potentially rename it to brands.ftl - for browser brands)?

Comment 9

[Francesco Lodolo [:flod]]

I would go with a separate file (brandSync.ftl), if nothing else because brands.ftl would end up containing elements with completely different policies (Firefox and Sync not localizable, Firefox Account localizable).

https://hg.mozilla.org/l10n/gecko-strings/file/default/browser/chrome/browser/syncBrand.dtd

Comment 10

[Axel Hecht [:Pike]]

I'd go for kebab-case for the file name. Mostly, because, nomnomkebab.

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1432338 - Introduce sync-brand.ftl.

Review commit: https://reviewboard.mozilla.org/r/219566/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/219566/

Comment 12

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8950326 [details]
Bug 1432338 - Introduce sync-brand.ftl.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/219566/diff/1-2/

Comment 13

[Francesco Lodolo [:flod]]

The patch looks good to me, but the migration doesn't work. Can you test that on your side?

If I change the destination file to 'browser/browser/preferences/preferences.ftl' it works, replacing the existing content. For a brand new file, even if already available in the repo, it doesn't work.

I'm trying to figure out what's causing it, because it seems like a bug in the migration code.

Comment 14

[Francesco Lodolo [:flod]]

Comment on attachment 8950326 [details]
Bug 1432338 - Introduce sync-brand.ftl.

https://reviewboard.mozilla.org/r/219566/#review225568

::: python/l10n/fluent_migrations/bug_1432338_sync_brand.py:12
(Diff revision 2)
> +import fluent.syntax.ast as FTL
> +from fluent.migrate import COPY
> +
> +
> +def migrate(ctx):
> +    """Bug 1424683 - Introduce sync-brand.ftl, part {index}."""

Wrong bug number (bug 1432338)

Comment 15

[Francesco Lodolo [:flod]]

Comment on attachment 8950326 [details]
Bug 1432338 - Introduce sync-brand.ftl.

r+ with the commit message fixed in migration

Comment 16

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8950326 [details]
Bug 1432338 - Introduce sync-brand.ftl.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/219566/diff/2-3/

Comment 17

[Francesco Lodolo [:flod]]

Comment on attachment 8950326 [details]
Bug 1432338 - Introduce sync-brand.ftl.

https://reviewboard.mozilla.org/r/219566/#review229662

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a96df7af049c
Introduce sync-brand.ftl. r=flod

Comment 19

[Noemi Erli[:noemi_erli]]

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