Comment 1 Sergey Prigogin 2005-10-12 18:20:56 EDT

Created attachment 28213 [details]
Test case

Comment 2 Steve Northover 2005-10-14 13:23:44 EDT

Of course this WORKSFORME in what I am running (the JFace used to test 99266 
and my work in progress Table code).  I will try and recreate it in I20051012-
1200.

Comment 3 Steve Northover 2005-10-14 13:26:05 EDT

Can you give me some exact steps? (ie. select 'Name 220', press 'Delete', ... 
etc)

Comment 4 Sergey Prigogin 2005-10-14 14:10:58 EDT

The only thing you have to do is click Remove button. The rows that have to be 
selected are already selected programmatically.

Comment 5 Billy Biggs 2005-10-14 14:55:17 EDT

Like Steve, I was at first confused by the test case.  As it turns out, no rows
are selected for me when running under an older version of JFace.  I updated and
was easily able to reproduce the problem.

The problem goes away if I comment out the preservingSelection() part of
remove() in TableViewer and just call internalRemove().  Moving to UI first to
see if this is a bug in StructuredViewer.preservingSelection().

Comment 6 Sergey Prigogin 2005-10-26 16:20:10 EDT

This test case can be easily changed to eliminate any human interaction. It 
makes sense to add it to an automatic test that runs with every build.

Comment 7 Boris Bokowski 2005-10-26 17:43:58 EDT

At least part of the problem is that you are not updating your model (i.e. the
objects array used by the lazy content provider).

I don't think this is a bug in SWT.

Here is an interesting experiment: Comment out the preservingSelection part of
TableViewer.remove(), but add a call to tableViewer.refresh() after calling
tableViewer.remove(). Again, it appears as if the last four rows have been
deleted, but really this is what happened: TableViewer and the SWT Table
correctly deleted rows 80 to 83, but after that, the table refreshes and asks
the content provider (index based) for the current elements, which include the
elements that you just tried to remove, and hence they reappear.

I tried to fix your example, but failed, so I think there really is a problem
with TableViewer, but it's not as simple as it first seemed.

Changing the target milestone to M4 since I probably won't be able to work on
this before M3 is released.

Comment 8 Boris Bokowski 2005-12-08 17:52:24 EST

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

Comment 9 Thomas Schindl 2006-06-01 15:07:34 EDT

