[PATCH] When dragging and dropping relation members to the same position, they are removed

[taylor.smock]

Steps to reproduce:

1. Get a relation with at least one member
2. Drag and drop any member to the same position (black bar above the currently selected element)
3. See member list decrease by one

Note: This can be done with multiple members.

This bug exists in r15224 and later (may exist earlier, I don't have enough time right now to do a binary search).

[pl71]

confirmed

[njtbusfan]

Is this the same bug that sees order get screwed up when you select a bunch of elements and move them up or down?

[Don-vip]

@njtbusfan no this one has been fixed, please update to JOSM 15238.

[Don-vip]

Bug already present in r14824 so not a recent regression.

[Don-vip]

Probably here since we introduced the feature in #12300

[Don-vip]

Mmm I debugged a little bit and don't see what's wrong. Simon, do you have an idea?

[simon04]

When dropping one element at its position in the table, strangely, the selection inside addMembersAtIndexKeepingOldSelection to both elements (the previously selected one and the inserted one). Thus, both are deleted in org.openstreetmap.josm.gui.dialogs.relation.MemberTransferHandler#exportDone.

Not sure why the selection expands. Vincent, do you have an idea.

For debugging:

src/org/openstreetmap/josm/gui/dialogs/relation/MemberTableModel.java

@@ -2 +2 @@
 package org.openstreetmap.josm.gui.dialogs.relation;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.BitSet;
 import java.util.Collection;
 import java.util.Collections;
@@ -447 +448 @@
     }
 
     void addMembersAtIndexKeepingOldSelection(final Iterable<RelationMember> newMembers, final int index) {
+        System.out.println(Arrays.toString(getSelectedIndices()));
         int idx = index;
         for (RelationMember member : newMembers) {
             members.add(idx++, member);
         }
         invalidateConnectionType();
         fireTableRowsInserted(index, idx - 1);
+        System.out.println(Arrays.toString(getSelectedIndices()));
     }
 
     public void addMembersAtBeginning(List<? extends OsmPrimitive> primitives) {

[Don-vip]

When we call fireTableRowsInserted(index, idx - 1); we execute this code of JTable:

    /*
     * Invoked when rows have been inserted into the table.
     * <p>
     * Application code will not use these methods explicitly, they
     * are used internally by JTable.
     *
     * @param e the TableModelEvent encapsulating the insertion
     */
    private void tableRowsInserted(TableModelEvent e) {
        int start = e.getFirstRow();
        int end = e.getLastRow();
        if (start < 0) {
            start = 0;
        }
        if (end < 0) {
            end = getRowCount()-1;
        }

        // Adjust the selection to account for the new rows.
        int length = end - start + 1;
        selectionModel.insertIndexInterval(start, length, true);

then in DefaultListSelectionModel

    /**
     * Insert length indices beginning before/after index. If the value
     * at index is itself selected and the selection mode is not
     * SINGLE_SELECTION, set all of the newly inserted items as selected.
     * Otherwise leave them unselected. This method is typically
     * called to sync the selection model with a corresponding change
     * in the data model.
     */
    public void insertIndexInterval(int index, int length, boolean before)
    {
        /* The first new index will appear at insMinIndex and the last
         * one will appear at insMaxIndex
         */
        int insMinIndex = (before) ? index : index + 1;
        int insMaxIndex = (insMinIndex + length) - 1;

        /* Right shift the entire bitset by length, beginning with
         * index-1 if before is true, index+1 if it's false (i.e. with
         * insMinIndex).
         */
        for(int i = maxIndex; i >= insMinIndex; i--) {
            setState(i + length, value.get(i)); // <- selection changes here
        }

[GerdP]

I cannot reproduce this with r17432. Seems it was fixed?

[taylor.smock]

Ticket #21889 has been marked as a duplicate of this ticket.

[taylor.smock]

This duplicates #12617 (fixed in r9993, fix probably accidentally removed in r10604).

I'll go ahead and add a non-regression test so it doesn't happen in the future.

[taylor.smock]

Fix deletion issue, add non-regression test, fix performance issue when moving many members

[taylor.smock]

Notes on attachment:17906.patch​:

• Fixes #17906, #21889, see #12617 for the original issue
• #12617 was partially fixed in r9993 for single selections (but not for multiple selections). Fix was probably accidentally removed in r10604.
• Improves performance when moving large numbers of relation members (e.g., if someone decides that moving 99% of ​Lake Huron's members is a good idea, it won't take <some really long time> to finish the move).

I don't think we are doing a 22.04 release, but I'll merge the patch Wednesday just in case we do decide to do a 22.04 release.

[taylor.smock]

In 18439/josm:

Fix #17906, #21889, see #12617 for the original issue

This fixes an issue where a relation member could be deleted accidentally
when a user moved the relation member to its current position. This also
affected multiple members when multiple members were selected. The user
just had to move the selected members to another selected member position.

This patch also improves relation move performance when large numbers of
relation members are moved.

[stoecker]

Milestone renamed