Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Remove support for multi-package xpis

RESOLVED FIXED in Firefox 53

Status

()

Toolkit
Add-ons Manager
RESOLVED FIXED
7 months ago
4 months ago

People

(Reporter: aswan, Assigned: aswan)

Tracking

({addon-compat, dev-doc-needed})

51 Branch
mozilla53
addon-compat, dev-doc-needed
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

(Whiteboard: triaged)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

7 months ago
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 hidden (mozreview-request)

Comment 2

7 months ago
mozreview-review
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 3

7 months ago
mozreview-review-reply
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 hidden (mozreview-request)
(Assignee)

Comment 5

7 months ago
mozreview-review-reply
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...
(Assignee)

Updated

7 months ago
Whiteboard: triaged
Comment hidden (mozreview-request)

Comment 7

7 months ago
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b133e1edcf02
Remove support for multi-package xpis r=rhelmer
(Assignee)

Updated

7 months ago
Keywords: addon-compat, dev-doc-needed
https://hg.mozilla.org/mozilla-central/rev/b133e1edcf02
Status: NEW → RESOLVED
Last Resolved: 7 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53

Comment 9

5 months ago
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.