We track webcompat-reporter bugs in Web Compat Tools::General, moving there. :)

Andrew, will it be possible to somehow get preference values as a webextension? e.g., https://dxr.mozilla.org/mozilla-central/source/browser/extensions/webcompat-reporter/content/WebCompatReporter.jsm#21-28 (we also maintain webextension-based addons here https://github.com/webcompat/webcompat-reporter-extensions, but they're less powerful).

Its not possible to get arbitrary prefs from a general-purpose webextension, but we can either add a new API that's restricted to webcompat-reporter or webcompat-reporter can bundle some privileged code to get at the preferences it needs. I'd be happy to walk you through how this works on irc or vidyo or something some time if that would help.

I'm gonna try to port this over to a bundle web extension experiment, hopefully within this quarter! References: https://firefox-source-docs.mozilla.org/toolkit/components/extensions/webextensions/index.html https://github.com/rhelmer/webext-experiment-crashme/tree/bundled-experiment

Hi Mike, Checking in on if this bug has an owner. I'm not sure which channels webcompat-reporter is used against. Timing wise bootstrapped code will start being removed from 65 Nightly after Sept 22 (completed before Dec 11). Bootstrapped add-ons won't work in 65 Beta (on Dec 11) or 65 Release (on Jan 29th).

correction - Nightly code removal starts OCTOBER 22 (not Sept), sorry.

Heya Shell. Yeah, Tom will be working on this -- our goal is to get this finished by the end of Q3, so I think we should be in good shape. AFAIU, he has local patches, which is good...given that the end of the quarter is quickly approaching. When he's back from PTO next week he'll pick this up.

Here's a work-in-progress patch that just removes the old addon's files, adds in my webextension version's files, and updates moz.build accordingly. Unfortunately, it's not working when I actually build it - the page action doesn't appear, and nothing related is logged in the browser console. I don't see any build warnings, either. Whereas if I load the webextension from the patched directory in about:debugging in a running browser, it does work fine. I'm not sure what I'm doing wrong here.

Comment on attachment 9010051 [details] [diff] [review] wip.patch Ah, it turns out that the patch *was* working, I just had a stale temp profile when testing with mach run (thanks to aswan for helping me figure that out!) I'm marking the patch I uploaded here as obsolete, as I still have to get the mochitests passing anyway.

I'm running into test failures with the webextension version that I'm not sure how to handle: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8a54431f1152eeae7a2324eab6cf1f23f4c15c61&selectedJob=200230911 Basically, the webextension version is causing resource://gre/modules/WebNavigationFrames.jsm to be loaded early enough to trip browser_startup_content.js, which isn't expecting that module to be loaded early. But the webextension seems to be loaded after browser-delayed-startup-finished, since if I add a listener for browser-delayed-startup-finished, it never hears that event. florian, aswan suggested that I check with you on whether to just add webNavigationFrames.jsm to the whitelists in the tests, or if there's something else we should potentially be doing here?

Actually, nevermind. It turns out that using webNavigation in any way seems to cause a lot of very strange breakage, and so since I only needed webNavigation to detect when the page action needs to be updated, and page actions have an internal onLocationChanged API, I just stopped using webNavigation entirely in favor of that internal API (via an extension experiment, since I need a bunch of those anyhow). So far this approach seems to have fixed the weird failures in my try-run, so I'm optimistic that I'm almost done here.

Heh. Should have known better than to feel optimistic. Looks like there are still odd issues to work out: https://treeherder.mozilla.org/#/jobs?repo=try&revision=584c7dd86131e92987edb59f8dcfbabd108947ab

