1959147 - Migrate XSLT errors l10n strings from .properties to Fluent

Status: RESOLVED FIXED

(Core :: XSLT, task)

Description

[Pier Angelo Vendrame]

In D244597, it has been suggested we move xslt.properties to Fluent before handling the spoof English part (Bug 1900648).

Comment 1

[Pier Angelo Vendrame]

It seems that XSLT is the only user of nsStringBundleService::FormatStatusMessage.
So, this migration should also allow to delete that method, which in turn is the only consumer of chrome://global/locale/global-strres.properties, as far as I can tell.
So, I wonder: can I move the only string from that file in the new XSLT Fluent file (assuming it is still needed)?
Otherwise, we might be able to remove the file, if the string isn't needed.

Also, is there some bug I could use as a reference (in addition to the migration documentation)?
Bug 1733498 seems one (and XSLT might need some necko error strings as well).

Comment 2

[Francesco Lodolo [:flod]]

It seems that XSLT is the only user of nsStringBundleService::FormatStatusMessage.

// XXX hack for mailnews who has already formatted their messages

This comment alone raises a potential red flag. Have you checked if it's still used in comm-central? I don't know enough of C++ code to confirm if it's the same function: https://searchfox.org/comm-central/search?q=FormatStatusMessage&path=&case=false&regexp=false

So, I wonder: can I move the only string from that file in the new XSLT Fluent file (assuming it is still needed)?
Otherwise, we might be able to remove the file, if the string isn't needed.

Yes, migrating from .properties to .ftl is always a good thing, as it is removing unused content.

Also, is there some bug I could use as a reference (in addition to the migration documentation)?

For the migration or for Fluent in general? For the former, that doc is the right reference. You can also find a lot of old recipes here, just don't go too far in the past, because the syntax might have changed
https://github.com/flodolo/fluent-migrations/tree/main/recipes/archive

Comment 3

[Pier Angelo Vendrame]

(In reply to Francesco Lodolo [:flod] from comment #2)

It seems that XSLT is the only user of nsStringBundleService::FormatStatusMessage.

// XXX hack for mailnews who has already formatted their messages

This comment alone raises a potential red flag. Have you checked if it's still used in comm-central? I don't know enough of C++ code to confirm if it's the same function: https://searchfox.org/comm-central/search?q=FormatStatusMessage&path=&case=false&regexp=false

Thanks for your answer!
The function used in comm-central is https://searchfox.org/comm-central/rev/1016048a635bd062b826bfe767c86acaeadd004a/mailnews/base/src/nsMsgUtils.cpp#749-780

Comment 4

[Pier Angelo Vendrame]

I've checked the various calls to reportError, which is the function that does localization.
I don't think it can take any networking error (they will be changed with NS_ERROR_XSLT_NETWORK_ERROR, see https://searchfox.org/mozilla-central/rev/9c6e9ca72d4a59267d985fb35ba259fb937c8503/dom/xslt/xslt/txMozillaXSLTProcessor.cpp#987-990).
However, reportError is called also for errors returned by event dispatching and if processing the root of the XML node fails (I don't know which error this could return).
So, I think necko.properties was used in FormatStatusMessage because nsDocLoader used to use it, but at this point I think it's unused, but it'd be a good idea to keep the string for generic errors.

Comment 5

[Pier Angelo Vendrame]

Attachment: Bug 1959147 - Part 1: Migrate XSLT to Fluent. r?#fluent-reviewers

Comment 6

[Pier Angelo Vendrame]

Attachment: Bug 1959147 - Part 2: Remove nsStringBundleService::FormatStatusMessage. r?#xpcom-reviewers

SLT was the last consumer of this method.
After its migration to Fluent, it is not needed anymore.

Also, remove properties files that were used only by
FormatStatusMessage.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d57f3a859955
Part 1: Migrate XSLT to Fluent. r=fluent-reviewers,flod,smaug
https://hg.mozilla.org/integration/autoland/rev/a019651cc743
Part 2: Remove nsStringBundleService::FormatStatusMessage. r=xpcom-reviewers,emilio

Comment 8

[agoloman]

Backed out for causing build bustages @txMozillaXSLTProcessor.cpp.

• Backout link
• Push with failures
• Failure Log

Comment 9

[Pier Angelo Vendrame]

It seems for some reason it builds also without the Localization.h header on my machine.
Anyway, I've added it and I'll wait for the try build at https://treeherder.mozilla.org/jobs?repo=try&revision=79250f9c07c8c3a2b8c0f3ea1b9f2f38660687ae to succeed before sending the updated patch to phab.

Comment 10

[Manuel Bucher [:manuel]]

(In reply to Pier Angelo Vendrame from comment #9)

It seems for some reason it builds also without the Localization.h header on my machine.

This is likely due to compiling with unified builds by default to speed up compilition. A non-unified build is also run in CI.

Comment 11

[Andrew McCreight [:mccr8]]

For local testing of a non-unified build for this directory, you can change UNIFIED_SOURCES to SOURCES in dom/xslt/xslt/moz.build.

Comment 12

[Pier Angelo Vendrame]

I see, thanks for the help!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d87cd2702a67
Part 1: Migrate XSLT to Fluent. r=fluent-reviewers,flod,smaug
https://hg.mozilla.org/integration/autoland/rev/8139136ad2d6
Part 2: Remove nsStringBundleService::FormatStatusMessage. r=xpcom-reviewers,emilio

Comment 14

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/d87cd2702a67
https://hg.mozilla.org/mozilla-central/rev/8139136ad2d6