535961 - Send proper notification for cancelled installs

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[:Gijs (he/him)]

STR:
0. Listen for item-cancel-action events, intending to check for the ID of the add-on so we know we're dealing with the right one.
1. Install an add-on.
2. Click the "cancel" button.

Expected results:
Observer gets notified about the canceling of installation with the nsIUpdateItem corresponding to the add-on that wasn't installed after all.

Actual results:
Observer gets notified about the canceling of installation with a null subject.

This is (presumably?) because the code here: http://hg.mozilla.org/mozilla-central/annotate/4d0b8367d940/toolkit/mozapps/extensions/src/nsExtensionManager.js.in#l4500

already removes the data from the datasource, and then by the time _notifyAction tries to get it from the datasource, it's not there anymore. 

If it's at all sane to do so, the notification should go before the actual removal of the data. If that violates promises item-cancel-action wants to make about what has/hasn't happened before the notification fires, perhaps _notifyAction should be updated so it takes (or can take) an nsIUpdateItem.

Maybe:

if (typeof id == "string")
   item = this.datasource.getItemForID(id);
else
   item = id;
gOS.notifyObservers(item, EM_ACTION_REQUESTED_TOPIC, reason);


in _notifyAction, and sending the item through directly from the callsite linked above? AFAICT that shouldn't be breaking anything else. I'd be happy to do a patch if this is the correct approach.

Comment 1

[Dave Townsend [:mossop]]

Re-wording since we will be moving to different notifications in the next iteration of the EM.

Comment 2

[Dave Townsend [:mossop]]

The new EM provides the InstallListener which should give all the notifications you could possibly want.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

See http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/XPIProvider.jsm#3950

Should have been fixed by bug 553169 (as far as I can see from hg log).