Modify

Opened 12 months ago

Last modified 9 months ago

#17906 new defect

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

Reported by: taylor.smock Owned by: team
Priority: major Milestone:
Component: Core Version: tested
Keywords: relation member dnd drag-and-drop Cc: simon04

Description

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

Attachments (0)

Change History (12)

comment:1 Changed 12 months ago by Don-vip

Summary: When dragging and dropping relations to the same position, they are removedWhen dragging and dropping relation members to the same position, they are removed

comment:2 Changed 12 months ago by pl71

confirmed

comment:3 Changed 12 months ago by 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?

comment:4 Changed 12 months ago by Don-vip

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

comment:5 Changed 10 months ago by Don-vip

Milestone: 19.09
Owner: changed from team to Don-vip
Priority: normalmajor
Status: newassigned

comment:6 Changed 10 months ago by Don-vip

Keywords: member added

Bug already present in r14824 so not a recent regression.

comment:7 Changed 10 months ago by Don-vip

Cc: simon04 added
Keywords: dnd drag-and-drop added

Probably here since we introduced the feature in #12300

comment:8 Changed 10 months ago by Don-vip

Owner: changed from Don-vip to team
Status: assignednew

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

comment:9 Changed 10 months ago by 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

    diff --git a/src/org/openstreetmap/josm/gui/dialogs/relation/MemberTableModel.java b/src/org/openstreetmap/josm/gui/dialogs/relation/MemberTableModel.java
    index a2e4084ee..57ee139e2 100644
    a b  
    22package org.openstreetmap.josm.gui.dialogs.relation;
    33
    44import java.util.ArrayList;
     5import java.util.Arrays;
    56import java.util.BitSet;
    67import java.util.Collection;
    78import java.util.Collections;
    RelationMember getRelationMemberForPrimitive(final OsmPrimitive primitive) { 
    447448    }
    448449
    449450    void addMembersAtIndexKeepingOldSelection(final Iterable<RelationMember> newMembers, final int index) {
     451        System.out.println(Arrays.toString(getSelectedIndices()));
    450452        int idx = index;
    451453        for (RelationMember member : newMembers) {
    452454            members.add(idx++, member);
    453455        }
    454456        invalidateConnectionType();
    455457        fireTableRowsInserted(index, idx - 1);
     458        System.out.println(Arrays.toString(getSelectedIndices()));
    456459    }
    457460
    458461    public void addMembersAtBeginning(List<? extends OsmPrimitive> primitives) {

comment:10 Changed 9 months ago by 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
        }
Last edited 9 months ago by Don-vip (previous) (diff)

comment:11 Changed 9 months ago by Don-vip

Milestone: 19.0919.10

comment:12 Changed 9 months ago by Don-vip

Milestone: 19.10

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain team.
as The resolution will be set.
to The owner will be changed from team to the specified user.
The owner will change to taylor.smock
as duplicate The resolution will be set to duplicate.The specified ticket will be cross-referenced with this ticket
The owner will be changed from team to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.