Replace the WebExtension browser action with a Photon page action. #3239
Conversation
This removes the browser action and adds a Photon page action, pursuant to https://bugzilla.mozilla.org/show_bug.cgi?id=1366041. Right now Photon page actions are unrelated to WebExtension page actions unfortunately, which means that this patch has to do most of its work in bootstrap.js. The WebExtension part passes messages to bootstrap.js to handle clicks and update the action's title and icon as necessary.
…action. Fix the test for the previous commit.
|
Oh, of course it can't find the node: my changes in https://bugzilla.mozilla.org/show_bug.cgi?id=1366041 haven't landed in Firefox yet. All right. For future reference, what version of Firefox is the CI using? |
|
@0c0w3: The CI tests should fetch the latest Nightly each time. Our codebase has to support back to Firefox 55, which I think means we need to detect this feature and handle both cases appropriately. I'm not sure exactly how that should work though. |
|
OK. It should be pretty easy to detect whether PageActions is available and fall back to a browser action if it's not I think. |
|
I need to be able to create a browserAction on 56 and earlier and a Photon page action on 57 and later. aswan says there's no way to create a browserAction programmatically. He suggests using CustomizableUI directly instead, which is how browser actions end up in the UI. So I'll have to do yet more stuff in bootstrap.js instead of in the actual WebExtension. :-( I'll post a patch tomorrow. |
…ction when not. Extend the Photon-related port used between bootstrap.js and the WebExtension so that bootstrap.js can tell the WebExtension whether it's OK to use the Photon page action. The WebExtension should not attempt to use the browser action when Photon is enabled because bootstrap.js will have removed the browser action's navbar button. Modify the test so that it checks the Photon page action when Photon is enabled and the browser action when it's not.
|
Here's an explanation of how this works: aswan says that it's not possible to dynamically create a browser action. He suggested removing the browser action entirely and replacing it with calls directly to CustomizableUI from bootstrap.js, to add back the toolbar button. I took a look at that, but I really don't want to have to replicate all the things the WebExtensions implementation does regarding the toolbar button. So what I do instead is to leave the browser action alone, and then if bootstrap.js sees that Photon is enabled, I call into CustomizableUI directly to destroy the toolbar button. So the button is gone from both the toolbar and the customization palette. I was hoping to leave all the browser action code in the actual WebExtension completely untouched. But unfortunately Firefox's WebExtensions implementation doesn't expect the widget to be gone (reasonable), and when main.js/startBackground.js tries to set the icon, Firefox throws an exception. So I had to make bootstrap.js specifically tell the WebExtension that it's OK to use the Photon page action and to not use the browser action, and then add branches everywhere for the two cases. Sorry for adding all this gunk to your add-on... :-/ |
|
@0c0w3 This looks great. I don't mind the clutter, it'll be gone soon enough. |
jaredhirsch
merged commit 4396250
into
mozilla-services:master
This removes the browser action and adds a Photon page action,
pursuant to https://bugzilla.mozilla.org/show_bug.cgi?id=1366041.
Right now Photon page actions are unrelated to WebExtension page
actions unfortunately, which means that this patch has to do most
of its work in bootstrap.js. The WebExtension part passes
messages to bootstrap.js to handle clicks and update the action's
title and icon as necessary.