about:crashes localization migration from DTD to Fluent

RESOLVED FIXED in Firefox 63

Status

()

P1
normal
RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: alexisdeschamps, Assigned: alexisdeschamps)

Tracking

({meta})

Trunk
Firefox 63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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)

Updated

8 months ago
Assignee: nobody → adeschamps
Status: NEW → ASSIGNED
(Assignee)

Updated

8 months ago
Blocks: 1463515
No longer depends on: 1463515
(Assignee)

Updated

8 months ago
Depends on: 1469339
(Assignee)

Updated

8 months ago
Summary: about:crashes migration from DTD to Fluent → about:crashes localization migration from DTD to Fluent
(Assignee)

Updated

8 months ago
Blocks: 1476062
(Assignee)

Updated

8 months ago
No longer blocks: 1476062
(Assignee)

Comment 1

8 months ago
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)
(Assignee)

Comment 3

8 months ago
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.
(Assignee)

Comment 4

8 months ago
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

Comment 5

8 months ago
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.

Comment 6

8 months ago
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.
(Assignee)

Comment 7

8 months ago
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
(Assignee)

Updated

8 months ago
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)
(Assignee)

Updated

8 months ago
Keywords: checkin-needed
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)

Comment 19

8 months ago
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)

Comment 20

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a92a4691c0da
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
status-firefox63: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63

Updated

8 months ago
Depends on: 1480798

Updated

8 months ago
Depends on: 1481061
You need to log in before you can comment on or make changes to this bug.