Closed Bug 1457865 Opened 6 years ago Closed 5 years ago

Load localizable name and description for WebExtensions from fluent files

Categories

(WebExtensions :: General, enhancement, P3)

enhancement

Tracking

(firefox72 fixed)

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: Pike, Assigned: aswan)

References

(Depends on 2 open bugs, Blocks 1 open bug)

Details

Attachments

(1 file)

Filing a bug to track a replacement for json-l10n for extension metadata in the fluent world.

This is orthogonal to the usage of fluent inside a web extension, as this is the browser reading data from the extension and showing it somewhere else.

I wonder if we can just port the semantics of messages.json 1 to 1 to reading the same data from messages.ftl?
Component: Add-ons Manager → WebExtensions: General
Flags: needinfo?(gandalf)
Priority: -- → P3
Product: Toolkit → WebExtensions
Depends on: 1492593
Blocks: 1425104

Since bug 1580816 got finished, I took a stab at switching screenshots over to Fluent, but stumbled across the fact that the page action title is set to a localized value from the manifest, and there's no way to do this programmatically:
https://searchfox.org/mozilla-central/rev/05a22d864814cb1e4352faa4004e1f975c7d2eb9/browser/extensions/screenshots/manifest.json#47

So, I'd like to propose something that we do something quick here just for privileged extensions.

As a strawman, what if we add a new manifest key like

  "l10n_resources": ["browser/whatever.ftl"],

If this key is present and the extension is privileged, strings of the form MSG_xxx in a localizable property will get looked up in the given fluent resource(s) instead of messages.json.
If that key isn't present, we still use messages.json and if the extension is not privileged, we log a warning and use messages.json as usual.

Tom, does this sound okay to you? I can do the implementation but want to have some consensus before starting.

Flags: needinfo?(tomica)

Meh, hit submit to fast. The idea here is, like bug 1580816, that the fluent resources referenced from the manifest come from either the browser's built-in strings or from the current language pack. Obviously, we don't want to do this for arbitrary webextensions; for those we'll presumably want to declare resources that are bundled with the extension. But that can come later (ie bug 1492593).

Blocks: 1587544

Sorry for the delay, I had to go read up on Fluent a bit in order to understand how these things all fit together. The proposal seems fine but I would like us to at least sketch out how this will look and work for non-system addons.

For regular pages with a DOM, the story would be similar for all addons, they can us a <link rel> to reference a FTL file packed with the extension, or additionally one bundled with Firefox for system addons.

For the manifest and css files, your proposed solution could work for both regular and system addons, with the latter also being able to reference Firefox bundled resources. That would need something like a schema to distinguish the two cases, for example chrome://branding/brand.ftl, except that I don't like chrome nor any other option I can think of at the moment.

An alternative would be to have a default/fixed "fluent localization resource" path (or resolution algorithm), similar to the existing _locales/XX/messages.json, with an optional override for system addons that wish to use built-in fluent files. I'm guessing your proposal is along those lines?

Flags: needinfo?(tomica)

For regular pages with a DOM, the story would be similar for all addons, they can us a <link rel> to reference a FTL file packed with the extension, or additionally one bundled with Firefox for system addons.

Rereading this, I realize this would also require some way to distinguish between extension FTL files and those built into Firefox. :/