(In reply to Thomas Wisniewski [:twisniewski] from comment #10) > Basically, the webextension version is causing > resource://gre/modules/WebNavigationFrames.jsm to be loaded early enough to > trip browser_startup_content.js, which isn't expecting that module to be > loaded early. > > But the webextension seems to be loaded after > browser-delayed-startup-finished, since if I add a listener for > browser-delayed-startup-finished, it never hears that event. Note: browser_startup_content.js is testing the code loaded in new content processes. browser-delayed-startup-finished is a notification in the parent process, so you shouldn't expect it in content processes.

Thankfully it seems as though I've finally managed to figure out the test failures, as only known intermittents are showing up now in my try-runs: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3766b40b6cad574b71dba85bb6989e911e7c2a3

convert webcompat-reporter to a webextension

Full disclosure, we don't have plans to add support for https://developer.chrome.com/extensions/i18n l10n for Firefox, and instead would like to see Fluent support for web extensions. If that's becoming time-pressing, we could use some help on bug 1425104.

But Firefox *does* support that API (otherwise how is this webextension version of the system addon using it and getting localized strings?) Of course once we have a Fluent-based API I don't mind migrating the addon over to it. But for now, this appears to do the job in time for our bootstrapped addon deprecation date. Or was there something I should already be doing instead? I'm not against changing something to ease an eventual transition.

Right, we support the API, but that's not enough to get Firefox localized. To use this scheme in Firefox itself, we'd have to support it in the full stack, from l10n repacks, to the logic that extracts strings from m-c to cross-channel, we might need changes in pontoon, we don't have dashboard support. We'd also effectively loose all current translations. We do have code to migrate to Fluent, but extending that to other file formats is all but straight-forward.

Fair enough. Then I'll just leave the code as-is for now (pending review), and when the time comes and we have a better API, we'll convert it to use that instead.

(In reply to Thomas Wisniewski [:twisniewski] from comment #19) > Fair enough. Then I'll just leave the code as-is for now (pending review), > and when the time comes and we have a better API, we'll convert it to use > that instead. This means shipping untranslated content in Firefox, doesn't it? I'd be OK with this in Nightly, not so much in Beta (which, I think, was enabled recently).

Comment on attachment 9011293 [details] Bug 1451485 - convert webcompat-reporter to a webextension; r?aswan Andrew Swan [:aswan] has approved the revision.

(In reply to Francesco Lodolo [:flod] from comment #20) > (In reply to Thomas Wisniewski [:twisniewski] from comment #19) > > Fair enough. Then I'll just leave the code as-is for now (pending review), > > and when the time comes and we have a better API, we'll convert it to use > > that instead. > > This means shipping untranslated content in Firefox, doesn't it? I'd be OK > with this in Nightly, not so much in Beta (which, I think, was enabled > recently). Erm yeah. We don't want to send English strings in Beta. What should we be doing to prevent that (are we blocked on some web extension API)?

Yes, we're basically blocked by supporting Fluent in WebExtension (comment 16 and 18, bug 1425104).

flod, how are formautofill and screenshots working around this (assuming they are?)

Screenshots is not localized in mozilla-central, and code+translations are periodically dumped into m-c. I believe they convert from .properties (the format used for localization) to JSON, using a npm package (pontoon-to-webext) https://searchfox.org/mozilla-central/source/browser/extensions/screenshots/webextension/_locales https://github.com/mozilla-services/screenshots/blob/010867ebe909788bd5a3cf2b5bb2915777404b8b/Makefile#L125 I really don't like the idea to use this approach for more projects, for one because it doesn't scale. Form Autofill is localized in mozilla-central (.properties), but I don't know exactly what they're doing. Seems to be a WebExtension since bug 1449055, with recent issues in packaging (bug 1484325).

Thanks, it looks like Bug 1425104 is getting pretty close to done. We should still be able to hit our 64 target.

I'm not confident that bug 1425104 is going to get done for 64, can we continue to use .properties for now (since the whole l10n pipeline can support it) and add another experiment api to shim those strings up to the webextension for now?

Is this a question for Flod? Or Tom?

For Tom. He and I just chatted on IRC and I think we have a plan...

Alright, I think I'm on track to have a patch update on Monday (I'm away tomorrow). As I discussed with aswan, I'm using a similar approach to formautofill to load the chrome.manifest that the bootstrapped version of this addon used, and have an experimetal API read strings from it using Services.strings.createBundle/GetStringFromName. A proof-of-concept is already working, and now I just have to hook the strings up properly.

Thank you. Feel free to request review on the l10n build bits, if there's still gonna be some. It'd be great if we could use the existing .properties files as is, where they are in the tree now. Then the build bits would also be zero, I guess?

Yes, my plan is to just keep the build bits working as they were for the bootstrapped addon. That is, the webcompat.properties file will continue to be used in the same way. I'll see if I can't get a candidate patch up here before I leave for the day.

Alright, I've updated the patchset accordingly: https://phabricator.services.mozilla.com/D6575?vs=19171&id=19595&whitespace=ignore-most#toc It seems to be working fine for me in a local build, but I haven't yet verified if there's any test breakage (I'm doing a try-run now: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6b21cb9a85c5d1b98aff1ba88b4f0b32754909b9)

I've addressed the latest review feedback: https://phabricator.services.mozilla.com/D6575?vs=19595&id=19891&whitespace=ignore-most#toc Let me know if there are any other concerns.

I'm doing a fresh try-run here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c96f8096e2e99de60b858b2e6c8b85a70962122 Also a Talos run: https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=7c96f8096e2e99de60b858b2e6c8b85a70962122&framework=1&showOnlyImportant=1&showOnlyConfident=1&selectedTimeRange=172800

Andrew, I'm not sure what to do about the Talos regressions. As we discussed on IRC they seem similar to what Screenshots is getting in their own webextension porting effort, and I'm not familiar enough with the issues I see in bug 1491997 to know where to go from here. I'll follow that bug just in case there's something I can do to help, but beyond that I think we're just blocked on those issues being fixed (or a decision being made to soak up the regressions for now).

:pike, could you give this a review ASAP? I didn't realize your go-ahead was still needed on Phabricator: https://phabricator.services.mozilla.com/D6575 (In the end I did what I suggested in comment 30, so I'm not sure what there will be for you to actually review).

