251100 - "Move" commands in EM context menu do nothing

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Jesse Ruderman]

1. Open Extension Manager with two extensions.
2. Right-click an extension.
3. Try "Move to top", "Move down", or "Move up" on either extension.

Result: Nothing happens.

Comment 1

[Jesse Ruderman]

WFM

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.2) Gecko/20040825
Firefox/0.9.1+

Comment 2

[(not currently active) Ted Mielczarek]

I can reproduce on Linux.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.2) Gecko/20040824 

I have "Focus follows mouse pointer" set.  If I move off the Extensions window,
then back, these menu items don't work.

Comment 3

[(not currently active) Ted Mielczarek]

On linux, using "Select windows when the mouse moves over them" (Gnome pref)

Open extensions window
mouse out of window
mouse back in
right click on an extension
select "move up" or "move down"
nothing happens.

Also, if you right click on a different extension than what's selected, the
menuitems aren't updated to reflect that.  i.e. select the first extension,
right click on the second, "move up" is still greyed out.  But that's probably a
separate bug.

Comment 4

[Jesse Ruderman]

Hmm, I wonder if there are platform differences due to Linux opening context
menus onmousedown.

Comment 5

[Phil Randal]

A variation of this bug still exists in the 20041106 Firefox build.

I have a load of extensions.  Move one up the list from near the bottom, and
then go back down, select another extension and try to move it.  No go,  Closing
and restarting enables you to do it, though.

Comment 6

[David McRitchie]

From Context menu: Can move extension to top,  can't move extension down, 
moving up is effective about 50% until it doesn't work at all and have to bring
up extensions (start over).    Of course the main purpose is to alphabetize the
placement of extensions and even if it were working properly from the context
menu it would still be very tedious.   Extensions can't be moved with arrow keys
alone and don't know if it is supposed to or not -- I thought that I had done it
that way before 1.0 but not sure. 

Comment 7

[(not currently active) Ted Mielczarek]

I think this will be fixed (or at least different) with Ben's EM changes.  He's
getting rid of the composite datasource.

Comment 8

[David McRitchie]

(In reply to comment #6)
Since I can use "Sort Extensions" by Robert William Vesterman, I have the
alphabetized list I wanted, and since sorting doesn't occur until I ask for it,
I still have the ability to see the recent additions and check them out before
realphabetizing the extensions list --
https://addons.update.mozilla.org/extensions/moreinfo.php?application=firefox&version=1.0&os=Windows&id=460
 

Comment 9

[Caleb]

This should supposedly be fixed by Bug 291572.

-> Adding 291572 to dependency tree.

Comment 10

[Ria Klaassen (not reading all bugmail)]

I also see this. I can always move to top but sometimes not up or down. The
harddisk says "prr" but the extension does not move.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050429
Firefox/1.0+

Comment 11

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

Attachment: patch

Extensions and themes are in the same RDF container so it isn't sufficient to
just move up/down by one in the container since it may just be moving an
extension before/after a theme which will not change the order in the EM. This
patch adds moveToIndexOf for use by both moveUp and moveDown and removes the
methods that are no longer needed. If desired, moveTop could also use
moveToIndexOf which would simplify nsIExtensionManager.idl even further.

Comment 12

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

Attachment: patch

I prefer this patch with the removal of moveTop from the idl (e.g. less code)
but if that isn't acceptable perhaps the previous patch is.

Comment 13

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

Attachment: cleaner patch

Sorry about canceling the reviews... I found some more remnants from the past
to clean up. It was calling getIDFromResourceURI and then later with that same
value calling getResourceForID which returns the original value.

Comment 14

[Benjamin Smedberg]

Comment on attachment 182913 [details] [diff] [review]
cleaner patch

Please remember to rev the IID of interfaces you change. r=me with
nsIExtensionManager revved.

Comment 15

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

Thanks for the review - I'll update the patch after bug 293461 is checked in or
they can both go in at the same time if it gets an a+ in time since they both
need to rev the same IID

Comment 16

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

Attachment: patch

Benjamin - I noticed performance during the renumberring is terrible when there
are lots of extensions - I tested with 30+. This patch batches the remove and
insert to lessen the performance impact.

Comment 17

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

Attachment: patch (includes IID rev)

The only change between this and the last patch is this one has the new IID
that was checked in with the patch from bug 293461 so it can be rev'd

Comment 18

[Asa Dotzler [:asa]]

Comment on attachment 183367 [details] [diff] [review]
patch

a=asa

Comment 19

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050513
Firefox/1.0+ ID:2005051315

WFM after this patch landed

Comment 20

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

Fix checked in - thanks bz - resolved->fixed

Comment 21

[Worcester12345]

Shouldn't there be a minus (-) flag on the blocker nomination? 

I know there are people counting which were blockers, fixed blockers, etc. and
this messes up the counts.

Thanks.

Comment 22

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

(In reply to comment #21)
> Shouldn't there be a minus (-) flag on the blocker nomination? 
> 
> I know there are people counting which were blockers, fixed blockers, etc. and
> this messes up the counts.
No... that isn't the purpose for the flags. Also, minusing would mess up the
counts just as much as removing the request since the request wasn't denied.
https://bugzilla.mozilla.org/flag-help.html