307266 - EM doesn't select newly installed extension

Status: RESOLVED WONTFIX

(Toolkit :: Add-ons Manager, defect)

Description

[Doron Rosenberg (IBM)]

Regression from my richlistbox patch probably.

Comment 1

[Doron Rosenberg (IBM)]

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?

Comment 2

[Doron Rosenberg (IBM)]

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.

Comment 3

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

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);

Comment 4

[Doron Rosenberg (IBM)]

Attachment: le patch

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.

Comment 5

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

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 6

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

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.

Comment 7

[Doron Rosenberg (IBM)]

cc: UI people to see if they accept

Comment 8

[Tracy Walker [:tracy]]

*** Bug 307718 has been marked as a duplicate of this bug. ***

Comment 9

[Mike Connor [:mconnor]]

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.

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

(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.

Comment 11

[Doron Rosenberg (IBM)]

(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.

Comment 12

[Doron Rosenberg (IBM)]

I filed the theme manager issue as bug 308358

Comment 13

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

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.

Comment 14

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

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.

Comment 15

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

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

Comment 16

[Doron Rosenberg (IBM)]

yum, new UI.