Um, not sure how I resolved this as fixed... re-opening.

I just started a couple of try-runs to hopefully help measure the Talos regressions; With this patch applied: https://treeherder.mozilla.org/#/jobs?repo=try&revision=be99436c021f5dd043a9624df3801e65de59fd23 Without this patch applied: https://treeherder.mozilla.org/#/jobs?repo=try&revision=99668a2d21348d5064727f2f1d03800a2ee3d91f

The try push helps me, too. Commented in phabricator.

Thanks! Here's hoping the try-runs give us some valuable insight.

Hm, the profiles for this job is an empty zip file :/ https://treeherder.mozilla.org/#/jobs?repo=try&revision=be99436c021f5dd043a9624df3801e65de59fd23&selectedJob=204289740

Darn. I retriggered that job in the hopes that the second time would be a charm, but it too spit out a useless 22-byte zip file :S If you think it's worth trying a completely fresh try-run in the hopes for a better result, let me know and I'll send it off.

I tried comparing the windows sessionrestore profiles and got myself thoroughly confused. I started with the script here: https://github.com/devtools-html/perf.html/issues/470#issuecomment-404571370 and tried to hack it up a bit to work for this test (since the script relies on user timing events which the talos sessionrestore test doesn't generate). I tried creating a diff of the profiler samples between the sessionRestoreInit and sessionRestored events but then I noticed that in the profiles those events are ~600 ms apart but the talos figures for this test are ~300 ms. And sure enough, talos uses some logic that I don't really understand to compute the time: https://searchfox.org/mozilla-central/source/testing/talos/talos/startup_test/sessionrestore/addon/bootstrap.js#87-90 From a quick glance, StartupPerformance.latestRestoredTimestamp is derived from SSTabRestored events, which aren't visible in the profiles. Argh. Mike, you have any advice to offer? The short background story is that there's a (pretty small) sessionrestore regression with webcompat-reporter running as a webextension (see comment 35). Tom gathered some profiles (comment 39) and I wanted to compare them to identify the cause of the regression, which led to the confusion described at the start of this commment.

aswan has spent the day looking at Talos reports related to this, and has decided that it seems likely that the only regressions of note are related to bug 1498027. As he told me on IRC, the regressions are very small and should go away as that bug is fixed, so he's okay with us landing this patch.

And for posterity, here's a link to a Talos comparison for the regression being investigated: https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=cac770e62f9263675e2f5676207a6e28838be6ff&framework=1&filter=sessionrestore&selectedTimeRange=172800

Alright, since there doesn't seem to be any blocking issues left, I just did a trivial rebase and one last try-run which confirms that things are still green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=16f7b0a775607933527db6314e33f6b039b9c124 Requesting check-in.

Pushed by rvandermeulen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/90b00e89e83e convert webcompat-reporter to a webextension; r=aswan,Pike