1518234 - Convert migration.dtd to Fluent

Status: RESOLVED FIXED

(Firefox :: Migration, task, P3)

Description

[Jared Wein [:jaws] (please needinfo? me)]

migration.dtd is located at https://searchfox.org/mozilla-central/source/browser/locales/en-US/chrome/browser/migration/migration.dtd and is referenced by https://searchfox.org/mozilla-central/source/browser/components/migration/content/migration.xul

A migration.ftl file should be created in browser/locales/en-US/browser. The path to reference it from migration.xul will be browser/migration.ftl, similar to how sanitize.ftl is referenced by https://searchfox.org/mozilla-central/source/browser/base/content/sanitize.xul#29.

A migration script will need to be created in python/l10n/fluent_migrations as well.

Comment 1

[Matthew N. [:MattN]]

Please take care to test with the -migration command line flag to ensure Fluent works before the first browser window is shown.

Comment 2

[Ian Kirkpatrick [:ink]]

Attachment: Bug 1518234 - Migrate migration wizard to Fluent r?Gijs

Comment 3

[Francesco Lodolo [:flod]]

@pike

selector=FTL.CallExpression(
    callee=FTL.Function("PLATFORM")
),

Function("PLATFORM") was removed in 0.8. I tried using FunctionReference(), but it doesn't accept a name as parameter.

How can I write this type of migrations with syntax 0.8?

Comment 4

[Axel Hecht [:Pike]]

You need to throw in an additional Identifier:

FTL.CallExpression(
  callee=FTL.FunctionReference(
    id=FTL.Identifier("PLATFORM"))
  ),
  positional=[],
  named=[],
)

Comment 5

[Axel Hecht [:Pike]]

PS: I usually go to https://projectfluent.org/play/ and enable the AST view to dig in to these.

Comment 6

[Francesco Lodolo [:flod]]

Redirecting to Bugzilla, since Phabricator mentions work really poorly for this.

These lines are causing the following error on try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a83e30f08d223da281294409e5809ddc023e3a55&selectedJob=227226835

When I run the test locally I see an error for Error: Source with name "toolkit" already registered. in the Browser Console when the test is hanging.

Could it be that these lines are necessary for running actual migrations but due to the way that Marionette runs the resources never lose their registration? @whimboo @zbraniecki

Comment 7

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

I don't know the test bootstrap model, but that error is specifically to catch a case where the code attempts to register the same source multiple times, as it should always indicate a mistake.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I don't think that I have helpful feedback here given that I don't know the test. Maybe Gijs can give some additional feedback.

But from a quick view it looks like that Firefox doesn't correctly restart.

Comment 9

[:Gijs (he/him)]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #7)

I don't know the test bootstrap model, but that error is specifically to catch a case where the code attempts to register the same source multiple times, as it should always indicate a mistake.

Does something unregister everything when an in-process restart happens?

Comment 10

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

Does something unregister everything when an in-process restart happens?

no. L10nRegistry registers/unregisters in the parent process only. https://searchfox.org/mozilla-central/source/intl/l10n/L10nRegistry.jsm#174-216

Comment 11

[:Gijs (he/him)]

OK, so, having looked at the patch... if the migration dialog runs on startup, we need to add the locale sources before we try to show the dialog. If the migration dialog doesn't run, we need to add the locale sources later (after we have a profile).

I'm not sure if there's any JS browser component code that runs in both cases, prior to the migration dialog showing up when it does, as it's opened directly from appstartup C++ code before much of anything else happens ( https://searchfox.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#4494 ).

I also don't see a way to ask the L10nRegistry if a source has been registered yet.

So some options:

1. propose a better place for initializing strings (ie not final-ui-startup)
2. add a way to check if sources have registered to L10nRegistry
3. add a try...catch around https://searchfox.org/mozilla-central/source/browser/components/nsBrowserGlue.js#1011 in case something else registers, so there's no error that interrupts startup
4. get rid of the exception and just error-less early-return.

Gandalf, which seems best to you?

Comment 12

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

I also don't see a way to ask the L10nRegistry if a source has been registered yet.

That would be trivial do add and I'm happy to have L10nRegistry.prototype.hasSource!

Comment 13

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

Can we add it in this patch or do you prefer a separate bug?

Comment 14

[:Gijs (he/him)]

Ian: you can import the patch from bug 1526925, rebase on top of that, and then use L10nRegistry.hasSource at https://searchfox.org/mozilla-central/rev/5e3bffe964110b8d1885b4236b8abd5c94d8f609/browser/components/nsBrowserGlue.js#1009-1015 to only add the sources there if they aren't already registered. That should hopefully deal with the fallout on the trypush.

Comment 15

[Ian Kirkpatrick [:ink]]

Attachment: Bug 1521792 - Migrate unknownContentType dialog to fluent

Comment 16

[Ian Kirkpatrick [:ink]]

Attachment: Bug 1518234 - Migrate migration wizard to Fluent r?jaws,Gijs

Comment 17

[Phabricator Automation]

Comment on attachment 9047869 [details]
Bug 1521792 - Migrate unknownContentType dialog to fluent

Revision D21799 was moved to bug 1521792. Setting attachment 9047869 [details] to obsolete.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cba0a2073456
Migrate migration wizard to Fluent r=flod,Gijs,fluent-reviewers

Comment 20

[Cristian Brindusan [:cbrindusan]]

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