(In reply to comment #5)
> Like Steve, I was at first confused by the test case.  As it turns out, no rows
> are selected for me when running under an older version of JFace.  I updated and
> was easily able to reproduce the problem.
> 
> The problem goes away if I comment out the preservingSelection() part of
> remove() in TableViewer and just call internalRemove().  Moving to UI first to
> see if this is a bug in StructuredViewer.preservingSelection().

Yeah you are completely right the problem in this case is the ILazyContentProvider which has to take into consideration that the index is shifted by 4 in case of removal.

That's why a working example looks like this:
-------------------------8<-------------------------
/*******************************************************************************
 * Copyright (c) 2005 Sigma Dynamics, Inc. All Rights Reserved.
 * This program and is made available under the terms of the Eclipse Public License v1.0
 * which is available at http://www.eclipse.org/legal/epl-v10.html
 *
 * Contributors:
 *     Sigma Dynamics, Inc - initial API and implementation
 *******************************************************************************/
import java.util.ArrayList;

import org.eclipse.jface.viewers.CellEditor;
import org.eclipse.jface.viewers.ColumnPixelData;
import org.eclipse.jface.viewers.ColumnWeightData;
import org.eclipse.jface.viewers.DialogCellEditor;
import org.eclipse.jface.viewers.ICellModifier;
import org.eclipse.jface.viewers.ILazyContentProvider;
import org.eclipse.jface.viewers.IStructuredSelection;
import org.eclipse.jface.viewers.ITableLabelProvider;
import org.eclipse.jface.viewers.LabelProvider;
import org.eclipse.jface.viewers.StructuredSelection;
import org.eclipse.jface.viewers.TableViewer;
import org.eclipse.jface.viewers.TextCellEditor;
import org.eclipse.jface.viewers.Viewer;
import org.eclipse.jface.viewers.ViewerFilter;
import org.eclipse.jface.viewers.ViewerSorter;
import org.eclipse.swt.SWT;
import org.eclipse.swt.events.SelectionAdapter;
import org.eclipse.swt.events.SelectionEvent;
import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.layout.GridData;
import org.eclipse.swt.layout.GridLayout;
import org.eclipse.swt.widgets.Button;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Control;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Event;
import org.eclipse.swt.widgets.Label;
import org.eclipse.swt.widgets.Listener;
import org.eclipse.swt.widgets.MessageBox;
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swt.widgets.Table;
import org.eclipse.swt.widgets.TableColumn;
import org.eclipse.swt.widgets.TableItem;
import org.eclipse.swt.widgets.Text;

public class WrongRowsAreDeleted {
  private static final int NUMBER_OF_ROWS = 220;
  private static int cnt = 20;
  public static int indexShift = 0;
  public static RowObj[] objects;

  public static void main(String[] args) {
    System.out.println(SWT.getVersion());
    Display display = new Display();
    Shell shell = new Shell(display);
    shell.setLayout(new GridLayout());
    final TableViewer tableViewer = new TableViewer(shell, SWT.VIRTUAL | SWT.BORDER | SWT.FULL_SELECTION
        | SWT.MULTI | SWT.H_SCROLL | SWT.V_SCROLL);
    final Table table = tableViewer.getTable();
    table.setLinesVisible(true);
    table.setHeaderVisible(true);
    final GridData gridData = new GridData(GridData.FILL_HORIZONTAL);
    gridData.widthHint = 200;
    gridData.heightHint = 300;
    table.setLayoutData(gridData);
    table.setHeaderVisible(true);
    DynamicTableLayout tableLayout = new DynamicTableLayout();
    table.setLayout(tableLayout);
    {
      final TableColumn tableColumn = new TableColumn(table, SWT.NONE);
      tableColumn.setText("Name");
      tableColumn.setWidth(100);
      tableLayout.addColumnData(new ColumnPixelData(100));
    }
    {
      final TableColumn tableColumn = new TableColumn(table, SWT.NONE);
      tableColumn.setText("Description");
      tableColumn.setWidth(100);
      tableLayout.addColumnData(new ColumnWeightData(100));
    }

    objects = new RowObj[NUMBER_OF_ROWS];
    for (int i = 0; i < objects.length; i++) {
      int x = NUMBER_OF_ROWS - i;
      objects[i] = new RowObj("Name " + x, "Description " + x);
    }

    tableViewer.setUseHashlookup(true);
    tableViewer.setColumnProperties(new String[]{"0", "1"});

    tableViewer.setLabelProvider(new TableLabelProvider());

    CellEditor cellEditor = new TestCellEditorD(table);

    tableViewer.setCellEditors(new CellEditor[]{null, cellEditor});
    tableViewer.setCellModifier(new ICellModifier() {
      public boolean canModify(Object element, String property) {
        return "1".equals(property);
      }

      public Object getValue(Object element, String property) {
        RowObj obj = getObj(element);
        if ("0".equals(property)) {
          return obj.name;
        } else {
          return obj.description;
        }
      }

      public void modify(Object element, String property, Object value) {
        RowObj obj = getObj(element);
        if ("0".equals(property)) {
          obj.name = (String) value;
        } else {
          obj.description = (String) value;
        }
      }

      private RowObj getObj(Object element) {
        RowObj obj = (element instanceof TableItem)
            ? (RowObj) ((TableItem) element).getData()
            : (RowObj) element;
        return obj;
      }
    });

    tableViewer.setContentProvider(new ILazyContentProvider() {
      public void inputChanged(Viewer viewer, Object oldInput, Object newInput) {
      }

      public void dispose() {
      }

      public void updateElement(int index) {
    	  if( index + indexShift < objects.length  ) {
    		  tableViewer.replace(objects[index+indexShift], index);
    	  }
      }
    });
    // tableViewer.setContentProvider(new ArrayContentProvider());
    tableViewer.setSorter(new TableColumnSorter());

    tableViewer.setItemCount(objects.length);
    // tableViewer.setInput(objects);

    ArrayList s = new ArrayList();
    s.add(objects[80]);
    s.add(objects[81]);
    s.add(objects[82]);
    s.add(objects[83]);

    tableViewer.setSelection(new StructuredSelection(s), true);
    
    Text text = new Text(shell, SWT.BORDER);
    Label label = new Label(shell, SWT.NULL);
    label.setText("Click Remove button and notice that wrong rows were deleted.");
    Button button = new Button(shell, SWT.NONE);
    button.setText("Remove Rows");

    
    button.addSelectionListener(new SelectionAdapter() {
      public void widgetSelected(SelectionEvent e) {
    	  
    	  IStructuredSelection selection = (IStructuredSelection) tableViewer.getSelection();
        Object[] objs = selection.toArray();
        indexShift += objs.length;
        tableViewer.remove(objs);
    	//tableViewer.getTable().remove(new int[] {80,81,82,83} );
      }
    });

    shell.open();
    while (!shell.isDisposed()) {
      if (!display.readAndDispatch())
        display.sleep();
    }
    display.dispose();
  }

  static public class RowObj {
    String name;
    String description;

    public RowObj(String name, String description) {
      this.name = name;
      this.description = description;
    }
  }

  static class TableLabelProvider extends LabelProvider implements ITableLabelProvider {
    public String getColumnText(Object element, int columnIndex) {
      RowObj obj = (RowObj) element;
      return columnIndex == 0 ? obj.name : obj.description;
    }

    public Image getColumnImage(Object element, int columnIndex) {
      return null;
    }
  }

  static class TableColumnSorter extends ViewerSorter {
    public int compare(Viewer viewer, Object o1, Object o2) {
      RowObj t1 = (RowObj) o1;
      RowObj t2 = (RowObj) o2;
      return t1.name.compareTo(t2.name);
    }
  }

  static class TableFilter extends ViewerFilter {
    String search;

    public TableFilter(String search) {
      this.search = search;
    }

    public boolean select(Viewer viewer, Object parent, Object element) {
      RowObj ro = (RowObj) element;
      return (ro.name.indexOf(search) != -1);
    }

  }

  private static class TestCellEditorD extends DialogCellEditor {
    protected Control createControl(Composite parent) {
      Control control = super.createControl(parent);
      control.addListener(SWT.Deactivate, new Listener() {
        public void handleEvent(Event event) {
          fireApplyEditorValue();
        }
      });
      return control;
    }

    public TestCellEditorD() {
      super();
    }

    public TestCellEditorD(Composite parent, int style) {
      super(parent, style);
    }

    public TestCellEditorD(Composite parent) {
      super(parent);
    }

    protected void doSetValue(Object value) {
      System.out.println("doSetValue");
      super.doSetValue(value);
    }

    protected void focusLost() {
      System.out.println("focusLost");
      super.focusLost();
    }

    protected Object openDialogBox(Control cellEditorWindow) {
      MessageBox dialog = new MessageBox(cellEditorWindow.getShell());
      dialog.open();
      return null;
    }
  }

  private static class TestCellEditorT extends TextCellEditor {
    public TestCellEditorT() {
      super();
    }

    public TestCellEditorT(Composite parent, int style) {
      super(parent, style);
    }

    public TestCellEditorT(Composite parent) {
      super(parent);
    }

    protected void doSetValue(Object value) {
      System.out.println("doSetValue");
      super.doSetValue(value);
    }

    protected void focusLost() {
      System.out.println("focusLost");
      super.focusLost();
    }
  }
}
-------------------------8<-------------------------

As you see I move the index by the amount of items removed which reveals another problem that the itemCount is not updated when items are removed hence the last 4 rows are empty. If the viewer is not scrolled to the end until the items are removed.