In bug 1359203 we will add a new preference that controls whether legacy extensions can be enabled. Beginning in 57, this preference should only be honored on Nightly and on unbranded builds. To do this, we need a build-time flag in AppConstants to indicate whether the preference is valid (suggested name MOZ_ALLOW_LEGACY_EXTENSIONS). It should end up looking a lot like the results of bug 1186522.

Not sure who to ask Kim, but you helped us out last time. Know where we should route this now?

I have too many other projects on my plate now, I'll ask in #releng if someone else has time to time to look at it.

Okay, bug 1359203 actually covered the build system part of this (adding a configure-time option for whether to allow legacy extensions). But we still need the releng work to set that flag properly in beta and release builds beginning with the 57 release. Re-targeting this bug to that effort.

(In reply to Andrew Swan [:aswan] from comment #3) > Okay, bug 1359203 actually covered the build system part of this (adding a > configure-time option for whether to allow legacy extensions). But we still > need the releng work to set that flag properly in beta and release builds > beginning with the 57 release. Re-targeting this bug to that effort. Tagging :kmoir. Maybe this is something buildduty could handle now.

Does "Beta" include Developer Edition repack?

Andrei, is this something you can look at?

Sure Kim,I'll check to see what I can do here

Andrei, you asked this morning for hints on this bug In this bug https://bugzilla.mozilla.org/show_bug.cgi?id=1359203 and this patch https://reviewboard.mozilla.org/r/139586/diff/3#index_header you can see there is a new flag enabled called MOZ_ALLOW_LEGACY_EXTENSIONS there are mozconfigs for different builds example for win64 here mozilla-inbound/browser/config/mozconfigs/win64 we need to implement patches in tree to only enable this value for nightly and unbranded builds beginning in 57. You can create patches, test on try and verify that this is true (try to install an old style extension). Let me know if you have questions or need additional help!

I think I may have misunderstood project_flag() when I wrote the patch referenced in the previous comment, and we should probably use option() instead of project_flag(). I assume that can happen as part of patches for this bug...

Mihai, is this the right direction?I'm not sure if I should define new flag or use the one that was created: MOZ_ALLOW_LEGACY_EXTENSIONS.

Andrei, see comment 9 -- I don't think your changes have any effect right now since bug 1359203 didn't actually create a configure-time settable switch. A build peer (eg :glandium) should be able to help with the right way to set this up.

(In reply to Andrew Swan [:aswan] from comment #11) > Andrei, see comment 9 -- I don't think your changes have any effect right > now since bug 1359203 didn't actually create a configure-time settable > switch. A build peer (eg :glandium) should be able to help with the right > way to set this up. (In reply to Andrei Obreja [:aobreja][:buildduty] from comment #10) > Created attachment 8884832 [details] [diff] [review] > bug1363897_v1.patch > > Mihai, is this the right direction?I'm not sure if I should define new flag > or use the one that was created: MOZ_ALLOW_LEGACY_EXTENSIONS. Glanced at these patches but it's build stuff way out of my knowledge. I'll redirect to :glandium. @glandium - can you please give us some advice as to how to tackle this here? Thanks!

:catlee - this just occurred to me, I just realized this might be needed before merging trains on the 2nd of August. Just to double-check, is this something high-priority to be done by 2 August or something we can work on while 57 stays in nightly train?

(In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #13) > :catlee - this just occurred to me, I just realized this might be needed > before merging trains on the 2nd of August. Just to double-check, is this > something high-priority to be done by 2 August or something we can work on > while 57 stays in nightly train? Per comment #3, I think this is something we can work on once 57 is in Nightly. I don't think we need to do anything for the 56 beta/release cycle.

(In reply to Chris AtLee [:catlee] from comment #14) > (In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #13) > > :catlee - this just occurred to me, I just realized this might be needed > > before merging trains on the 2nd of August. Just to double-check, is this > > something high-priority to be done by 2 August or something we can work on > > while 57 stays in nightly train? > > Per comment #3, I think this is something we can work on once 57 is in > Nightly. I don't think we need to do anything for the 56 beta/release cycle. Just to confirm, is this correct, can the work of this bug be completed *while* we have 57 in nightly?

Yes, it can be done during the 57 cycle. We would very much like to have it early in the cycle though so that simulated beta build/test runs start getting us coverage of automated tests with legacy extensions disabled.

(In reply to Andrew Swan [:aswan] from comment #16) > Yes, it can be done during the 57 cycle. We would very much like to have it > early in the cycle though so that simulated beta build/test runs start > getting us coverage of automated tests with legacy extensions disabled. Sounds good. I'll bump it up in priority to be done early in the 56 beta cycle. Thanks for confirming this!

(In reply to Andrew Swan [:aswan] from comment #0) > In bug 1359203 we will add a new preference that controls whether legacy > extensions can be enabled. Beginning in 57, this preference should only be > honored on Nightly and on unbranded builds. What do you mean "unbranded builds"?

(In reply to Mike Hommey [:glandium] from comment #18) > What do you mean "unbranded builds"? We provide builds of release and beta that don't use the official Firefox branding but that do honor the preference that controls addon signing (ie so that you can turn it off if you wish). We do a rather poor job of providing access to these builds but: https://wiki.mozilla.org/Add-ons/Extension_Signing#Unbranded_Builds I'm not certain but I think these builds are created with the "add-on-devel" configs in eg browser/config/mozconfigs/$platform/ The goal of this bug is to make a configure-time flag that controls whether legacy extensions can be enabled by flipping a pref. My understanding is that releng will then take care of making sure that that flag is set properly in each relevant config file and that everything is handled properly on merge days, but could use some help with actually exposing the configure-time option. Back in bug 1359203 I added this: http://searchfox.org/mozilla-central/rev/8a61c71153a79cda2e1ae7d477564347c607cc5f/toolkit/moz.configure#501-503 But I didn't understand the project_flag creates a fixed value. What do we do to make this something that can be overridden from mozconfig? Just switch from project_flag() to option() ?

> Just switch from project_flag() to option() ? That seems sensible. And probably change the default to milestone.is_nightly.

Unless I botched something, the attached patch should make it possible to enable/disable legacy extensions with an addition to mozconfig like: ``` ac_add_options --disable-legacy-extensions ``` or ``` ac_add_options --enable-legacy-extensions ``` It should automatically default to allowing on Nightly and disallowing elsewhere, but we'll still need overrides for dev edition and unbranded builds. Setting leave-open so we can get both parts in as part of this bug.

Comment on attachment 8898408 [details] Bug 1363897 Switch legacy extensions handling to option() https://reviewboard.mozilla.org/r/169808/#review175256 ::: toolkit/moz.configure:509 (Diff revision 1) > > project_flag('MOZ_ANDROID_HISTORY', > help='Enable Android History instead of Places', > set_as_define=True) > > -project_flag('MOZ_ALLOW_LEGACY_EXTENSIONS', > +option('--enable-legacy-extensions', Any particular reason to change to a flag, rather than keep a variable (i.e. option(env='MOZ_ALLOW_LEGACY_EXTENSIONS'... ) Also, you can just set default=milestone.is_nightly

Comment on attachment 8898408 [details] Bug 1363897 Switch legacy extensions handling to option() https://reviewboard.mozilla.org/r/169808/#review175256 > Any particular reason to change to a flag, rather than keep a variable (i.e. option(env='MOZ_ALLOW_LEGACY_EXTENSIONS'... ) > > Also, you can just set default=milestone.is_nightly As explained in the original bug, we (intend to) allow legacy extensions in dev edition and in unbranded builds, which I think requires a flag?

Comment on attachment 8898408 [details] Bug 1363897 Switch legacy extensions handling to option() https://reviewboard.mozilla.org/r/169808/#review175288 An env variable will do.

Comment on attachment 8898408 [details] Bug 1363897 Switch legacy extensions handling to option() https://reviewboard.mozilla.org/r/169808/#review175288 That would have been a helpful thing to say back at https://bugzilla.mozilla.org/show_bug.cgi?id=1363897#c20

(In reply to Andrew Swan [:aswan] from comment #27) > Comment on attachment 8898408 [details] > Bug 1363897 Switch legacy extensions handling to option() > > https://reviewboard.mozilla.org/r/169808/#review175288 > > That would have been a helpful thing to say back at > https://bugzilla.mozilla.org/show_bug.cgi?id=1363897#c20 In that context, I was assuming that's what you meant: project_flag('MOZ_...') becoming option(env='MOZ_...').

I think I misunderstood comment 26, sorry for the frustrated outburst. New patch coming in a bit.

I'm not sure what I'm doind gwrong, using this fragment in moz.configure: ``` option(env='MOZ_ALLOW_LEGACY_EXTENSIONS', default=milestone.is_nightly, help='Allow legacy browser extensions') ``` and this in .mozconfig: ``` MOZ_ALLOW_LEGACY_EXTENSIONS=0 ``` I get this error from "mach configure": ``` 0:02.93 mozbuild.configure.options.InvalidOptionError: MOZ_INSTRUMENT_EVENT_LOOP takes 0 values ``` Help?

Set it to nothing.

Comment on attachment 8898408 [details] Bug 1363897 Switch legacy extensions handling to option() https://reviewboard.mozilla.org/r/169808/#review176678

Pushed by aswan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/def77bfc578f Switch legacy extensions handling to option() r=glandium

Thank you Mike and Andrew for updating the bug and come with fix patches. I wish to ask you both if there is something else left for us buildduty to do,I think that in 20 september the release 57 will reach Beta so we may want to finish until then.Please let us know if we can help here with anything else.

Like I said in comment 22, we'll need overrides for dev edition and unbranded configs. I set leave-open on this bug so that gets done before resolving, or if you prefer, that can happen in a separate bug.

I am photosensitive, and need more than 20 animation-blocking tools, including 8 animation-blocking extensions, to be able to use the internet. I am stuck in an arms race with advertisers and web designers. Luckily I just get migraines, not seizures. Do you really want to disable people's safety and accessibility tools? If you are going to do this, then either (a) clearly warn about the Firefox 57 update, and accept that some people are going to have to keep using Firefox 56 for the forseeable future, (b) make sure every accessibility extension has a 57-compatible version, or (c) make sure Firefox itself is accessible without accessibility extensions. (b) and (c) seem very impractical.

As spoken with kmoir yesterday: >so there used to be add-on builds which were unbranded. The builds have been replaced by devedition builds and this change is >riding the trains. It is currently on beta. On the next uplift (end of month), it will ride the trains to release and there >won't be any addon builds enabled. >so what I'm saying is that I think you just need to change the devedition mozconfigs >not worry about unbranded builds because they are going away at the same time >unbranded builds will be removed after the next merge So we only need to add this change to the nightly and devedition as the unbranded builds will be removed after the next merge I think.

Not sure if this is all that need to be changed.

Comment on attachment 8902182 [details] [diff] [review] bug1363897_v2.patch Review of attachment 8902182 [details] [diff] [review]: ----------------------------------------------------------------- :glandium can give you the right exact syntax, my empirical experience is that you need either MOZ_ALLOW_LEGACY_EXTENSIONS='' or MOZ_ALLOW_LEGACY_EXTENSIONS=1 But more importantly, the configure script defaults to allowing legacy extensions in nightly and not in any other builds, so you only really need to manually override it for dev edition and unbranded (if unbranded builds are still a thing?)

Comment on attachment 8902182 [details] [diff] [review] bug1363897_v2.patch You only need to add either moz_add_options "export MOZ_ALLOW_LEGACY_EXTENSIONS=1" or ac_add_options "MOZ_ALLOW_LEGACY_EXTENSIONS=1" to the devedition mozconfigs. Nothing else, since the default is enabled for builds off mozilla-central and integration branches and disabled for all other builds.

The new patch which enable the option to devedition in mozconfig. I don't know if we still need maybe to integrate this part from the previous script or change it a little bit: >diff --git a/browser/confvars.sh b/browser/confvars.sh >--- a/browser/confvars.sh >+++ b/browser/confvars.sh >@@ -67,3 +67,10 @@ MOZ_ADDON_SIGNING=1 > > # Include the DevTools client, not just the server (which is the default) > MOZ_DEVTOOLS=all >+# Enable legacy extensions in nightly post 57 - Bug1363897 >+MOZ_ALLOW_LEGACY_EXTENSIONS=1 >+if test "$MOZ_UPDATE_CHANNEL" = "beta" -o \ >+ "$MOZ_UPDATE_CHANNEL" = "release" -o \ >+ "$MOZ_UPDATE_CHANNEL" = "esr"; then >+ MOZ_ALLOW_LEGACY_EXTENSIONS=0 >+fi

(In reply to Andrei Obreja [:aobreja][:buildduty] from comment #43) > Created attachment 8902643 [details] [diff] [review] > bug1363897_v3.patch > > The new patch which enable the option to devedition in mozconfig. > I don't know if we still need maybe to integrate this part from the previous > script or change it a little bit: > > >diff --git a/browser/confvars.sh b/browser/confvars.sh > >--- a/browser/confvars.sh > >+++ b/browser/confvars.sh > >@@ -67,3 +67,10 @@ MOZ_ADDON_SIGNING=1 > > > > # Include the DevTools client, not just the server (which is the default) > > MOZ_DEVTOOLS=all > >+# Enable legacy extensions in nightly post 57 - Bug1363897 > >+MOZ_ALLOW_LEGACY_EXTENSIONS=1 > >+if test "$MOZ_UPDATE_CHANNEL" = "beta" -o \ > >+ "$MOZ_UPDATE_CHANNEL" = "release" -o \ > >+ "$MOZ_UPDATE_CHANNEL" = "esr"; then > >+ MOZ_ALLOW_LEGACY_EXTENSIONS=0 > >+fi No, you don't need this.

Pushed by mtabara@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/a2723b650460 enable MOZ_ALLOW_LEGACY_EXTENSIONS for devedition configs. r=glandium

Comment on attachment 8902643 [details] [diff] [review] bug1363897_v3.patch https://hg.mozilla.org/integration/mozilla-inbound/rev/a2723b65046096e587b968a3b6b1cb056c914b78

Is there something else that we buildduty can help with ,on this bug.

(In reply to Andrei Obreja [:aobreja][:buildduty] from comment #47) > Is there something else that we buildduty can help with ,on this bug. I can't think of anything, but QA should definitely test that everything worked properly when 57 beta and dev edition builds come out. CC krupa.

Merge day is fast approaching, as I understand from RelMan, merge day could happen a week sooner. @aswan: Sorry to insist, but just want to double-check: a) there's nothing else we need to do while 57 is still nightly b) more testing is needed when this hits beta/devedition. Re: testing: is this something you mention to QE or does this fall under RelEng's radar? Am doing bug triaging and am not sure if this is something to keep under Buildduty's plate or move elsewhere. Thank you!

(In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #50) > Re: testing: is this something you mention to QE or does this fall under > RelEng's radar? I was assuming QE would do it.

(In reply to Andrew Swan [:aswan] (PTO through 9/4) from comment #51) > (In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #50) > > Re: testing: is this something you mention to QE or does this fall under > > RelEng's radar? > > I was assuming QE would do it. Definitely. Sorry for confusion, what I meant is who's giving them a heads-up with respect to this? Not sure if testing this falls under regular testing they are performing. If this is already on their plate and they are aware of it, that's awesome and we're all good. Otherwise, if RelEng needs to heads-up them on this or somebody else, just wanted to make sure that's being tracked.

Krupa, see comment 48 above

This bug is verified on Firefox 57.0b7 (20171009192146) and under Windows 10 64bit/Mac OS X 10.12.3. Please see the attached video.