1524327 - reconsider MOZ_ALLOW_LEGACY_EXTENSIONS behavior

Status: RESOLVED FIXED

(WebExtensions :: General, enhancement, P5)

Description

[Shane Caraveo (:mixedpuppy)]

At a minimum, we could rename that to ALLOW_EXTENSION_EXPERIMENTS or something, but we have both isPrivileged and experimentsAllowed looking at appconstant and/or a pref. It's not clear why that is being done when looking at the code.

Comment 1

[David Durst [:ddurst]]

Minimum approved!

Comment 2

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1524327 remove MOZ_ALLOW_LEGACY_EXTENSIONS and extensions.legacy.enabled

Legacy extensions are no longer loaded, so we can drop the build config for it. We
still need flags for handling experimental APIs since what we require differs between builds
and distributions.

Comment 3

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Geoff, do we need any of this for Thunderbird or are we good with the way we use WebExtensions now?

Comment 4

[Geoff Lankow (:darktrojan)]

I think this is fine but I need to look more closely. Leaving the NI? until I can do that.

This is a problem though:

  get addonExperimentsAllowed() {
    return this.NIGHTLY_BUILD || this.MOZ_DEV_EDITION;
  },

Comment 5

[Shane Caraveo (:mixedpuppy)]

Geoff, I'll land this patch in the next day or two. In addition to the above, any build that has MOZ_REQUIRE_SIGNING set to false will be able to set the preference. Based on what is in mozilla-central, this patch essentially does not change what builds can set the expiremental preference.

Comment 6

[Geoff Lankow (:darktrojan)]

Right. That's fine. I'd done nothing here because of a lack of Bugzilla action … but of course everything's happened on Phabricator and I forgot to subscribe to that. :-/

Comment 7

[Geoff Lankow (:darktrojan)]

This hasn't gone so well though: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ba22d3581ecc4563acabf714809b71711c9cd194&group_state=expanded

xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_upgrade.js | xpcshell return code: 0
xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_upgrade.js | test_1 - [test_1 : 111] Extension 1 is active - false == true
xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_onPropertyChanged_appDisabled.js | xpcshell return code: 0
xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_onPropertyChanged_appDisabled.js | run_test - [run_test : 29] false == true
xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_strictcompatibility.js | xpcshell return code: 0
xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_strictcompatibility.js | test_1 - [test_1 : 113] Expected value of addon.appDisabled - true deepEqual false

Comment 8

[Shane Caraveo (:mixedpuppy)]

Geoff, the latest patch addresses those failures. I'll have to deal with them in more detail in the followup bug 1608552.

Comment 9

[Shane Caraveo (:mixedpuppy)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d4a6b848609b231d3fccf73921b4e5026b2140a

Comment 10

[Tomislav Jovanovic :zombie]

We should announce this far and wide.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/85753283ef2e
remove MOZ_ALLOW_LEGACY_EXTENSIONS and extensions.legacy.enabled r=zombie,aswan

Comment 12

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/85753283ef2e

Comment 13

[Magnus Melin [:mkmelin]]

There's one reference of MOZ_ALLOW_LEGACY_EXTENSIONS left in toolkit/mozapps/extensions/test/xpcshell/test_temporary.js

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/dba02c1a5a98
remove Thunderbird reference to MOZ_ALLOW_LEGACY_EXTENSIONS define (which was removed in bug 1524327). r=bustage-fix

Comment 15

[Miruna Curtean]

Hello,

If this ticket requires manual testing please provide the steps to reproduce/test. Otherwise, the "qe verify -" flag should be set.
Thank you

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/c1457ba6362c
Remove SeaMonkey reference to MOZ_ALLOW_LEGACY_EXTENSIONS define (which was removed in bug 1524327). r=bustage-fix (DONTBUILD)

Comment 17

[Alex Vallat]

Part of this fix adds a line to firefox.js setting pref("extensions.experiments.enabled", false);

Is that intentionally part of this bug? Experiments are not discussed explicitly as part of it anywhere that I see, and it has had the effect of disabling experiments where they were previously enabled.