Convert migration.dtd to Fluent

ASSIGNED
Assigned to

Status

()

P3
normal
ASSIGNED
3 months ago
18 hours ago

People

(Reporter: jaws, Assigned: ink, Mentored)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

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.

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

Priority: -- → P3
Assignee: nobody → kirkpa47
Status: NEW → ASSIGNED

@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?

Flags: needinfo?(l10n)

Comment 4

2 months ago

You need to throw in an additional Identifier:

FTL.CallExpression(
  callee=FTL.FunctionReference(
    id=FTL.Identifier("PLATFORM"))
  ),
  positional=[],
  named=[],
)
Flags: needinfo?(l10n)

Comment 5

2 months ago

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

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

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

Flags: needinfo?(gandalf)

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.

Flags: needinfo?(hskupin) → needinfo?(gijskruitbosch+bugs)

Comment 9

a month ago

(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?

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(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

Flags: needinfo?(gandalf)

Comment 11

a month ago

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?

Flags: needinfo?(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!

Flags: needinfo?(gandalf)

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

Updated

a month ago
Depends on: 1526925

Comment 14

a month ago

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.

Attachment #9047869 - Attachment description: Bug 1518234 - Migrate migration wizard to Fluent → Bug 1521792 - Migrate unknownConetentType dialog to fluent
Attachment #9038598 - Attachment description: Bug 1518234 - Migrate migration wizard to Fluent, r?jaws → Bug 1518234 - Migrate migration wizard to Fluent r?jaws,Gijs

Updated

18 hours ago
Depends on: 1538968
You need to log in before you can comment on or make changes to this bug.