1359203 - Create a preference to control whether legacy extensions may be enabled

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Comment 1

[Andrew Swan [:aswan]]

First and last call for bikeshedding on the preference name.
I propose "extensions.legacy.enabled"

Comment 2

[Andy McKay]

+1, bikeshedding period over.

Comment 3

[Andrew Swan [:aswan]]

Attachment: Bug 1359203 Part 1 Replace AddonConstants with AddonSettings

AddonConstants duplicated AppConstants, but having it just include
the build-time constants meant that the logic for whether to enforce
signing was repeated in a few places (and it was incorrect in
the front-end code!).  Now we centralize that logic in AddonSettings.jsm

Review commit: https://reviewboard.mozilla.org/r/139584/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139584/

Comment 4

[Andrew Swan [:aswan]]

Attachment: Bug 1359203 Part 2 Add build-time constant for allowing legacy extensions

Review commit: https://reviewboard.mozilla.org/r/139586/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139586/

Comment 5

[Andrew Swan [:aswan]]

Attachment: Bug 1359203 Part 3 Add extensions.legacy.enabled preference

Review commit: https://reviewboard.mozilla.org/r/139588/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139588/

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8868017 [details]
Bug 1359203 Part 2 Add build-time constant for allowing legacy extensions

https://reviewboard.mozilla.org/r/139586/#review143388

::: browser/confvars.sh:69
(Diff revision 1)
>  
>  # Enable checking that add-ons are signed by the trusted root
>  MOZ_ADDON_SIGNING=1
>  
> +# Allow Legacy extensions unless explicitly overridden
> +MOZ_ALLOW_LEGACY_EXTENSIONS=1

Please move this to browser/moz.configure|toolkit/moz.configure. See what's done for e.g. MOZ_PLACES.

Comment 7

[Kris Maglione [:kmag]]

Comment on attachment 8868015 [details]
Bug 1359203 Part 1 Replace AddonConstants with AddonSettings

https://reviewboard.mozilla.org/r/139584/#review143806

::: toolkit/mozapps/extensions/test/browser/browser_details.js:713
(Diff revision 1)
>      });
>    });
>  });
>  
>  // These tests are only appropriate when signing can be turned off
> -if (!REQUIRE_SIGNING) {
> +if (!AppConstants.REQUIRE_SIGNING) {

These should be MOZ_REQUIRE_SIGNING

::: toolkit/mozapps/extensions/test/browser/browser_list.js:421
(Diff revision 1)
>    is(get_node(addon, "error-link").value, "More Information", "Error link text should be correct");
>    is(get_node(addon, "error-link").href, "http://example.com/[e-mail]", "Error link should be correct");
>    is_element_hidden(get_node(addon, "pending"), "Pending message should be hidden");
>  
>    // These tests are only appropriate when signing can be turned off
> -  if (!REQUIRE_SIGNING) {
> +  if (!AppConstants.REQUIRE_SIGNING) {

*MOZ_REQUIRE_SIGNING

Comment 8

[Kris Maglione [:kmag]]

Comment on attachment 8868018 [details]
Bug 1359203 Part 3 Add extensions.legacy.enabled preference

https://reviewboard.mozilla.org/r/139588/#review143812

::: toolkit/mozapps/extensions/internal/AddonSettings.jsm:43
(Diff revision 1)
> +
> +if (AppConstants.MOZ_ALLOW_LEGACY_EXTENSIONS || Cu.isInAutomation) {
> +  XPCOMUtils.defineLazyPreferenceGetter(AddonSettings, "ALLOW_LEGACY_EXTENSIONS",
> +                                        PREF_ALLOW_LEGACY, true);
> +} else {
> +  makeConstant( "ALLOW_LEGACY_EXTENSIONS", false);

Nit: Remove space after `(`

Comment 9

[Andrew Swan [:aswan]]

Comment on attachment 8868015 [details]
Bug 1359203 Part 1 Replace AddonConstants with AddonSettings

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139584/diff/1-2/

Comment 10

[Andrew Swan [:aswan]]

Comment on attachment 8868017 [details]
Bug 1359203 Part 2 Add build-time constant for allowing legacy extensions

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139586/diff/1-2/

Comment 11

[Andrew Swan [:aswan]]

Comment on attachment 8868018 [details]
Bug 1359203 Part 3 Add extensions.legacy.enabled preference

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139588/diff/1-2/

Comment 12

[Mike Hommey [:glandium]]

Comment on attachment 8868017 [details]
Bug 1359203 Part 2 Add build-time constant for allowing legacy extensions

https://reviewboard.mozilla.org/r/139586/#review144498

Comment 13

[Andrew Swan [:aswan]]

Comment on attachment 8868015 [details]
Bug 1359203 Part 1 Replace AddonConstants with AddonSettings

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139584/diff/2-3/

Comment 14

[Andrew Swan [:aswan]]

Comment on attachment 8868017 [details]
Bug 1359203 Part 2 Add build-time constant for allowing legacy extensions

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139586/diff/2-3/

Comment 15

[Andrew Swan [:aswan]]

Comment on attachment 8868018 [details]
Bug 1359203 Part 3 Add extensions.legacy.enabled preference

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139588/diff/2-3/

Comment 16

[Andrew Swan [:aswan]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2766cd8808dd2d1d66bc4e9e9e313bbc60b9a197
Bug 1359203 Part 1 Replace AddonConstants with AddonSettings

https://hg.mozilla.org/integration/mozilla-inbound/rev/49c132a43a7dd020d6777dbd6afe6b7dc83300e9
Bug 1359203 Part 2 Add build-time constant for allowing legacy extensions

https://hg.mozilla.org/integration/mozilla-inbound/rev/4e40b5b294428fc51707a47a471cdb5d90424fdd
Bug 1359203 Part 3 Add extensions.legacy.enabled preference

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2766cd8808dd
https://hg.mozilla.org/mozilla-central/rev/49c132a43a7d
https://hg.mozilla.org/mozilla-central/rev/4e40b5b29442