Closed Bug 1323128 Opened 3 years ago Closed 3 years ago

Remove support for multi-package xpis

Categories

(Toolkit :: Add-ons Manager, defect)

51 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: aswan, Assigned: aswan)

Details

(Keywords: addon-compat, dev-doc-needed, Whiteboard: triaged)

Attachments

(1 file)

Proposal and reasoning were circulated on a few mailing lists:
https://groups.google.com/forum/#!topic/mozilla.dev.platform/om5BrXM-Gb8

If there isn't any major objection I'll aim to land this later this week.
Comment on attachment 8818161 [details]
Bug 1323128 Remove support for multi-package xpis

https://reviewboard.mozilla.org/r/98270/#review98992

I see the string `linkedInstalls` in at least two tests:
`test_install.js`
`test_install_strictcompat.js`

I expect a try run will dredge these up though.

I don't think there's a need to re-review if you just kill some old test code to get try to pass.

::: toolkit/mozapps/extensions/internal/XPIProvider.jsm:227
(Diff revision 1)
>  // Map new string type identifiers to old style nsIUpdateItem types
>  const TYPES = {
>    extension: 2,
>    theme: 4,
>    locale: 8,
> -  multipackage: 32,
> +//  multipackage: 32,

Why commented out and not removed?
Attachment #8818161 - Flags: review?(rhelmer) → review+
Comment on attachment 8818161 [details]
Bug 1323128 Remove support for multi-package xpis

https://reviewboard.mozilla.org/r/98270/#review98992

Ah on further inspection, these just test that `linkedInstalls` is explicitly `false` so don't cause any failures. Still, should remove them to avoid confusion for future archaeologists.
Comment on attachment 8818161 [details]
Bug 1323128 Remove support for multi-package xpis

https://reviewboard.mozilla.org/r/98270/#review98992

Thanks for catching that, removed.

> Why commented out and not removed?

Ah, I figured I should leave something here indicating that this used to be used for something else so nobody should come along and re-use that value.  But you're right, leaving it commented out isn't great, what do you think?  And while we're at it, why do we only use powers of 2 here?  Its not like we can combine different values together in a bitfield...
Whiteboard: triaged
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b133e1edcf02
Remove support for multi-package xpis r=rhelmer
https://hg.mozilla.org/mozilla-central/rev/b133e1edcf02
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
There are still a dozen themes that used multi-package XPIs to bundle an add-on with their theme, such as Silvermel and Charamel, which bundle an add-on that allows for customization of the theme.
I didn't find any notice about the removal or any advance warning that support for this would be removed. 

Wouldn't it be better to delay the removal until 57? Themes (in their current form) are going to break with Firefox 57 anyway so why remove it early and break updates to these themes? I tried installing a multi-package XPI in 53 and all it did was tell me that the add-on was corrupt. No doorhanger notification about them not being supported, nothing.
If I were a user of a multi-XPI theme or add-on I would at least want to be notified about one of my add-ons no longer being supported.
You need to log in before you can comment on or make changes to this bug.