391899 - getItemForID returns an empty nsIUpdateItem if the item doesn't exist

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Dave Townsend [:mossop]]

I think the API would make more sense for getItemForID to return null in the event you request an item that isn't installed. I don't see anywhere that relies on having this empty item. Thoughts Rob?

Comment 1

[Robert Strong (they/them - no direct email)]

Agreed though I swear there were some places that depended on it.

Comment 2

[Dave Townsend [:mossop]]

Attachment: patch rev 1

I've been through every call I could find and only on in fuel looked like it needed an amendment and for the better as well.

This just checks if the id is in the known list and bails out of not. Includes a simple unit test to check the behaviour.

Comment 3

[Dave Townsend [:mossop]]

Rob, I've done some testing with this patch applied and not come across any issues as yet.

Comment 4

[Robert Strong (they/them - no direct email)]

Comment on attachment 276510 [details] [diff] [review]
patch rev 1

Dave, I vaguely recall there being a call site that depended on this returning an item when it wasn't in the datasource but for the life of me I can't find it. I've looked through the call sites and I don't see any that do so perhaps this was fixed at some point. Let's keep an eye on this especially incompatible installs that are then made compatible by their update rdf. r=me

Comment 5

[Dave Townsend [:mossop]]

RCS file: /cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/test_bug391899.js,v
done
Checking in toolkit/mozapps/extensions/test/unit/test_bug391899.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/test_bug391899.js,v  <--  test_bug391899.js
initial revision: 1.1
done
Checking in toolkit/mozapps/extensions/src/nsExtensionManager.js.in;
/cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v  <--  nsExtensionManager.js.in
new revision: 1.240; previous revision: 1.239
done
Checking in toolkit/mozapps/extensions/public/nsIExtensionManager.idl;
/cvsroot/mozilla/toolkit/mozapps/extensions/public/nsIExtensionManager.idl,v  <--  nsIExtensionManager.idl
new revision: 1.51; previous revision: 1.50
done
Checking in browser/fuel/src/fuelApplication.js;
/cvsroot/mozilla/browser/fuel/src/fuelApplication.js,v  <--  fuelApplication.js
new revision: 1.19; previous revision: 1.18
done