1526236 - Priveleged extensions must use new style imports as of Nightly 67 (2019-02-07)

Status: VERIFIED FIXED

(Shield :: Shield Study, defect)

Description

[Ciprian Muresan [:cmuresan], Ecosystem QA]

Attachment: image.png

[Affected versions]:

• Firefox Nightly 67.0a1, Build ID 20190207214423

[Affected Platforms]:

• All Windows
• All Mac
• All Linux

[Prerequisites]:

• Create the "xpinstall.signatures.dev-root" boolean pref and set it to "true"
• Download the extension from https://bugzilla.mozilla.org/show_bug.cgi?id=1522598#c15

[Steps to reproduce]:

1. Start the browser and navigate to about:addons.
2. Click the Extensions tab.
3. Install the extension from prerequisites.
4. Observe the behavior.
5. Click the wrench and screwdriver icon from the Browser Toolbar and observe the behavior.

[Expected results]:

• Step 4: A new tab is opened with the extension's onboarding page.
• Step 5: A doorhanger is displayed.

[Actual results]:

• Step 4 & Step 5: Nothing happens.

[Notes]:

• The issue is also reproducible with the extension from https://bugzilla.mozilla.org/show_bug.cgi?id=1522525#c29
• Attached a screenshot of the Browser Console after installing the addon

[Regression range]:
8:51.98 INFO: No more inbound revisions, bisection finished.
8:51.98 INFO: Last good revision: d1b523e67100557e4adceabe8ca6ea434f6cb5e7
8:51.99 INFO: First bad revision: 6b56696d713a7f7858f16235e37baa8307e73b49
8:51.99 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d1b523e67100557e4adceabe8ca6ea434f6cb5e7&tochange=6b56696d713a7f7858f16235e37baa8307e73b49

Comment 1

[Ciprian Muresan [:cmuresan], Ecosystem QA]

From the pushlog it looks like bug 1514594 or bug 1519596 has caused this.

@Kris, can you please take a look at this? The two extensions in question are addons that are used in Shield studies.

Comment 2

[Mark Banner (:standard8)]

The shield study has "privileged" components which are allowed access into Chrome, and hence is more susceptible to changes in core than normal WebExtensions.

I just looked at privileged/study/api.js from the extension in bug 1522525 comment 29.

I found these two lines:

ChromeUtils.import("resource://gre/modules/ExtensionCommon.jsm");
ChromeUtils.import("resource://gre/modules/ExtensionUtils.jsm");

They won't work any more post bug 1514594. They need to be of the style:

let {something} = ChromeUtils.import("resource://gre/modules/ExtensionCommon.jsm");
let {something2} = ChromeUtils.import("resource://gre/modules/ExtensionUtils.jsm");

There may be more cases, but they were just two I saw, so I think this is more likely an issue with the Sheild Study rather than the WebExtension core.

Comment 3

[Mark Banner (:standard8)]

Adding needinfo for Michael to check he sees this.

Comment 4

[Pascal Chevrel:pascalc (PTO until April 26)]

Tracking for 67 as it may affect the results of multiple Shield studies.

Comment 5

[Michael Cooper [:mythmon]]

Thanks for the heads up Pascal. I'll keep an eye out for other issues like this in add-on studies that are launching soon.

Comment 6

[Neha Kochar [:neha]]

Michael, since this is tracking 67, can you add an assignee who can look into it per Mark's suggestions in comment 2?

Comment 7

[Michael Cooper [:mythmon]]

This isn't a bug in Firefox, it is a bug in the shield study extensions. I'll make sure that all the extensions in flight that are targeting 67 make the needed changes here.

:standard8, do you know if the new style syntax work pre-67 as well? If it does, that will make extension compatibility a lot easier. If not, do you have another idea we could use to make extensions compatible with both ways?

Comment 8

[Mark Banner (:standard8)]

(In reply to Michael Cooper [:mythmon] from comment #7)

:standard8, do you know if the new style syntax work pre-67 as well? If it does, that will make extension compatibility a lot easier. If not, do you have another idea we could use to make extensions compatible with both ways?

I think you might be able to do:

let {something} = ChromeUtils.import("resource://gre/modules/ExtensionCommon.jsm", null);

That would work in at least recentish version prior to 67.

Comment 9

[Pascal Chevrel:pascalc (PTO until April 26)]

Michael, could you give us a status update on the affected shield study extensions? Have they all been audited and updated yet? Thanks

Comment 10

[Michael Cooper [:mythmon]]

Since all study extensions go through QA and Experimenter, I've made a note to check during the Experimenter ingress process that all studies targeting 67 have been QAed specifically on 67. Given this, combined with the usual testing process, I don't think there is anything else to do in this bug.

Comment 11

[Neha Kochar [:neha]]

Marking as fixed per comment 10.

Comment 12

[Carmen Tataru [:ctataru], Ecosystem QA]

Marking as Verified as the addon is no longer available.