Support extensions.%NAME%.userdisabled

[ghost]

%NAME% is simply whatever we decide to name this thing. Right now it's "pageshot".

#2332 is our high-level disable flag. No one should be messing with that flag (random power-user aside) -- it's entirely there to enable us to roll this out slowly with Shield.

This issue is about implementing the code that allows the user to disable Page Shot. Some details:

• I'm suggesting using the flag extensions.pageshot.userdisabled but pick a different one if you like
• The code should look for this flag pretty early on startup (could be right after the code in Support extensions.%NAME%.disabled #2332)
• This code actually needs some UI in about:preferences. @johngruen where should this go and what should it look like?

[jaredhirsch]

I'm not sure about preferences UI for embedded webextensions. This also depends on whether we want users to see page shot in about:addons or not (I don't recall what the decision was there).

@andymckay, if we want to allow users to disable page shot (embedded webextension system addon), should we implement that UI using the webextension approach or the bootstrapped addon approach?

[andymckay]

Let's try to avoid writing more XUL and focus on the webExtension approach please.

Note that chrome_style for the settings page is landing in 55, sadly a release too late.

https://bugzilla.mozilla.org/show_bug.cgi?id=1275287

[jaredhirsch]

@andymckay sounds good to me! thanks :-)

[ianb]

If we want to disable the WebExtension entirely when the pref is off (by not calling .startup()), then I don't think we can use the WebExtension approach, can we? Also since the add-on won't appear in about:addons, there's no place for the preferences button.

[jaredhirsch]

TL;DR: yes, you can shutdown and restart embedded webextensions, but I'm not sure it's safe to do so

It turns out that embedded webextensions have an undocumented shutdown method. Here's how you shut down the webextension without shutting down the outer extension (we could do this in a pref listener):

const { EmbeddedExtensionManager } = Cu.import("resource://gre/modules/LegacyExtensionsUtils.jsm");
const webext = EmbeddedExtensionManager.getEmbeddedExtensionFor({id: 'pageshot@mozilla.org'});
webext.shutdown();

One problem here is that there's a bit of state that tracks whether the webextension has ever been started. To disable, then re-enable the webextension, we have to manually unset that state. This might be okay, if the addons team thinks these APIs are stable:

// re-enabling a shutdown embedded webextension:

// manually unset the 'started' state :-\
webext.started = false;
// retrigger startup
webext.startup();

I've tried toggling the webextension off and back on using these code snippets, and the toolbar button does appear and disappear, and the re-enabled webextension seems to take shots just fine. So, it seems ok in 54 today (currently dev edition).

The big question is whether we can trust this code to work in the future. I'll follow up with the addons team to figure this out.

[jaredhirsch]

According to kmaglione, we should create a new instance, rather than restart a webextension that's been shutdown.

This is the snippet that should work, but I need to make sure the jar:file: URI creation bit is done properly:

/* how to safely shutdown the webext */
const { EmbeddedExtensionManager } = Cu.import("resource://gre/modules/LegacyExtensionsUtils.jsm");
const webext = EmbeddedExtensionManager.getEmbeddedExtensionFor({id: 'pageshot@mozilla.org'});
webext.shutdown();
// unlink the old webextension
EmbeddedExtensionManager.untrackEmbeddedExtension(webext);

/* how to safely lazily create a new webextension */
let pageshotAddon;
AddonManager.getAddonById('pageshot@mozilla.org', addon => { pageshotAddon = addon; });
// I dunno about this line, jarfile stuff is outside my comfort zone
const funkyURI = 'jar:' + pageshotAddon.getResourceURI().asciiSpec + '!/';
const resourceURI = Services.io.newURI(funkyURI);
// because the old one was unlinked, this get call spawns a new one,
// but fails if the resourceURI isn't passed in
const newWebext = EmbeddedExtensionManager.getEmbeddedExtensionFor({
  id: 'pageshot@mozilla.org',
  resourceURI: resourceURI
});
newWebext.startup(); // works

[jaredhirsch]

My approach to building the jar URI seems to be correct, or at least, that same approach is used in some Firefox tests.

[jaredhirsch]

I think self.id could be used in place of the addon's name, which would let this skate around any impending mass name changes.

[jaredhirsch]

Hitting some super weird errors. It's surprisingly quite difficult to get the embedding bootstrapped extension to load properly; ./bin/run-addon --bootstrap does not seem to consistently work. I cannot figure out why.

I've tried doing jpm xpi, then loading the addon into a running Nightly profile, but jpm xpi outputs a file named null.xpi, so I suspect the generated xpi is not working quite right.

Anyhow, the code as written seems correct, but there are two recurring, intermittent bugs:

• after setting userdisabled to true, then restarting, the toolbar button reappears
• after setting userdisabled to true, then to false, the toolbar button reappears, but clicking it does nothing

When these two bugs occur, the profile seems to be in an odd state: I can't find bootstrap.js in the debugger (hence, can't debug the error). I don't know if this points to caching problems in the debugger, or some other weirdness. jpm xpi spitting out 'null.xpi' probably indicates I'm not building the addon correctly.

[johngruen]

@6a68 Pref should go at the bottom of about:preferences#content