Bug 338696 - [Table] Virtual table sorting causes selection problems
Summary: [Table] Virtual table sorting causes selection problems
Status: REOPENED
Alias: None
Product: RAP
Classification: RT
Component: RWT (show other bugs)
Version: 1.3   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 1.4 M7   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-02 11:06 EST by Michael Haendel CLA
Modified: 2013-02-20 02:01 EST (History)
6 users (show)

See Also:


Attachments
Proposed patch (2.23 KB, patch)
2011-04-14 10:11 EDT, Ivan Furnadjiev CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Haendel CLA 2011-03-02 11:06:56 EST
Build Identifier: 1.4

There are some issues with the selection of items when sorting a TABLE with the style VIRTUAL:
1. If I do not change the selection, the table keeps the item indice selected instead of the item, allthough I would expect that the table keeps the selected item (as it does in normal tables).
2. When I select the new indice of the previously selected item (= attempt to keep the item selected), the selected new indice is selected correctly, but the previous indice as well. 

To me it seems to be a problem with clearing the selection of the client-side table. The table object on the server reports only one selected item, while there are 2 items selected on the client.

You can reproduce this bug with RAP 1.3.2 and 1.4M5

Cheers,
Michael.

Reproducible: Always

Steps to Reproduce:
A self running snip is attached. Simply start the application, select an item and use the colum header for sorting.

Self running snip:

public class WidgetTest implements IEntryPoint {
	static int COUNT = 10000;
	static String[] itemStrings = new String[COUNT];
	static {
		for (int i = 0; i < COUNT; i++) {
			itemStrings[i] = "item " + i;
		}

	}
	public int createUI() {
		
		Listener sortListener = new Listener() {
			public void handleEvent(Event e) {
				// determine new sort column and direction
				TableColumn currentColumn = (TableColumn) e.widget;
				Table table = currentColumn.getParent();
				TableItem[] selItems = table.getSelection();
				String sel[] = new String[selItems.length];
				for (int i = 0; i < selItems.length; i++) {
					sel[i] = selItems[i].getText();
				}
				table.deselectAll();
				TableColumn sortColumn = table.getSortColumn();
				int dir = table.getSortDirection();
				if (sortColumn == currentColumn) {
					dir = dir == SWT.UP ? SWT.DOWN : SWT.UP;
				} else {
					table.setSortColumn(currentColumn);
					dir = SWT.UP;
				}
				// sort the data based on column and direction
				final int direction = dir;
				Arrays.sort(itemStrings, new Comparator<String>() {
					public int compare(String arg0, String arg1) {
						String a = arg0;
						String b = arg1;
						if (a.equals(b))
							return 0;
						if (direction == SWT.UP) {
							return a.compareTo(b);
						}
						return b.compareTo(a);
					}
				});
				// update data displayed in table
				table.setSortDirection(dir);

				table.clearAll();
	            if (sel != null){
	            	int[] selInd = new int[sel.length];
	            	// determine new position of previously selected items
	            	for (int s = 0; s < sel.length; s++){
	            		for (int i = 0; i < itemStrings.length; i++){
		            		String selStr = sel[s];
		            		String listStr = itemStrings[i];
		            		if (selStr.equals(listStr)){
		            			selInd[s] = i;
		            		}
		            	}
	            	}
	            	table.setSelection(selInd);
	            }
			}
		};

		Display display = new Display();
		Shell shell = new Shell(display, SWT.NO_TRIM);
		shell.setLayout(new FillLayout());
		shell.setSize(800, 600);
		final Table table = new Table(shell, SWT.BORDER | SWT.VIRTUAL);
		table.setHeaderVisible(true);
		for (int i = 0; i < 1; i++) {
			TableColumn col = new TableColumn(table, SWT.DEFAULT);
			col.addListener(SWT.Selection, sortListener);
			col.setText("Item Nr");
			col.setWidth(200);
		}

		table.setSortColumn(table.getColumn(0));
	    table.setSortDirection(SWT.UP);

		table.addListener(SWT.SetData, new Listener() {
			public void handleEvent(Event event) {
				TableItem item = (TableItem) event.item;
				int index = event.index;
				item.setText(itemStrings[index]);
			}
		});
		table.setItemCount(COUNT);
		shell.open();
		while (!shell.isDisposed()) {
			if (!display.readAndDispatch()) {
				display.sleep();
			}
		}
		return 0;
	}
}
Comment 1 Ivan Furnadjiev CLA 2011-03-10 05:11:36 EST
I can reproduce with CVS HEAD and your snippet too.
Comment 2 Ivan Furnadjiev CLA 2011-04-14 10:11:15 EDT
Created attachment 193257 [details]
Proposed patch

