Closed Bug 792209 (nuke-nsSupportsArray) Opened 10 years ago Closed 6 years ago
nsISupportsArray is ugly and it's deprecated (for almost 10 years!); we should just remove it. Let's use this bug to coordinate the various pieces of work that need to be done
This is still lingering, we can see it in 6 interfaces: - docshell/shistory/nsISHEntry.idl - dom/media/nsIMediaManager.idl - layout/inspector/inIDOMUtils.idl - rdf/base/nsIRDFDataSource.idl - widget/nsIDragService.idl - widget/nsIFormatConverter.idl We can see an example in embedding/components/windowwatcher/nsIWindowWatcher.idl where a param is declared as |nsISupports|  to allow both |nsIArray| and |nsISupportsArray| to be passed in, internally it is converted to an |nsIArray| . This might be an interesting solution allowing us to remove usage internally but not totally break addons.  http://searchfox.org/mozilla-central/rev/fd672b97f13aa83af5f04caa7b61bd443fb623e9/embedding/components/windowwatcher/nsIWindowWatcher.idl#53-56  http://searchfox.org/mozilla-central/rev/fd672b97f13aa83af5f04caa7b61bd443fb623e9/embedding/components/windowwatcher/nsWindowWatcher.cpp#307
All of the remaining IDL interfaces in m-c have been updated, two are pending review: - layout/inspector/inIDOMUtils.idl (bug 1311191) - rdf/base/nsIRDFDataSource.idl (bug 1309409) There's still a bit of usage in the tree (ignoring xpcom where it's defined): - toolkit/obsolete/content/nsClipboard.js Add-ons folks indicated it might be okay to nuke the obsolete dir, I'll file a bug to track that. - xpfe/components/directory/nsDirectoryViewer.cpp xpfe/components/directory/nsDirectoryViewer.h We can probably convert these to nsIMutableArray (or even nsCOMArray). - browser/base/content/browser.js browser/components/nsBrowserContentHandler.js browser/components/extensions/ext-windows.js I haven't totally worked this out, but we seem to have some functionality where we set window.arguments to a nsISupportsArray of URLs. I haven't been able to find a consumer of that yet. - browser/base/content/sanitizeDialog.js This looks like an invokeDragSession user I may have missed. - embedding/components/windowwatcher/nsWindowWatcher.cpp This is our handling of window args being either nsISupportsArray or nsIArray, since nsISupportsArray *is* an nsIArray now I think we can get rid of this logic and make the args nsIArray.
Just landed bug 1309409 (rdf and friends), if that sticks we'll have removed all usage in m-c aside from the definition in xpcom.
The IDL files have been marked as deprecated in bug 1315812 as of Firefox 52. The plan is to let that make it to release and then completely remove the files. I posted a notice to dev-platform as well .  https://groups.google.com/d/msg/mozilla.dev.platform/_JghW4BJgls/V1qHcvq3BgAJ
This is the final removal patch. I wanted to get it done now while all of this is fresh, but we'll hold off on landing until 52 is on release.
Attachment #8811876 - Flags: review?(nfroyd)
Comment on attachment 8811876 [details] [diff] [review] Remove nsISupportsArray Review of attachment 8811876 [details] [diff] [review]: ----------------------------------------------------------------- So we're going to go through a complete 3-month release "cycle" rather than a six-week one? That works for me, but I had interpreted "waiting a release cycle" to be the six-week one. This probably works out a little better for addon compat and for Thunderbird, I suppose.
Attachment #8811876 - Flags: review?(nfroyd) → review+
(In reply to Nathan Froyd [:froydnj] from comment #7) > Comment on attachment 8811876 [details] [diff] [review] > Remove nsISupportsArray > > Review of attachment 8811876 [details] [diff] [review]: > ----------------------------------------------------------------- > > So we're going to go through a complete 3-month release "cycle" rather than > a six-week one? That works for me, but I had interpreted "waiting a release > cycle" to be the six-week one. This probably works out a little better for > addon compat and for Thunderbird, I suppose. Yes it's primarily for add-ons, I'm less concerned about TB because a) I'm giving them patches to work with and b) they are aware of what's going on.
Hi, I see you are pushing this to try server. What is the plan to merge this in m-c? We still have one remaining usage in mailnews in bug 1318806 (which you have filed, thanks).
(In reply to :aceman from comment #10) > Hi, I see you are pushing this to try server. What is the plan to merge this > in m-c? > We still have one remaining usage in mailnews in bug 1318806 (which you have > filed, thanks). I'm testing out a rebased version. My plan is to land when 52 hits release next week.
https://hg.mozilla.org/integration/mozilla-inbound/rev/70dec4a3877dd1dac1f819baa3557ed21f8b0249 Bug 792209 - Remove nsISupportsArray. r=froydnj
Can this change be added to https://developer.mozilla.org/en-US/Firefox/Releases/55? It broke one of the old legacy extensions that I work on.
You need to log in before you can comment on or make changes to this bug.