168728 - Languages, list order: Multiple selection is not supported by Move buttons

Status: NEW

(SeaMonkey :: Preferences, enhancement)

Description

[Serge Gautherie (:sgautherie)]

User-Agent:       Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.2a) Gecko/20020910
Build Identifier: Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.2a) Gecko/20020910

With multiple selection on the list,

Remove button works;

While Move_Up and Move_Down buttons are clickable but do nothing.

Reproducible: Always

Steps to Reproduce:
1. Edit > Preferences... > Languages
2. Make multiple selection
3. Press any of the 2 Move buttons

Actual Results:  
The selected items do not move.

Expected Results:  
The selected items should move.

Another possibility would be to disable the Move buttons when there is a
multiple selection. ("easier" but less usefull to the user)

Workaround:
*Move items one at a time.

Comment 1

[Serge Gautherie (:sgautherie)]

"Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.2) Gecko/20021126"

Bug still there.

Comment 2

[Alexander Opitz]

*** This bug has been marked as a duplicate of 168716 ***

Comment 3

[Serge Gautherie (:sgautherie)]

Re comment 2:

[Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.4) Gecko/20030529]

I disagree with your Resolution:
1) Not Duplicate: This bug is not related at all to bug 168716.
2) Not Resolved: In v1.4rc1, the current bug is still present !

Please, check again and reopen.


[Netscape® Communicator 4.8 : en-20020722]

(Not a (K) '4xp': v4.8 does not support multiple selection.)

Comment 4

[Daniel Wang]

reopen

Comment 5

[Erik Fabert]

Taking, seems easy enough.

Comment 6

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.5) Gecko/20041217]
Bug still there.

Erik,
Do you plan to disable the buttons, or make the multiple selection move ?

Comment 7

[Erik Fabert]

I plan to make all selected items move. It's already working, I just have a
problem with the XUL listbox not displaying its contents right if more than one
of its children get moved. 

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to comment #7)
> I plan to make all selected items move. It's already working, I just have a
> problem with the XUL listbox not displaying its contents right if more than one
> of its children get moved. 

Good :-)
Maybe Neil can help you on the listbox behaviour ?

Comment 9

[Erik Fabert]

Attachment: work in progress

Here is a preview of the patch if you are interested. Maybe the problem is just
with the latest nightly build. I've applied this patch to Mozilla 1.8a6; the
listbox behaves as expected.

Comment 10

[[e-mail]]

Would it perhaps be easier to move the unselected item the other way?

e.g. instead of moving item 2 (selected) down to 3, move item 3 up.

Comment 11

[Serge Gautherie (:sgautherie)]

(In reply to comment #9)
> Created an attachment (id=173083) [edit]
> work in progress
> 
> Here is a preview of the patch if you are interested. Maybe the problem is just
> with the latest nightly build. I've applied this patch to Mozilla 1.8a6; the
> listbox behaves as expected.

I'll test it in my v1.8a5 build (later), and probably change a few code nits.

If you had time, it could be interesting to find out what and when the v1.8b-xx
"broke or changed" ... possibly leading to a separate bug report, or a patch
update !?

Comment 12

[[e-mail]]

Also, a reduced test case (i.e. a standalone XUL file) would be helpful.

Comment 13

[Erik Fabert]

Attachment: testcase for the listbox problem

This fails in 1.8b, 2005013106, WinXP, works in 1.8a6.

Comment 14

[Erik Fabert]

Ok, it wasn't just a bad nightly, I filed bug 280754 on this issue.

Comment 15

[Serge Gautherie (:sgautherie)]

Attachment: (Av2-MAS_Bw) <pref-languages.*>

Previous patch, fixed and more.

I wonder why the |sort()| step is needed, but it is to get MouveDown working
right :-|

I still have an little issue with |ensureElementIsVisible()|: it always applies
to the top selected item, whereas I'd like the bottom selected item when
MoveDown :-/

Comment 16

[Serge Gautherie (:sgautherie)]

(In reply to comment #10)
> Would it perhaps be easier to move the unselected item the other way?
> e.g. instead of moving item 2 (selected) down to 3, move item 3 up.

I forgot:
I have yet to figure out what you had in mind (= how to code that easier way) :-(

Comment 17

[Erik Fabert]

Serge, please note that I have another patch for pref-languages in bug 56680
that will clash with this one. IMHO this should not be checked in before Listbox
is fixed. 

Here is why the sort is needed: Say your active languages are English, French
and Dutch and you want to move French and Dutch up. If you select Dutch first,
then French, selectedItems[0] is Dutch, selectedItems[1] is French. Without the
sort you first move Dutch before French, then in the next step you move French
back before Dutch because Dutch is now French's previousSibling.

Comment 18

[Serge Gautherie (:sgautherie)]

(In reply to comment #17)
> Serge, please note that I have another patch for pref-languages in bug 56680
> that will clash with this one. IMHO this should not be checked in before Listbox
> is fixed. 

Agreed: my plan could be to check this in, but with a trivial hack to disable
multiple item moves in the meantime.

> Here is why the sort is needed: Say your active languages are English, French
> and Dutch and you want to move French and Dutch up. If you select Dutch first,
> then French, selectedItems[0] is Dutch, selectedItems[1] is French. Without the
> sort you first move Dutch before French, then in the next step you move French
> back before Dutch because Dutch is now French's previousSibling.

Right, I had +/- seen this, but I was too tired to get a clear thought about it :->

Would you know of a simple way to get the "unselected" item list, which would
already be sorted ?
Neil: On the other hand, even if not that important, we can expect the selected
list to be much shorter than the unselected one, so what would be the improvment ?

Comment 19

[[e-mail]]

You misunderstood... once you've found the selected item, instead of removing it
from the list and reinserting it in its new position, remove the unselected
element from that position and reinsert that in the selected item's position.
i.e. A B [C] D > A [C] D > A [C] B D instead of A B [C] D > A B D > A [C] B D

Comment 20

[Serge Gautherie (:sgautherie)]

Attachment: (Av3-MAS_Bw) <pref-languages.*>

Av3-MAS_Bw, with comment 19 suggestion(s),
and a little more.

Comment 21

[Robert Kaiser]

MASS-CHANGE:
This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state.

If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way.
If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar).
If no action happens within the next few months, we move this bug report to an EXPIRED state.

Query tag for this change: mass-UNCONFIRM-20090614

Comment 22

[Ian Neal]

Still an issue, though marking as an enhancement because a multiple selection does now disable the move buttons in the new prefs window.

Comment 23

[Robert Kaiser]

Assigning to Serge as he has an open patch here.

Comment 24

[Thomas D. (:thomas8)]

Comment on attachment 173508 [details] [diff] [review]
(Av3-MAS_Bw) <pref-languages.*>

The current patch is from 2005 so most likely, it's obsolete and just sitting on Neil's review queue [1] in vain. With a perspective of allowing Neil to focus on current bugs, I'll remove the review flag. Pls re-request review after ensuring patch is still applicable and needed. Thanks.

[1] https://bugzilla.mozilla.org/request.cgi?action=queue&type=review&requestee=neil%40httl.net&group=type