We have the following scenario:
1. Virtual table
2. item( 0 ) visible, materialized and selected
3. on the server-side we call:
  table.clearAll(); // clear the item, make it not materialized
  table.deselectAll(); // remove the selection from the item
4. In TableItemLCA#renderChanges there is a special handling if item wasCleared, but here the changed selection was not written to the client. Note that the "selection" is not a property of TableItem that materialize it.
5. At the end we run into selected item on the client, but deselected on the server.
The solution in this patch is to call writeSelection after writeClear to ensure that client and server have the selection in sync. JUnit test included. Tested both with single and multi selection. Michael, is the attached patch fix your problem?
Comment 3 Michael Haendel CLA 2011-04-14 11:37:46 EDT
Ivan, the solution sounds reasonable but I can't test the patch as I'm using the pre-packaged JARs in my environment
Comment 4 Ivan Furnadjiev CLA 2011-04-14 12:47:47 EDT
Applied patch to CVS HEAD.
Comment 5 Ivan Furnadjiev CLA 2011-10-03 12:55:50 EDT
Seems like the problem is back. Reproducible with Controls Demo -> TableVewer Tab:
1. Check VIRTUAL
2. Click "Add 300 items"
3. Select an item - Sofia for example
4. Click on the Table column header "First Name" - selection may change, if not click once again and selected item is changed.
Comment 6 Stephan Leicht Vogt CLA 2011-10-05 11:37:15 EDT
I have also strange problems. If I change the content from the tree twice and select a TreeItem the method getItem(Point) does not work anymore. It seems my problem lies somewhere between topItemIndex and flatItemIndex. These members where included for bug 342978. 
I do not have my Table with style VIRTUAL.

Perhaps this helps to pinpoint the problem.
Comment 7 Ivan Furnadjiev CLA 2011-10-06 03:30:34 EDT
(In reply to comment #6)
Stephan, this bug is about virtual Table, not Tree. If you are experiencing problems with Tree selection, please file a separate bug.
Comment 8 juergen.panser CLA 2011-11-17 08:51:22 EST
There seems to be another bug not concerning the sorting.

The client side selection indices are wrong.

1.) Create a virtual table with style SWT.MULTI 
2.) Fill the table with 50.000 items
3.) Select some items on the first page e.g. item 5 and 10. When debugging the TableLCA.readSelection(final Table table) the selectedIndices are {4, 9 } (which is correct).
4.) Scroll down to the end of the table. On IE 8 with 30 visible table items / lines a message like "A script on this page is causing your web browser to run slowly..." pops up. Answer it with No to continue the script (MSDN states that this message appears when more than 5.000.000 million calls were made - is this correct? Do we need this amount of calls to scroll down a bit?).
5.) Additionally select the last item. SelectedIndices are {4, 9, 49999 } (which is correct).
6.) Scroll up some pages and add another item to the selection e.g. item 49500. SelectedIndices should be {4, 9, 49999, 49499 }, but all items beyond the newly selected one have been shifted: the SelectedIndices are {4, 9, 50061, 49499 }.
7.) Scroll up and select an item. SelectedIndices are {4, 9, 50091, 49529 }. Now select the second item.  SelectedIndices are {4, 9, 50095, 49534 }. Select the next item near the top index the later selections shift again by 4 (or sometimes 3).
8.) Scroll back to the first page and additionally select some items. Meanwhile the SelectedIndices was growing and now holds many more items than you have really selected (about 180 indices after maybe 20 selections).
9.) Sometimes (not reliably reproducable) now calling table.setSelection() on the server doesn't work properly. Instead of setting a completely new selection (e.g. 1 selected index expected) it adds the selection to the exising ones. The corrupt selectedIndices of the client are not overwritten (e.g. we now have 181 indices). But as mentioned - this is not always reproducable.

The shifting of the selection indices results in wrong selections!

The shifting of the selection indices also occurs with style SWT.SINGLE.

So the selection feature can't be really used in a virtual table!
Comment 9 juergen.panser CLA 2011-11-17 08:53:22 EST
Sorry, missed to mention that we use RAP 1.4
Comment 10 Tim Buschtoens CLA 2012-02-23 12:30:42 EST
(In reply to comment #5)
> Seems like the problem is back. Reproducible with Controls Demo -> TableVewer

Can you tell if its a server or client issue?
Comment 11 Ivan Furnadjiev CLA 2012-03-29 05:01:49 EDT
Today I tested our Controls Demo -> TableViewer Tab against SWT (JFace) and it behaves the same there. If you select (with a mouse) item(s) from the bottom half of the Table, than the selection jumps (changes) after the sorting. Probably a bug in JFace. More over, I tested Snippet192 from SWT snippets [1]
and it's working fine in RAP.

[1] http://git.eclipse.org/c/platform/eclipse.platform.swt.git/tree/examples/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet192.java
Comment 12 Wolfgang Pedot CLA 2012-12-13 10:31:03 EST
There also appears to be a related issue when using filters. We have a view with a TableViewer that is part of more than one perspective. In each perspective there is a (potentially) different set of filters applied to this TableViewer. Now when I select the item in the second line in the table and switch the perspective the selection stays on the second line most of the time even if that item is now different because of the new filter. 

There are two cases where the behavior is as expected:

1.) If the currently selected item is not visible in the new perspective the selection goes away.

2.) If the currently selected line does not exist in the new perspective (eg. 10. line selected but only 5 visible lines with new filter) but the selected ITEM is visible it is correctly selected.

Unfortunately these are rare cases, most of the time the selected item is still visible but its in another line and therefor the selection changes.

Any hope of fixing this?
Comment 13 Nicholas Hinds CLA 2013-02-04 18:24:58 EST
A workaround which appears to work is to override setSelectionToWidget(ISelection, boolean) from TableViewer to call internalRefresh(getRoot()) before calling the super impl. This ends up calling AbstractTableViewer.internalVirtualRefreshAll(), which updates the cached elements before the selection is made so the item index is calculated correctly.

This refresh could be quite expensive, so it's probably worth only calling it when the new selection actually differs from the current selection.
Comment 14 Uzma Pathan CLA 2013-02-19 08:25:56 EST
(In reply to comment #13)
> A workaround which appears to work is to override
> setSelectionToWidget(ISelection, boolean) from TableViewer to call
> internalRefresh(getRoot()) before calling the super impl. This ends up
> calling AbstractTableViewer.internalVirtualRefreshAll(), which updates the
> cached elements before the selection is made so the item index is calculated
> correctly.
> 
> This refresh could be quite expensive, so it's probably worth only calling
> it when the new selection actually differs from the current selection.


Hi Nicholas,

I have tried to implement the workaround suggested by you. However it doesn't solve problem.
I tried to go into debug and found out that the cache from the AbstractTableViewer.VirtualManager class seems to be up to date with the model. Forcing the cache to be used in the abstractTableViewer.virtualSetSelectionToWidget() can be a possible approach.

Please suggest some other alternative work around if any.

Thanks,
Uzma Pathan
Comment 15 Nicholas Hinds CLA 2013-02-20 01:12:26 EST
(In reply to comment #14)
> Hi Nicholas,
> 
> I have tried to implement the workaround suggested by you. However it
> doesn't solve problem.
> I tried to go into debug and found out that the cache from the
> AbstractTableViewer.VirtualManager class seems to be up to date with the
> model. Forcing the cache to be used in the
> abstractTableViewer.virtualSetSelectionToWidget() can be a possible approach.
> 
> Please suggest some other alternative work around if any.
> 
> Thanks,
> Uzma Pathan

You might be experiencing a different variation of this. I have only encountered this using TableViewers with the SWT.VIRTUAL flag with content providers implementing IStructuredContentProvider and not ILazyContentProvider. The workaround seems to be suitable for this case, but since it works for me I have not investigated any other workarounds, sorry.
Comment 16 Uzma Pathan CLA 2013-02-20 01:22:19 EST
(In reply to comment #15)
> (In reply to comment #14)
> > Hi Nicholas,
> > 
> > I have tried to implement the workaround suggested by you. However it
> > doesn't solve problem.
> > I tried to go into debug and found out that the cache from the
> > AbstractTableViewer.VirtualManager class seems to be up to date with the
> > model. Forcing the cache to be used in the
> > abstractTableViewer.virtualSetSelectionToWidget() can be a possible approach.
> > 
> > Please suggest some other alternative work around if any.
> > 
> > Thanks,
> > Uzma Pathan
> 
> You might be experiencing a different variation of this. I have only
> encountered this using TableViewers with the SWT.VIRTUAL flag with content
> providers implementing IStructuredContentProvider and not
> ILazyContentProvider. The workaround seems to be suitable for this case, but
> since it works for me I have not investigated any other workarounds, sorry.


Hi Nicholas,

Thanks for your reply. 
Actually I'm using TableViewer with the SWT.VIRTUAL flag with content providers implementing ILazyContentProvider. Maybe the work around suggested by you doesn't work for ILazyContentProvider. 

Someone please suggest some pointers if any.

Thanks in Advance,
Uzma Pathan
Comment 17 Uzma Pathan CLA 2013-02-20 02:01:37 EST
(In reply to comment #16)
> (In reply to comment #15)
> > (In reply to comment #14)
> > > Hi Nicholas,
> > > 
> > > I have tried to implement the workaround suggested by you. However it
> > > doesn't solve problem.
> > > I tried to go into debug and found out that the cache from the
> > > AbstractTableViewer.VirtualManager class seems to be up to date with the
> > > model. Forcing the cache to be used in the
> > > abstractTableViewer.virtualSetSelectionToWidget() can be a possible approach.
> > > 
> > > Please suggest some other alternative work around if any.
> > > 
> > > Thanks,
> > > Uzma Pathan
> > 
> > You might be experiencing a different variation of this. I have only
> > encountered this using TableViewers with the SWT.VIRTUAL flag with content
> > providers implementing IStructuredContentProvider and not
> > ILazyContentProvider. The workaround seems to be suitable for this case, but
> > since it works for me I have not investigated any other workarounds, sorry.
> 
> 
> Hi Nicholas,
> 
> Thanks for your reply. 
> Actually I'm using TableViewer with the SWT.VIRTUAL flag with content
> providers implementing ILazyContentProvider. Maybe the work around suggested
> by you doesn't work for ILazyContentProvider. 
> 
> Someone please suggest some pointers if any.
> 
> Thanks in Advance,
> Uzma Pathan

Hi Nicholas,

I made changes in my code and now I have TableViewer with the SWT.VIRTUAL flag with content providers implementing IStructuredContentProvider. I implemented the workaround suggested by you. However it still doesn't solve problem.
Please provide some alternative workaround if any.

Thanks,
Uzma Pathan.