Closed Bug 1476034 Opened 2 years ago Closed 2 years ago

about:crashes localization migration from DTD to Fluent

Categories

(Firefox :: General, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: alexisdeschamps, Assigned: alexisdeschamps)

References

Details

(Keywords: meta)

Attachments

(1 file, 1 obsolete file)

The about:crashes page currently uses DTD for localization and it should change to use Fluent (https://firefox-source-docs.mozilla.org/intl/l10n/docs/l10n/fluent_tutorial.html). Fluent is a more modern localization system being progressively introduced into the Gecko platform.
Assignee: nobody → adeschamps
Status: NEW → ASSIGNED
Blocks: 1463515
No longer depends on: 1463515
Depends on: 1469339
Summary: about:crashes migration from DTD to Fluent → about:crashes localization migration from DTD to Fluent
Blocks: 1476062
No longer blocks: 1476062
The about:crashes page is being updated (bug 1463515).
To facilitate these changes, the patch changes the about:crashes page
to use Fluent for localization instead of DTD.
Attachment #8992792 - Flags: review?(francesco.lodolo)
Comment on attachment 8992792 [details] [diff] [review]
about:crashes: switched to fluent ()

Review of attachment 8992792 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me, switching to feedback for now for two reason:
- I'd like a second pair of eyes on the non .ftl part (going to flag :pike)
- This bug needs a migration, i.e. a file to migrate strings from the old DTD and properties files, to the new FTL one. The process is documented here, and you'd be the first outside of our team to try it out ;-)
https://firefox-source-docs.mozilla.org/intl/l10n/l10n/fluent_migrations.html

Do you have cycles to add a migration recipe to this patch? It should be relatively straightforward, and I'm happy to answer questions on IRC if you get stuck (#l10n).
Attachment #8992792 - Flags: review?(francesco.lodolo) → feedback+
Attachment #8992792 - Flags: feedback?(l10n)
The about:crashes page is being updated (bug 1463515). To facilitate these changes,
this patch changes the about:crashes page to use Fluent for localization instead of the old systems.
This also includes a script to migrate strings from the old .DTD and .properties files
to the new .ftl one.
I am switching over to Phabricator now that I have level 1 commit access: https://phabricator.services.mozilla.com/D2225. I also updated the changes and added the migration script.
Attachment #8992792 - Flags: feedback?(l10n)
Attachment #8992792 - Attachment is obsolete: true
Alexis, I got this to build and package locally. Sadly I had to rebase, as there was a build bustage on mobile with the previous base revision. It also shows the files in resource://gre/localization now. Didn't do any actual testing beyond building it.

We'll also need someone from the build peers to review this, suggesting Nick.

Nick, the mobile/android/locales/jar.mn hackery is the least revolting thing I came up with. Any better suggestion?

What we need here is:

It should not break the build in en-US. That's why I put the whole thing in an #if AB_CD != EN_US. At some point, the build just complains about having packaged localization/en-US/crashreporter/aboutcrashes.ftl if I just duplicate the rule or something like it.

For chrome-%, add additional crashreporter l10n files, but not break if they're not there, as we're doing runtime fallback in that case. Without picking up all of toolkit, that's why we don't call in to toolkit's jar.mn in the first place.
PS: I've tweaked my local german l10n repo to have a localized version of aboutcrashes.ftl, and that showed up in the apk and in resource:// in the expected path.
Susheel Daswani suggested :snorp and :jchen for reviewing https://phabricator.services.mozilla.com/D2225. It would be great if you could help. Thanks!
r?snorp
r?jchen
Flags: needinfo?(snorp)
Flags: needinfo?(nchen)
Comment on attachment 8993040 [details]
about:crashes: switched to fluent (bug 1476034); r=flod, pike

Francesco Lodolo [:flod] has approved the revision.

https://phabricator.services.mozilla.com/D2225
Attachment #8993040 - Flags: review+
Comment on attachment 8993040 [details]
about:crashes: switched to fluent (bug 1476034); r=flod, pike

Axel Hecht [:Pike] has approved the revision.

https://phabricator.services.mozilla.com/D2225
Attachment #8993040 - Flags: review+
Comment on attachment 8993040 [details]
about:crashes: switched to fluent (bug 1476034); r=flod, pike

James Willcox (:snorp) (jwillcox@mozilla.com) has approved the revision.

https://phabricator.services.mozilla.com/D2225
Attachment #8993040 - Flags: review+
Comment on attachment 8993040 [details]
about:crashes: switched to fluent (bug 1476034); r=flod, pike

Jim Chen [:jchen] [:darchons] has approved the revision.

https://phabricator.services.mozilla.com/D2225
Attachment #8993040 - Flags: review+
Comment on attachment 8993040 [details]
about:crashes: switched to fluent (bug 1476034); r=flod, pike

Jim Chen [:jchen] [:darchons] has requested changes to the revision.

https://phabricator.services.mozilla.com/D2225
Attachment #8993040 - Flags: review+
Comment on attachment 8993040 [details]
about:crashes: switched to fluent (bug 1476034); r=flod, pike

Jim Chen [:jchen] [:darchons] has approved the revision.

https://phabricator.services.mozilla.com/D2225
Attachment #8993040 - Flags: review+
Looks good! Thanks for the patch!
Flags: needinfo?(nchen)
The failure is from this assertion,

> MOZ_Assert: Assertion failure: mIsServer (This should only be called in the server mode.), at /builds/worker/workspace/build/src/intl/locale/LocaleService.cpp:350

But looks like your try run is from before your parent-process change, so that might explain the error.
Thanks Sebastian and Jim. That try run did indeed use older changes. Here is the try with the latest changes from the revision: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1423cebc907fa99afa40dc0de7e8a74b264b0d0f. The tests seem to be green. Does this look better :aryx?
Flags: needinfo?(snorp)
Flags: needinfo?(aryx.bugmail)
Flags: needinfo?(adeschamps)
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a92a4691c0da
about:crashes: switched to fluent. r=flod,Pike
Keywords: checkin-needed
Flags: needinfo?(aryx.bugmail)
https://hg.mozilla.org/mozilla-central/rev/a92a4691c0da
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Depends on: 1480798
Depends on: 1481061
You need to log in before you can comment on or make changes to this bug.