Closed Bug 1662704 Opened 3 months ago Closed 3 months ago

Remove traces of MOZ_ALLOW_LEGACY_EXTENSIONS from source code

Categories

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

task

Tracking

()

RESOLVED FIXED
82 Branch
Tracking Status
firefox82 --- fixed

People

(Reporter: robwu, Assigned: michael, Mentored)

References

Details

(Keywords: good-first-bug)

Attachments

(1 file)

Mentor: rob
Severity: -- → N/A
Keywords: good-first-bug
Priority: -- → P5

Hi,

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

Flags: needinfo?(rob)

Just removing the linked code is enough.

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

Flags: needinfo?(rob)

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.

(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 ;)

Assignee: nobody → michael
Status: NEW → ASSIGNED

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

Pushed by ncsoregi@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/45326b7a2a8a
Remove unused traces of MOZ_ALLOW_LEGACY_EXTENSIONS r=robwu
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch

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!

You need to log in before you can comment on or make changes to this bug.