Closed
Bug 558134
Opened 15 years ago
Closed 14 years ago
Canceling add-on installation leaves behind unwanted (multiple) entries in the extensions list
Categories
(Toolkit :: Add-ons Manager, defect)
Toolkit
Add-ons Manager
Tracking
()
VERIFIED
FIXED
mozilla2.0b7
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: whimboo, Assigned: mossop)
References
Details
(Whiteboard: [rewrite])
Attachments
(2 files, 1 obsolete file)
(deleted),
image/jpeg
|
Details | |
(deleted),
patch
|
Unfocused
:
review+
|
Details | Diff | Splinter Review |
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a4pre) Gecko/20100407 Minefield/3.7a4pre (.NET CLR 3.5.30729)
Canceling the installation of an add-on leaves entries in the extensions pane behind. The same happens when you install the same add-on multiple times.
Steps:
1. Go to https://addons.mozilla.org/en-US/firefox/addon/6622
2. Install the extension but cancel the dialog
3. Install the extension
Comment 1•15 years ago
|
||
Status: NEW → ASSIGNED
Flags: in-testsuite?
Flags: in-litmus?
Whiteboard: [rewrite] → [rewrite][fixed-in-addonsmgr]
Updated•15 years ago
|
Assignee: nobody → bmcbride
Assignee | ||
Comment 3•15 years ago
|
||
Landed on trunk as a part of bug 554007
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [rewrite][fixed-in-addonsmgr] → [rewrite]
Target Milestone: --- → mozilla1.9.3a5
Reporter | ||
Comment 4•15 years ago
|
||
It's still there with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a5pre) Gecko/20100514 Minefield/3.7a5pre
Just try to install that extension and cancel the installation dialog:
https://addons.mozilla.org/en-US/firefox/addon/26/
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Updated•15 years ago
|
Status: REOPENED → ASSIGNED
Assignee | ||
Updated•15 years ago
|
blocking2.0: --- → final+
Assignee | ||
Comment 5•14 years ago
|
||
Blair, this is basically what we discussed about hiding STATE_AVAILABLE entries from the list. Ben, did you say you were likely to do this as part of something else anyway?
Comment 6•14 years ago
|
||
Hiding non-active entries (state = STATE_AVAILABLE) has mostly been done. When a list is shown, it will not have any non-active entries (part of patch for Bug 572561). However, non-active entries can still appear because the List View has a onNewInstall listener that adds items. So, Henrik's steps do show a non-active install. Reloading the list will hide the non-active install. So, to maintain consistency, we should probably remove the onNewInstall callback, and add onDownloadStarted and onInstallStarted callbacks, or do something similar.
I am able to reproduce the bug using the first link Henrik gave (DOM Inspector), but not the second link (Download Statusbar). The differentiating factor, I think, is that I have DOM inspector already installed, but not Download Statusbar. It seems that the onDownloadCancelled nor onInstallCancelled is fired if an install is canceled for an add-on that is already installed. Note that this was just a quick assessment, and could be wrong.
Assignee | ||
Comment 7•14 years ago
|
||
Sounds like you know more about what is going on here than anyone else, so it's yours to own ;)
Assignee: bmcbride → bparr
Assignee | ||
Updated•14 years ago
|
Assignee: bparr → dtownsend
Assignee | ||
Updated•14 years ago
|
Status: ASSIGNED → NEW
Assignee | ||
Comment 8•14 years ago
|
||
The problem here is that once gEventManager sees that an install event is for an install of an existing add-on it only sends notifications to listeners registered for that add-on which leaves the install entry out in the cold.
Assignee | ||
Comment 9•14 years ago
|
||
This relies on the patch from bug 597620 as it builds on top of the test there.
There are a couple of changes here.
The API only sends out onOperationCompleted after it has removed the pendingUpgrade property from the add-on, this allows undoing an upgrade to work in the UI.
All install events are sent out to all install listeners regardless of whether they are for upgrades too. This allos the list view to see that an install has become an upgrade and so to remove the installing entry.
Also a minor fix to stop trying to send onInstallCompleted when the install status is attached to an installed add-on which does not have that method.
Attachment #478547 -
Flags: review?(bmcbride)
Assignee | ||
Updated•14 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•14 years ago
|
Whiteboard: [rewrite] → [rewrite][has patch][needs review Unfocused]
Comment 10•14 years ago
|
||
Comment on attachment 478547 [details] [diff] [review]
patch rev 1
(In reply to comment #9)
> All install events are sent out to all install listeners regardless of whether
> they are for upgrades too. This allos the list view to see that an install has
> become an upgrade and so to remove the installing entry.
Most install listeners only expect events for new installs (they don't filter out upgrades because they assume the event manager did that). So this ends up creating new items in the list view for upgrades.
> Also a minor fix to stop trying to send onInstallCompleted when the install
> status is attached to an installed add-on which does not have that method.
This fixes bug 565562.
Attachment #478547 -
Flags: review?(bmcbride) → review-
Assignee | ||
Comment 11•14 years ago
|
||
Corrects and tests for that problem. I can't really see an alternative to startingto send all install events out to all install listeners short of having the listview register for addon events for all in the list, but that wouldn't really help any anyway.
Attachment #478547 -
Attachment is obsolete: true
Attachment #479885 -
Flags: review?(bmcbride)
Updated•14 years ago
|
Attachment #479885 -
Flags: review?(bmcbride) → review+
Assignee | ||
Updated•14 years ago
|
Whiteboard: [rewrite][has patch][needs review Unfocused] → [rewrite][has patch][needs-checking-post-b7]
Assignee | ||
Comment 12•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 15 years ago → 14 years ago
Flags: in-testsuite?
Flags: in-testsuite+
Flags: in-litmus?
Flags: in-litmus-
Resolution: --- → FIXED
Whiteboard: [rewrite][has patch][needs-checking-post-b7] → [rewrite]
Target Milestone: mozilla1.9.3a5 → mozilla2.0b8
Reporter | ||
Comment 14•14 years ago
|
||
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101012 Firefox/4.0b8pre. That only covers the steps from comment 0 with cancel involved. Bug 562922 which covers the multiple installation of the same extension is still valid.
Status: RESOLVED → VERIFIED
Updated•14 years ago
|
Target Milestone: mozilla2.0b8 → mozilla2.0b7
You need to log in
before you can comment on or make changes to this bug.
Description
•