Create an about:compat page to list active site patches
Categories
(Web Compatibility :: Interventions, enhancement, P3)
Tracking
(firefox67 fixed)
Tracking | Status | |
---|---|---|
firefox67 | --- | fixed |
People
(Reporter: miketaylr, Assigned: twisniewski)
References
Details
Attachments
(1 file)
Comment 1•6 years ago
|
||
Reporter | ||
Comment 2•6 years ago
|
||
Assignee | ||
Comment 3•6 years ago
|
||
Add an about:compat page to the Webcompat GoFaster addon.
Assignee | ||
Comment 4•6 years ago
|
||
Here's a patch. I think I caught all the try failures in a previous run, but I'm doing another one here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a1330f91327ed7d4045b7d374b08795c9b0d955a
Note that I'm making the same modifications to the Fennec and desktop directories of code for the in-tree system addon; they shouldn't vary. The only real difference between the two will be that the CSS stylesheet for about:compat has two media query sections (one for Fennec, one for desktop).
I've also made the about: page and Fluent API stand-in experimental APIs general-purpose enough that they ought to be re-usable without too much extra effort. Hopefully they're self-explanatory, if not ask away.
As for the rest, I've basically just added a background page to the addon to handle communication between the about:compat page and the addon, nothing terribly complicated. The most interesting part is probably how the addon uses a ProcessScript to ensure that the about:compat page has access to content script APIs (browser.*, including my experimental Fluent API). The ALLOW_SCRIPT and URI_MUST_LOAD_IN_CHILD flags were necessary for the about page, otherwise that didn't work.
Note that the specific text and UI details may change, but I felt this was essentially ready for review regardless, as those changes should be trivial compared to the meat of the patch.
Assignee | ||
Comment 5•6 years ago
|
||
Hmm, no I still see a couple of oddball l10n failures that I don't really understand, so I guess I'm not 100% done with that part of the code either (but that doesn't strike me as worth blocking on the reviews).
Assignee | ||
Comment 6•6 years ago
|
||
Alright, I've figured out the knobs to pull and dials to turn to fix that l10n-related build bustage, and another try-run seems fine so far: https://treeherder.mozilla.org/#/jobs?repo=try&revision=74bc22abd8a5451f5f12a5cc271e85a7f829fcde
Assignee | ||
Comment 7•6 years ago
|
||
A try-run of the latest revision seems fine: https://treeherder.mozilla.org/#/jobs?repo=try&revision=412d825b33491888417aa9bda2456d5276a65a16
Updated•6 years ago
|
Comment 8•6 years ago
|
||
Hi Thomas,
I'm one of the authors and maintainers of the Fluent library and I work on Fluent in Gecko effort.
I've been pointed at this bug by :flod and :pike.
I looked at the patchset here and it seems to me like a pretty significant effort to rewrite what DOMLocalization class is doing and to rewrite what FluentBundle class is doing using .properties.
That's pretty tricky because this code is fairly new and active. If you try to rewrite initial document translation (which has a non-trivial interactions with document's load phases) and retranslation using MutationObserver, you end up writing a lot of code that someone has to maintain.
Fortunately, DOMLocalization is pluggable - you can plug your own generateMessages
method which allows you to reuse DOMLocalization directly.
This would mean that instead of writing the whole DOM interaction, and straying away from the declarative API that Fluent uses (data-l10n-id/attrs/args etc.) to use some version of it with data-localization
etc.) you could just use it.
For generateMessages
you'd just keep the part of your code that uses postMessage to retrieve the messages based on a list of message IDs and locale fallback chain. That would be much less code.
On the chrome side, you'd need to retrieve the list of messages, format it using L10nRegistry/Fluent, and push back formatted messages to the content side for DOMLocalization to apply.
Since DOMLocalization is async, it should work well.
Alternatively, we could use this opportunity to finalize bug 1425104 and get Fluent in WebExtensions.
Tbh, since the page is very technical, I'd suggest to even land it non-localized first and file a follow-up to localize it using Fluent over localizing it using a new l10n mechanism that uses the old localization system.
Comment 9•6 years ago
|
||
Actually, one more thought. Since you want to localize a custom about: page that is coming from the extension... could we just expose DocumentL10n WebIDL[0] to that? It should be possible (or maybe it even is already?)
And if that's the case, you should be able to just add <link rel="localization">
to your page and have document.l10n
API available for free!
[0] https://searchfox.org/mozilla-central/source/dom/base/Document.cpp#2031-2056
Assignee | ||
Comment 10•6 years ago
|
||
Hi Zibi, thanks for the info!
I just tried adding these copy-and-pasted lines to my HTML as a quick and dirty test:
<link rel="localization" href="toolkit/about/aboutSupport.ftl"/>
<title data-l10n-id="page-title"/>
Which resulted in my HTML file having an empty body tag, though the head still contains expected info.
So I guess it is working, but I'm doing something catastrophically wrong.
I don't see any browser/web console output, or anything in my console debug log, however. I guess it's time to put on the ol' rubber debugging gloves...
Assignee | ||
Comment 11•6 years ago
|
||
Ah, silly me. I copy-pasted too quickly to realize I wasn't closing the title tag properly for HTML5.
It does look as though it's working "for free", both on desktop and Fennec. But now I'm not sure where I should put my new .ftl file.
@Zibi, would it be better for me to use L10nRegistry manually so I can bundle the file in the browser/extensions/webcompat subfolder with the system addon? Or should I just add a new ftl file to toolkit/locales/en-US/toolkit/about, along with the other about pages? (even if it would be unused if the system addon goes away?)
Comment 12•6 years ago
|
||
It does look as though it's working "for free", both on desktop and Fennec.
Excellent news!
But now I'm not sure where I should put my new .ftl file.
That question is likely for :flod. I'd suggest placing it in browser/extensions/webcompat
but if it's meant to be used in Fennec, then it seems like it should be part of toolkit? I'm not sure... on the other hand if the extension is in browser/extensions/webcompat
, maybe we could place it together and package into the toolkit's omni.ja?
Flod - also, seems like we may be able to migrate all of the data-localization
use cases to Fluent now? https://searchfox.org/mozilla-central/search?q=data-localization&path=
Assignee | ||
Comment 13•6 years ago
|
||
Alright, I'll let flod chime in.
But if it helps make a decision, I have a patch working with the ftl file in toolkit already: https://treeherder.mozilla.org/#/jobs?repo=try&revision=07f37c8a1ad578a2b24b4437909b406d3b0ab8fe
I'm also guessing I would need to manually use the L10nRegistry method if I want the file to come in from browser/extensions/webcompat, since it doesn't end up in the usual objdir localization folder? If so it sounds like I'd have to use similar hackery to what I'm seeing in devtools/client/application/initializer.js?
Comment 14•6 years ago
|
||
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #12)
It does look as though it's working "for free", both on desktop and Fennec.
Excellent news!
But now I'm not sure where I should put my new .ftl file.
That question is likely for :flod. I'd suggest placing it in
browser/extensions/webcompat
but if it's meant to be used in Fennec, then it seems like it should be part of toolkit? I'm not sure... on the other hand if the extension is inbrowser/extensions/webcompat
, maybe we could place it together and package into the toolkit's omni.ja?
I don't have a strong preference. Toolkit seems like a good idea, since it's shared across different products, and I don't think we ever packages something from /extensions into mobile.
Flod - also, seems like we may be able to migrate all of the
data-localization
use cases to Fluent now? https://searchfox.org/mozilla-central/search?q=data-localization&path=
Looking at
https://searchfox.org/mozilla-central/search?q=data-localization%3D&path=
That means Form Autofill, because I don't think we can look at DevTools? Do you mind filing a bug? I could do it, but I would hardly be able to provide enough technical details on how to move away from properties. Maybe worth waiting for this bug to be completed, so it can be used as an example.
Comment 15•6 years ago
|
||
(In reply to Thomas Wisniewski [:twisniewski] from comment #13)
I'm also guessing I would need to manually use the L10nRegistry method if I want the file to come in from browser/extensions/webcompat, since it doesn't end up in the usual objdir localization folder? If so it sounds like I'd have to use similar hackery to what I'm seeing in devtools/client/application/initializer.js?
That and bug 1492597, most likely. In that dependency tree, there's also a bug about adding the registration to the add-on manifest.
Comment 16•6 years ago
|
||
If toolkit works, I'd love to minimize the amount of FileSources we have in play :)
Assignee | ||
Comment 17•6 years ago
|
||
toolkit works for me, if my vote helps :)
Comment 18•6 years ago
•
|
||
(In reply to Francesco Lodolo [:flod] from comment #14)
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #12)
Flod - also, seems like we may be able to migrate all of the
data-localization
use cases to Fluent now? https://searchfox.org/mozilla-central/search?q=data-localization&path=Looking at
https://searchfox.org/mozilla-central/search?q=data-localization%3D&path=That means Form Autofill, because I don't think we can look at DevTools? Do you mind filing a bug? I could do it, but I would hardly be able to provide enough technical details on how to move away from properties. Maybe worth waiting for this bug to be completed, so it can be used as an example.
Bug 1446164 covers part of that and was blocked on bug 1492593 and bug 1492597. Are you saying that those two bugs are no longer necessary? (If so, probably better to have that discussion on bug 1446164).
Reporter | ||
Comment 19•6 years ago
|
||
Tom's proposed strings have been reviewed by product and they have some suggested simplifications -- I'll get those to you shortly Tom. This should also be a NIGHTLY_BUILD feature for now.
Assignee | ||
Comment 20•6 years ago
|
||
Assignee | ||
Comment 21•6 years ago
|
||
Oops, missed a duplicate file in the package: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0464494ece02ffd4d24359d41aa1c1993b5bb92
Assignee | ||
Comment 22•6 years ago
|
||
Alright, I'm not seeing anything worrying in the latest try-run, which I believe address all of the feedback and requests I've received so far:
- I no longer roll my own Fluent stand-in API, I just use document.l10n methods.
- I tweaked the layout and styling of the pages to match the notes I was given.
- I've made it it nightly-only for now.
- I updated how I load the process script to match the latest notes.
In addition, I don't believe there is anything left in the .ftl file that would require careful notes for localizers (it would be good to have that confirmed).
Assignee | ||
Comment 23•6 years ago
|
||
Assignee | ||
Comment 24•6 years ago
|
||
Alright, r+s are granted and the latest try-run seems fine, so I've queued to land this on Lando.
Comment 25•6 years ago
|
||
Updated•6 years ago
|
Comment 26•6 years ago
|
||
bugherder |
Reporter | ||
Updated•6 years ago
|
Description
•