Closed Bug 307266 Opened 19 years ago Closed 19 years ago

EM doesn't select newly installed extension

Categories

(Toolkit :: Add-ons Manager, defect)

x86
All
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: doronr, Assigned: doronr)

Details

Attachments

(1 file)

Regression from my richlistbox patch probably.
rob - so I added code to addDownloads and it works, untill the install has finished, and then it gets unselected. Do we do a template refresh for when we finished downloading an extension?
Status: NEW → ASSIGNED
oh yeah, if anyone wants to step up, I won't get to this till afternoon tomorrow. gExtensionsView.selectedIndex = gExtensionsView.getRowCount() - 1; in : addDownloads() is one part of the fix.
I'm not sure and I haven't been keeping up with the richlistbox mods... I'll try to find out this evening. Also, the selected theme isn't previewed (at least not on linux) when opening the theme manager. I suppose the select event when it is opened is called before the listener has been added. Cheap workaround in extensions.js startup if (!isExtensions) { + onThemeSelect(); gExtensionsView.addEventListener("select", onThemeSelect, false);
Attached patch le patchSplinter Review
1) fixes theme preview - the select handler was being added after the datasource was set. 2) fixes EM to select the newly added extension 3) if an extensions/theme is finished downloading, we create a new item and remove the downloading version from the rdf ds, but the richlistitem dtor isn't called. So manually set it to the last item.
Attachment #195147 - Flags: review?(rob_strong)
doron - sorry, I got hung up on bug 307235. This patch doesn't take into account items that are updated so it will need some reworking. I should have time tomorrow evening to look at how to accomplish this.
Comment on attachment 195147 [details] [diff] [review] le patch > >- // Finally, update the UI. >- gExtensionsView.database.AddDataSource(gExtensionManager.datasource); >- gExtensionsView.setAttribute("ref", RDFURI_ITEM_ROOT); >- gExtensionsView.focus(); >- >- var pref = Components.classes["@mozilla.org/preferences-service;1"] >- .getService(Components.interfaces.nsIPrefBranch); >- var defaultPref = pref.QueryInterface(Components.interfaces.nsIPrefService) >- .getDefaultBranch(null); You want to leave the pref section where it is since it is used directly below. > if (!isExtensions) { > gExtensionsView.addEventListener("select", onThemeSelect, false); > > try { > gCurrentTheme = pref.getCharPref(PREF_GENERAL_SKINS_SELECTEDSKIN); >+ // Finally, update the UI. >+ gExtensionsView.database.AddDataSource(gExtensionManager.datasource); >+ gExtensionsView.setAttribute("ref", RDFURI_ITEM_ROOT); >+ gExtensionsView.focus(); >+ >+ var pref = Components.classes["@mozilla.org/preferences-service;1"] >+ .getService(Components.interfaces.nsIPrefBranch); >+ var defaultPref = pref.QueryInterface(Components.interfaces.nsIPrefService) >+ .getDefaultBranch(null); >+ and this won't need the pref section. ;) After reacquainting mmyself with the code I don't think selecting is a good idea at this time due to other issues... if an item that is already installed is installed again and it isn't the last item it won't be selected, it is possible that multiple items can be installed at the same time in which case only one of the items will be selected, etc. Current behavior is to ensure the item is visible during the start of the install when it is appended to the list while leaving the last item that was selected as selected. I think this is as acceptable as any of the other options but Ben may see things differently. I'll r+ a patch with just the theme fix.
Attachment #195147 - Flags: review?(rob_strong) → review-
cc: UI people to see if they accept
*** Bug 307718 has been marked as a duplicate of this bug. ***
Flags: blocking1.8b5?
Unless we have a reliable way to select updated extensions, this is a non-starter. I don't see why that's not possible though.
(In reply to comment #6) > is installed again and it isn't the last item it won't be selected, it is > possible that multiple items can be installed at the same time in which case If I understand things right, the only case where we get multiple installs that kick off at the exact same time is when we're doing updates, is that right? I think there we can get away with selecting the last of the list of extensions being updated. In any other case where there's already an install in progress and the user kicks off a new one, then we select the most-recently installed item.
(In reply to comment #9) > Unless we have a reliable way to select updated extensions, this is a > non-starter. I don't see why that's not possible though. Say you have extension Foo. You then go and install a update manually. So we get a downloading Foo.xpi item, which once its down downloading goes away. The issue is how do we figure out that Foo was "reinstalled" and select that item? I don't know much about the itnernals of EM.
I filed the theme manager issue as bug 308358
Currently there is no clean way to do this which is why I'd prefer to hold off until 2.0 for this. Ben put together some ascii showing a possible ui rewrite where there would be different views in a single ui. With this it would be possible to change to a view on install or upgrade for one or multiple items (extensions and themes) and simplify what we'd like to accomplish in this bug.
Flags: blocking1.9a1?
Flags: blocking1.8b5?
Flags: blocking1.8b5-
There is now an updates and an installs view on the trunk both of which will select the first add-on in the list when they are displayed whereas the extensions, themes, languages, and plugins views will always select the last selected add-on. With these changes I believe this should now be wontfix.
If there are no objections in the next week I'm going to mark this wontfix. If there are I'd appreciate details as to how this should work in the new ui. Thanks
yum, new UI.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → WONTFIX
Flags: blocking1.9a1?
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: