Closed
Bug 1348394
Opened 7 years ago
Closed 7 years ago
Rewrite WebPageTest's Firefox add-on as a WebExtension
Categories
(Firefox :: Extension Compatibility, enhancement)
Firefox
Extension Compatibility
Tracking
()
RESOLVED
FIXED
People
(Reporter: cpeterson, Assigned: alexical)
References
()
Details
(Whiteboard: [fce-active-legacy])
WebPageTest uses a Firefox add-on to control its tests, but the add-on is neither e10s nor WebExtension compatible. Firefox 57 will drop support for legacy add-ons and only support WebExtensions. WebPageTest has a Chrome extension, but we can't easily port it to a Firefox WebExtension because it relies on some Chrome APIs that Firefox doesn't support yet, such as: * chrome.management.getAll (Firefox bug 1282981) * chrome.management.launchApp * contentSettings * debugger * some differences in tabs.create The WebPageTest developers say the main needed features are: * Navigate a tab * Get onload and error notifications for the navigation * Run JS as if it was running from the dev console As gravy, getting the request/response timings, headers and bodies and being able to do it all remotely would eliminate the need for an extension at all. https://github.com/WPO-Foundation/webpagetest/issues/811
Reporter | ||
Comment 2•7 years ago
|
||
Moved to Firefox::General. Thanks.
Component: Networking → General
Flags: needinfo?(cpeterson)
Product: Core → Firefox
Comment 3•7 years ago
|
||
Andy, do we have bugs on file for these APIs to be added to webextensions?
Component: General → WebExtensions: General
Flags: needinfo?(amckay)
Product: Firefox → Toolkit
Comment 4•7 years ago
|
||
(In reply to Chris Peterson [:cpeterson] from comment #0) > * chrome.management.getAll (Firefox bug 1282981) > * chrome.management.launchApp These won't be ported. We'll never support Chrome apps, and Chrome is removing support too, so if they're relying on this, they'll need to stop relying on them anyway. I'd be interested to know what they need them for. > * contentSettings Bug 1291841 > * debugger Bug 1316741 > * some differences in tabs.create What differences, exactly? > * Navigate a tab This should already be possible. > * Get onload and error notifications for the navigation Likewise. > * Run JS as if it was running from the dev console Can you be more specific? They can execute JS in a page using tabs.executeScript. What more do they need, exactly?
Comment 5•7 years ago
|
||
> What differences, exactly? Using selected instead of active, it's negligible I think. > Can you be more specific? They can execute JS in a page using tabs.executeScript. What more do they need, exactly? I don't think they need anything else, this is what they're doing in the old Firefox version: https://github.com/WPO-Foundation/webpagetest/blob/master/agent/browser/firefox/extension/chrome/content/mozutil.js#L123 My recommendation when asked about this was to just rewrite the Firefox version as a WebExtension, given the time constraints and the status of chrome.debugger support in Firefox. I did not know that we were moving to Chrome's debugging protocol, but that doesn't seem like it will be ready before 57.
Updated•7 years ago
|
Flags: needinfo?(amckay)
Updated•7 years ago
|
Assignee: nobody → dothayer
Whiteboard: [fce-active]
Assignee | ||
Comment 6•7 years ago
|
||
I think this is done: https://github.com/squarewave/webpagetest I was planning on shooting a PR up but I wanted to check in here first. Is there anything special I should be doing to test this? What I've done so far is: - Run all the tests in allTests.js - Run all the fake commands in firefox/extension/chrome/content/fakeCommandSource.js - Stand up a local instance of WebpageTest and run it against https://facebook.com with the WebExtension in the profile, then compare the results to the same test with the old extension
Flags: needinfo?(jhofmann)
Comment 7•7 years ago
|
||
Oh, wow, awesome! (In reply to Doug Thayer [:dthayer] from comment #6) > I was planning on shooting a PR up but I wanted to check in here first. Is > there anything special I should be doing to test this? What I've done so far > is: > > - Run all the tests in allTests.js > - Run all the fake commands in > firefox/extension/chrome/content/fakeCommandSource.js > - Stand up a local instance of WebpageTest and run it against > https://facebook.com with the WebExtension in the profile, then compare the > results to the same test with the old extension I don't know a lot about wpt myself, but your steps sound solid, so you should probably make a PR and repeat that question there. It seems like moving things over from Chrome was easier than I expected. It's a bit hard to diff this against the Chrome source, but what I can gather is that you simply removed the debugger code and modernized a bit of code e.g. introduced class syntax. Considering that, would you think that it's viable to consolidate the two code bases by hiding the Chrome specific parts (mostly the debugger from what I can tell) behind #ifdef ?
Flags: needinfo?(jhofmann) → needinfo?(dothayer)
Assignee | ||
Comment 8•7 years ago
|
||
Yeah, mostly that's all I did. Removing the debugger code however did involve adding a bit of code to replace what we needed. I think the extent of that was replacing the eval in the debugger with window.postMessage to the underlying page for the page to eval the code (to avoid exposing WebExtension APIs which would be available with tabs.executeScript.) While exposing those APIs to arbitrary users of webpagetest.org probably wouldn't be the end of the world, it's still probably a good idea. Also, we need this logic anyway to access the page's window.performance object which seems to differ between the page's context and the tabs.executeScript context. I added that question to the PR though to see what the maintainer thinks (https://github.com/WPO-Foundation/webpagetest/pull/855).
Flags: needinfo?(dothayer)
Reporter | ||
Comment 9•7 years ago
|
||
pmeenan merged Doug's PR and says he pushed wptdriver 371 to use the new extension. Let's leave this bug open until someone at Mozilla confirms that WPT is using the new extension and it works correctly.
Updated•7 years ago
|
Component: WebExtensions: General → Extension Compatibility
Product: Toolkit → Firefox
Reporter | ||
Comment 10•7 years ago
|
||
The new webpagetest extension seems to work for people, so we can close this bug now. Thanks again, Doug!
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Comment 11•7 years ago
|
||
:heart:
Updated•6 years ago
|
Whiteboard: [fce-active] → [fce-active-legacy]
You need to log in
before you can comment on or make changes to this bug.
Description
•