1662704 - Remove traces of MOZ_ALLOW_LEGACY_EXTENSIONS from source code

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, task, P5)

Description

[Rob Wu [:robwu]]

The MOZ_ALLOW_LEGACY_EXTENSIONS flag was removed in bug 1524327,
but (inadvertently) restored in https://hg.mozilla.org/mozilla-central/rev/acbb75628d35 (part of bug 1602840).

The constant is always false and just dead code.
We should remove it:

• delete https://searchfox.org/mozilla-central/rev/84922363f4014eae684aabc4f1d06380066494c5/toolkit/modules/AppConstants.jsm#303-309
• remove from https://searchfox.org/mozilla-central/rev/84922363f4014eae684aabc4f1d06380066494c5/toolkit/mozapps/extensions/test/xpcshell/test_temporary.js#177

Comment 1

[Michelle Goossens [:masterwayz]]

Hi,

If possible I'd like to handle this bug.
Other than deleting the lines, does anything else have to be done?

Comment 2

[Rob Wu [:robwu]]

Just removing the linked code is enough.

To get started, see https://wiki.mozilla.org/WebExtensions/Contribution_Onramp

Comment 3

[Michelle Goossens [:masterwayz]]

I've set up the environment successfully and also installed moz-phab. I've removed the first one with the lines that you have highlighted, on the second one, does the if (!packed && !AppConstants.MOZ_ALLOW_LEGACY_EXTENSIONS) become if (!packed)?

I don't know the codebase enough to know if the !packed part is still needed or not.

Lastly, once I have the answer I can make the commit. What are the next steps from there? Submit the diff here or with moz-phab and set who as the reviewer?

Sorry for the many questions, just getting started here and I'd like to make sure that I do things right and can contribute more in the future.

Comment 4

[Rob Wu [:robwu]]

(In reply to Michael Goossens from comment #3)

I've set up the environment successfully and also installed moz-phab. I've removed the first one with the lines that you have highlighted, on the second one, does the if (!packed && !AppConstants.MOZ_ALLOW_LEGACY_EXTENSIONS) become if (!packed)?

Upon taking a closer look at the code, the condition looks strange. Could you remove the whole condition and the comment, and run the test? It should be possible to load an unsigned unpacked extension, so I would expect the test to pass based on the comment.

Lastly, once I have the answer I can make the commit. What are the next steps from there? Submit the diff here or with moz-phab and set who as the reviewer?

Use moz-phab to upload the patch. If you add r=robwu to the local commit message, then I'll automatically be selected as the reviewer. For more info, see https://wiki.mozilla.org/WebExtensions/Contribution_Onramp#Submitting_a_Patch .

Sorry for the many questions, just getting started here and I'd like to make sure that I do things right and can contribute more in the future.

No worries! Van vragen stellen kan je alleen maar wijzer worden ;)

Comment 5

[Michelle Goossens [:masterwayz]]

Attachment: Bug 1662704 - Remove unused traces of MOZ_ALLOW_LEGACY_EXTENSIONS r=robwu

Comment 6

[Michelle Goossens [:masterwayz]]

I've just submitted it, the toolkit/mozapps/extensions/test/xpcshell/test_temporary.js test passes on my Windows machine.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/45326b7a2a8a
Remove unused traces of MOZ_ALLOW_LEGACY_EXTENSIONS r=robwu

Comment 8

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/45326b7a2a8a

Comment 9

[Caitlin Neiman [:caitmuenster]]

Thanks so much for the patch, Michael! 🎉 Your contribution has been added to our recognition wiki at https://wiki.mozilla.org/Add-ons/Contribute/Recognition.

Welcome onboard and we look forward to seeing you around!