Closed Bug 1488845 Opened 6 years ago Closed 5 years ago

Create an about:compat page to list active site patches

Categories

(Web Compatibility :: Interventions, enhancement, P3)

Product:
Web Compatibility
Component:
Interventions
Type:
enhancement

Tracking

(firefox67 fixed)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox67 --- fixed

People

(Reporter: miketaylr, Assigned: twisniewski)

References

Details

Attachments

(1 file)

Bug 1488845 - Add an about:compat page to the Webcompat GoFaster addon. r?kmag r?denschub
47 bytes, text/x-phabricator-request
Details | Review 
We should create a new about page that lists active webcompat system addon site patches. 

It's possible it could show Enterprise-policy compat settings as well like in Edge, but that can come at some future point.
on Android, in addition, this page should probably list also the user agent overrides  in a more readable form than just a link to the UA override JSON file.
Given that the site patches live in a web extension, we could have the web extension population the about:compat page... which by default would say "nothing active" (which would be true if the addon was disabled).

Add an about:compat page to the Webcompat GoFaster addon.

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.

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).

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

Attachment #9040607 - Attachment description: Bug 1488845 - Add an about:compat page to the Webcompat GoFaster addon. r?aswan r?denschub → Bug 1488845 - Add an about:compat page to the Webcompat GoFaster addon. r?kmag r?denschub

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.

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

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...

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?)

Flags: needinfo?(gandalf)

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=

Flags: needinfo?(gandalf) → needinfo?(francesco.lodolo)

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?

(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 in browser/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.

Flags: needinfo?(francesco.lodolo)

(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.

If toolkit works, I'd love to minimize the amount of FileSources we have in play :)

toolkit works for me, if my vote helps :)

(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).

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.

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).

Alright, r+s are granted and the latest try-run seems fine, so I've queued to land this on Lando.

Pushed by twisniewski@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ecb22648747e
Add an about:compat page to the Webcompat GoFaster addon. r=denschub,kmag,Pike
Assignee: nobody → twisniewski
Status: NEW → ASSIGNED

https://hg.mozilla.org/mozilla-central/rev/ecb22648747e

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox67: --- → fixed
Resolution: --- → FIXED
Depends on: 1530824
Blocks: 1533437
Depends on: 1533841
Depends on: 1539833
Depends on: 1539916
No longer depends on: 1539833
No longer depends on: 1533841, 1539916
See Also: → 1747267
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: