Closed Bug 857456 Opened 11 years ago Closed 5 years ago

stop supporting install.rdf

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: Unfocused, Assigned: aswan)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

Currently, the Add-ons Manager uses nsIRDFService to read install.rdf - we'd like to kill that with fire. However, we can't just drop support for install.rdf. Instead, I'd like to move to using the RDF reader JSM that bug 857454 will implement.
Depends on: 357276
No longer depends on: 857454
Blocks: 833098
Summary: Use RDF reader module for reading install.rdf manifests → Use RDF reader module for reading install.rdf manifests, or convert them to JSON
:mak, it seems like we can close this now that we only support web extensions. Is that correct?
Flags: needinfo?(mak77)
We still support legacy extensions (which use install.rdf) for things like system addons and extensions used in automation.  Switching these over to json so we can ditch rdf would be a fine thing but there are a lot of pieces to update.
Flags: needinfo?(mak77)
(In reply to Andrew Swan [:aswan] from comment #2)
> We still support legacy extensions (which use install.rdf) for things like
> system addons and extensions used in automation.  Switching these over to
> json so we can ditch rdf would be a fine thing but there are a lot of pieces
> to update.

Do we have a list somewhere? How long do we plan on supporting them?
(In reply to Eric Rahm [:erahm] (please no mozreview requests) from comment #3)
> (In reply to Andrew Swan [:aswan] from comment #2)
> > We still support legacy extensions (which use install.rdf) for things like
> > system addons and extensions used in automation.  Switching these over to
> > json so we can ditch rdf would be a fine thing but there are a lot of pieces
> > to update.
> 
> Do we have a list somewhere? How long do we plan on supporting them?

I don't mean this to be cheeky but I would start with something like:
http://searchfox.org/mozilla-central/search?q=&case=false&regexp=false&path=install.rdf

Off the top of my head, test pilot and devtools are the other big users of legacy extensions.

This has sort of been in the background for a while but folks working on addons have had a lot of other things in front of us.  Now that 57 is mostly done we can come back to things like this.  Just out of curiosity, is there something other than a general desire to get rid of old cruft that brought this to the foreground right now?
(In reply to Andrew Swan [:aswan] from comment #4)
> (In reply to Eric Rahm [:erahm] (please no mozreview requests) from comment
> #3)
> > (In reply to Andrew Swan [:aswan] from comment #2)
> > > We still support legacy extensions (which use install.rdf) for things like
> > > system addons and extensions used in automation.  Switching these over to
> > > json so we can ditch rdf would be a fine thing but there are a lot of pieces
> > > to update.
> > 
> > Do we have a list somewhere? How long do we plan on supporting them?
> 
> I don't mean this to be cheeky but I would start with something like:
> http://searchfox.org/mozilla-central/
> search?q=&case=false&regexp=false&path=install.rdf

That's reasonable! Looks like we have quite a few (even ignoring tests).

> This has sort of been in the background for a while but folks working on
> addons have had a lot of other things in front of us.  Now that 57 is mostly
> done we can come back to things like this.  Just out of curiosity, is there
> something other than a general desire to get rid of old cruft that brought
> this to the foreground right now?

I'm trying to revive the removal of RDF (which I get is probably low priority). It seems like adding a shim for parsing add-on install manifests might be the easiest route or I guess adding json support and some sort of automated conversion.
(In reply to Eric Rahm [:erahm] (please no mozreview requests) from comment #5)
> I'm trying to revive the removal of RDF (which I get is probably low
> priority). It seems like adding a shim for parsing add-on install manifests
> might be the easiest route or I guess adding json support and some sort of
> automated conversion.

I'd vote for just getting rid of it completely.  The very crude outline is of course:
1. Offer a json alternative
2. Convert existing users over
3. Rip out the rdf support

I don't think #1 will be too tough, let me see if I can crank something out to get the ball rolling.
As per bug 357276
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
No longer blocks: 833098
While we won't get a reader, install.rdf is far from being gone:
https://searchfox.org/mozilla-central/search?q=install.rdf&path=

It's true now we don't support legacy add-ons by default, but still the situation doesn't allow to remove rdf code.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Summary: Use RDF reader module for reading install.rdf manifests, or convert them to JSON → stop supporting install.rdf
Blocks: 833098
Depends on: 1449052
Depends on: 1452330
Update: we have removed a bunch of addon types that used install.rdf.  The two remaining are dictionaries and bootstrapped extensions.  Rather than changing from install.rdf to a JSON format as suggested in comment 6, we will remove support for both of these extension types (in favor of WebExtension-based alternatives).  The dependencies on this bug cover that effort.
Depends on: 1410214
No longer depends on: 1452330
No longer blocks: 833098
Blocks: 1460180
No longer blocks: 1460180
AddonInternal objects have a "type" field which is used inside the
addon manager, but AddonWrapper objects map the internal values to
different externally visible values.  The internal values generally
convey whether a particular addon uses webextension packaging
(ie manifest.json) or not.  This patch cleans that all up by adding a
new isWebExtension property and then just using the externally visible
values for type consistently.
This patch adds a hook to the addon manager that can be used to teach it
how to load xpi-packaged addons that the addon manager doesn't already
know about.  Handing for install.rdf/bootstrap.js is (temporarily)
converted to use this hook rather than being built directly into the
addon manager.
The addon manager has a bunch of handling for "strict compatibility" --
an option for individual profiles and addons to control exactly how
compatibility is handled.  However, WebExtensions don't use or expose
this feature.  We want to retain this capability for other projects that
use the addon manager but since it cannot be tested with WebExtensions,
this patch converts its tests to use the new extension loading hook added
in the previous patch.
First draft patches are up on phabricator.

For folks working on Thunderbird or Sea Monkey or anything else that uses toolkit and wants to keep using bootstrapped extensions, you'll basically want to revert the part 4 patch and reinstate BootstrapHandler.jsm in some appropriate place.
I think we would accept patches to change the way the handler is loaded (eg to use the category manager) but I didn't want to try to handle that without understanding how other projects will actually use this.
Assignee: nobody → aswan
Priority: -- → P1
Andrew, thanks a lot for the details. I hope out "add-ons man" Geoff can handle it.
Flags: needinfo?(geoff)
Thanks Andrew, this all looks reasonable and I think I understand what I'll need to do. I'm guessing your ETA is at the end of this week?
Flags: needinfo?(geoff)
That's the rough plan, yes.
Depends on: 1510097
I've copied everything across and mostly working as expected, but the tests stop at a call to AddonManager.getInstallForFile, which returns an AddonInstall with most properties set to null. This isn't intended, is it?
Flags: needinfo?(aswan)
No.  I'd be happy to help if you can give a little more detail, but lets keep that discussion over on bug 1510097.
Flags: needinfo?(aswan)
https://hg.mozilla.org/integration/mozilla-inbound/rev/efac27a2bec4fa7e81fa3c8a6d023795aa6447dc
Bug 857456 Part 0: Clean up remaining tests using legacy extensions r=kmag

https://hg.mozilla.org/integration/mozilla-inbound/rev/51bb2e2f30d2c1486ebbc75568e09976a374fe6c
Bug 857456 Part 1: Get rid of internal/external type distinction in addons manager r=kmag

https://hg.mozilla.org/integration/mozilla-inbound/rev/061b97e02ede2133f50956feba25fd8798745347
Bug 857456 Part 2: Separate handling for non-webextensions from the addons manager r=kmag

https://hg.mozilla.org/integration/mozilla-inbound/rev/e3b6db3b34ab06b60ab5b0f9d8d5695df754b7e1
Bug 857456 Part 3: Convert (non-) strict compatibility tests to use an external extension loader r=kmag

https://hg.mozilla.org/integration/mozilla-inbound/rev/0c715a8f0170fd4b19c4281c269c03f872090648
Bug 857456 Part 4: Remove toolkit support for install.rdf r=kmag
https://hg.mozilla.org/integration/mozilla-inbound/rev/368e88f08417bc4edcd4c4b48b43369083f0e7b4
Bug 857456 Follow-up: temporarily disable addon test in testDistribution.java rs=nalexander
Depends on: 1511345
Depends on: 1522237
Depends on: 1525814
Regressions: 1540368
You need to log in before you can comment on or make changes to this bug.