I now really dislike Fluent <link rel=localization> urls which look relative, but are in fact absolute (or "global"), unlike basically any other kind of <link>. :(

(In reply to Tomislav Jovanovic :zombie from comment #4)

I now really dislike Fluent <link rel=localization> urls which look relative, but are in fact absolute (or "global"), unlike basically any other kind of <link>. :(

This is probably a better question for :gandalf or :Pike but I think the use of relative urls is a feature of fluent, not a bug. It enables fallback locations, where if a particular translation is not found in the primary language, fluent can follow a configured list of locales to search for a fallback translation. If those elements had absolute urls, fallback locales would need to have rules for url rewriting or something that sounds a lot messier.

Rereading this, I realize this would also require some way to distinguish between extension FTL files and those built into Firefox. :/

Right, I agree this needs to get addressed if/when unprivileged extensions get access to fluent. I don't think it needs to be any more complicated than what you stated above though, how about a simple boolean-valued manifest key that can only be set by privileged extensions? Or in other words, extensions default to reading translations from (some path within) their xpi file, but privileged extensions can opt in to getting their translations directly from the browser/langpack.

Andrew is right. Fluent paths are relative since they allow for fallback composition.

Rereading this, I realize this would also require some way to distinguish between extension FTL files and those built into Firefox. :/

Yes, that was the plan all along, but we haven't had a driver for that yet. Bug 1462839 is about the next revision of L10nRegistry and bug 1440969 is in particular related. The vision was to have separate "registries" per extension and per each app. Firefox has the one we have right now, but each extensions instantiates its own, independently.

I hope it'll get tackled, but at the moment L10nRegistry is in a weird vacuum between L10nDrivers ownership and Gecko where L10nDrivers want to drive the functionality decisions, but don't have resources to implement it, and Gecko team has not found incentive to staff the project.

Flags: needinfo?(gandalf)

So, the truth is that we don't treat <link rel="localization" href="fo"> as a URL at all. It's a path fragment, which might contain variables, and it's looked up relatively based on a lot of base paths.

Reading up on https://www.w3.org/TR/html50/document-metadata.html#attr-link-href, this might not have been a great choice. Not that I have a good choice at my fingertips.

We're certainly in a stage where we can use a different way of describing the information fluent needs to load resources.

Are there some docs or at least code comments that explain how fluent resource resolution/fallback composition works? It seems that we don't have a shared meaning of the term "relative".

Since we're talking about the Web and web-related platforms (Firefox source and extensions), I'm using the term "relative" to the URI of the document that contains <link rel=localization>. What I understand from reading the public Fluent docs, using the same resource "branding/brand.ftl" in two different documents (at two different paths, like "chrome://toolkit/content" and "chrome://toolkit/content") would resolve to the same set of messages, which makes that localization "url" non-relative, and thus "absolute" or "global" (whatever it ends up resolving to and returning, however complex that algorithm is).

It looks to me that all the references currently in firefox should be changed to be of the form "/branding/brand.ftl". I believe chrome:// and resource:// are schemes without hosts, so all of firefox localization resources would be from the same registry. The moz-extensions:// scheme on the other hand has hosts, so each would get their own registry (and additionally, system addons could explicitly reference chrome://branding/brand.ftl).

This is an important discussion, but does it need to be a blocker for this bug? All I'm proposing to do here is extend what we already do with document.l10n to the manifests of privileged webextensions. FWIW, I agree with :zombie (and :Pike in comment 7 I think) that using href attributes that look like relative urls but that are not relative to documentURI is at best confusing, but I think that if/when that is changed, a bunch of existing code will have to be updated. I think the benefits we'll get from being able to convert screenshots to fluent right now outweigh the cost of adding one more thing that will need to change.

Reading up on https://www.w3.org/TR/html50/document-metadata.html#attr-link-href, this might not have been a great choice. Not that I have a good choice at my fingertips.

We spent a lot of time investigating our options and we were aware that link-href is not matching the spec, but we couldn't find anything better.

It seems that we don't have a shared meaning of the term "relative".

Ah, right! Your explanation does make sense, thank you!

I agree that we're derailing the ticket here a bit, but there's a few things to clarify:

<link rel="localization"> does not link to a resource. It's merely providing data to the algorithm in the L10nRegistry to create a series of URLs, with a series of languages as additional data. With locales ["de", "en-US"], <link rel="localization" href="browser/browser.ftl"> corresponds to the following list of URLS:

resource://app/localization/de/browser/browser.ftl
resource://gre/localization/de/browser/browser.ftl
resource://app/localization/en-US/browser/browser.ftl
resource://gre/localization/en-US/browser/browser.ftl

No Fluent file should be accessible through the chrome:// protocol. That's because it's locale management can only return a single resource, while we need a number of those.

Both resource:// and chrome:// protocols have hosts. In the case of resource://, it's empty, 'app', or 'gre'. In the case of chrome://, common hosts are browser or toolkit.

All Firefox Fluent files are accessible through the resource:// protocol, with a shared registry for all hosts (app and gre, per https://searchfox.org/mozilla-central/search?q=l10n-registry&case=false&regexp=false&path=*.manif). We shouldn't conflate registries with hosts here.

There's sadly no spec or docs beyond code for the actual resolver, https://searchfox.org/mozilla-central/rev/17756e2a5c180d980a4b08d99f8cc0c97290ae8d/intl/l10n/L10nRegistry.jsm#622-629.

Last but not least, resource://app/localization/en-US/browser/browser.ftl and resource://app/localization/en-US//browser/browser.ftl are not the same thing, the latter is a 404.

Thanks for the explanation and providing context Axel.

Just a note that I never wanted to imply that a localization rel link needs to resolve to a single specific ftl file, only that we should probably make them look and feel more like real URLs, so that we don't need to reinvent a bunch of stuff out of whole cloth, and can probably reuse schemes/hosts to map to different Registries.

(In reply to Andrew Swan [:aswan] from comment #9)

This is an important discussion, but does it need to be a blocker for this bug?

I didn't expect for any of the these things to get resolved here, but I believe the discussion itself was worthwhile, at least for me to be able to evaluate the proposed solution within the full context.

I think the benefits we'll get from being able to convert screenshots to fluent right now outweigh the cost of adding one more thing that will need to change.

Sure, I just wanted to understand all the things that will need changing if we go down this path.

There's a bit of orange here but it looks to all be unrelated known intermittents:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b48a5ffc63b3553dded6d794f09a231090e9eb6b

I appear to have lost my level 3 access and the checkin-needed keyword is gone. Help?

Flags: needinfo?(tomica)

(In reply to Andrew Swan [:aswan] from comment #15)

I appear to have lost my level 3 access and the checkin-needed keyword is gone. Help?

You should be able to do it from Phabricator
https://wiki.mozilla.org/Sheriffing/How_To/Landing_checkin-needed_patches

Pushed by tjovanovic@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2cea92c059ec
Allow (some) manifest keys to be localized with fluent in privilged extensions r=zombie

Ah, found the tag in Phabricator (eventually) but I think I mid-aired zombie...

Flags: needinfo?(tomica)

(In reply to Andrew Swan [:aswan] from comment #15)

I appear to have lost my level 3 access

You should be able to ask for your SCM bits back, see for example bug 1577299 (just ask to "move" instead of "copy").

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
Assignee: nobody → andrew.swan

Hello,

If this requires manual validation could you please provide some steps to test by for QA? Also the "qe+ verify" flag should be set.
If no manual testing is needed, please set this as "qe verify-". Thank you.

Flags: needinfo?(andrew.swan)

This is an internal api change for system addons, no need for manual verification, thanks.

Flags: qe-verify-
Flags: needinfo?(andrew.swan)
Depends on: 1733466
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: