Bug 97786 - [Viewers] - Lazy Virtual Table View Test - duplicate items appear after delete
Summary: [Viewers] - Lazy Virtual Table View Test - duplicate items appear after delete
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.1   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.5 M3   Edit
Assignee: Boris Bokowski CLA
QA Contact:
URL:
Whiteboard: contained
Keywords: api, helpwanted
: 112411 (view as bug list)
Depends on:
Blocks: 154755
  Show dependency tree
 
Reported: 2005-06-01 01:26 EDT by Susan McCourt CLA
Modified: 2008-10-28 16:48 EDT (History)
6 users (show)

See Also:


Attachments
patch (1.78 KB, patch)
2008-09-15 15:30 EDT, Boris Bokowski CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Susan McCourt CLA 2005-06-01 01:26:34 EDT
3.1 RC1, Win XP

- open the Lazy Virtual Table View Test
- Scroll down so that element 6110 is the top item
- select item 6114 and delete it
- select item 6118 and delete it
- scroll up two items so that 6108 is the top item
- scroll back down and notice duplicate elements at item 6128 and 6129 
   6126
   6127
   6128
   6129
   6128
   6129
   6130
   6131

you might not get the exact same index for the duplicates - it may depend on 
the number of visible items, etc.  But it is definitely reproducible.
Resetting the input and trying again will show the problem, but the duplicate 
items appear later in the list (numbers 6132 and 6133 were duplicates after I 
reset input and repeated the steps).

The location of the duplicates is the same if you open/close the view.  ie. 
once you find the duplicates, you can reproduce consistently by 
opening/closing the view and following the same steps.
Comment 1 Boris Bokowski CLA 2005-11-09 15:30:47 EST
The reason for the strange behaviour is that clicking the delete button does not
update the underlying model, it only calls remove() on the viewer.

Comment 2 Boris Bokowski CLA 2005-11-18 13:23:11 EST
While trying to fix the Lazy Virtual Table View Test I noticed a bug in
TableViewer.remove. In the VIRTUAL case, preservingSelection() may end up
calling the content provider for an index that no longer exists, potentially
causing an AIOOBE.
Comment 3 Boris Bokowski CLA 2005-12-08 17:52:24 EST
*** Bug 112411 has been marked as a duplicate of this bug. ***
Comment 4 Boris Bokowski CLA 2005-12-10 00:14:00 EST
Sorry, I ran out of time for M4 - moving to M5.

One solution for TableViewer would be to not use preservingSelection() if we are removing elements from a virtual table. Instead, we could remember the selected indices, update them based on the indices of the elements being removed, and then setting the selection back using the updated indices.

There will be a similar problem for the virtual tree, and the index-based magic would be way more complicated for trees. I don't know what the best answer is.
Comment 5 Boris Bokowski CLA 2006-01-19 15:46:09 EST
Another solution would be to introduce a new method that is called whenever we need an updated item count.

I'm considering deprecating ILazyContentProvider and introducing a replacement (possibly IVirtualTableContentProvider) with a new method as follows:

/**
 * Called when there is a chance that the item count needs to
 * be updated, namely from {@link TableViewer#refresh()} and
 * {@link TableViewer#setInput(Object)}. The content provider
 * should respond by calling {@link TableViewer#setItemCount(int)}.
 */
public void updateItemCount();
Comment 6 Boris Bokowski CLA 2006-02-13 22:05:45 EST
Sorry, I ran out of time again.
Comment 7 Boris Bokowski CLA 2006-04-12 14:25:47 EDT
I don't know why, but higher priority items seem to prevent me from fixing this.
Comment 8 Thomas Schindl CLA 2006-05-31 06:09:10 EDT
Where from do I get the mentionned test cases? Are they somewhere in CVS?

(In reply to comment #7)
> I don't know why, but higher priority items seem to prevent me from fixing
> this.
> 

Comment 9 Sergey Prigogin CLA 2006-05-31 10:10:45 EDT
Bug 112411 has an attached test case.
Comment 10 Susan McCourt CLA 2006-05-31 11:43:35 EDT
The LazyVirtualTableView test class can be found in CVS in the plug-in org.eclipse.ui.tests.  To run the test cases you will also need the plug-ins org.eclipse.core.tests.boot, org.eclipse.core.tests.harness, and org.eclipse.test.performance.
Comment 11 Thomas Schindl CLA 2006-06-01 15:16:03 EDT
(In reply to comment #10)
> The LazyVirtualTableView test class can be found in CVS in the plug-in
> org.eclipse.ui.tests.  To run the test cases you will also need the plug-ins
> org.eclipse.core.tests.boot, org.eclipse.core.tests.harness, and
> org.eclipse.test.performance.
> 

Ok. I've got them and try to run the "JUnit"-Test but they fail with "Unsatisfied LinkError" do I need to set the swt.dll myself when start those test.
Comment 12 Boris Bokowski CLA 2006-06-01 15:35:03 EDT
Run As->JUnit Plug-in Test
Comment 13 Thomas Schindl CLA 2006-06-01 15:42:35 EDT
(In reply to comment #12)
> Run As->JUnit Plug-in Test
> 

Thanks figured it out myself. But the test I'm trying too launch is one of the interactive ones there's no Run As->JUnit Plug-in Test|Junit I needed to start the workbench-plugin and select the view ;-) Thanks Tom
Comment 14 Boris Bokowski CLA 2008-05-02 14:56:30 EDT
Mass update - removing 3.4 target. This was one of the bugs I marked for investigation (and potential fixing) in 3.4 but I ran out of time. Please ping on the bug if fixing it would be really important for 3.4, and does not require API changes or feature work.
Comment 15 David Pérez CLA 2008-08-27 07:58:19 EDT
Is there any plan to solve such an old bug in such a basic GUI class as TableViewer for such a usual operation as delete an item?

Is there any known workaround?

I have created a CRUD editor based on TableViewer, and I observe this bug nearly always I try to delete an item.
Comment 16 Boris Bokowski CLA 2008-08-28 13:10:00 EDT
I believe the workaround is to empty the selection before you delete the element. I can mark this for investigation in 3.5 but am not sure if I will have the time if there is no easy fix.
Comment 17 David Pérez CLA 2008-09-08 06:41:52 EDT
Confirmed that this workaround works!
Until the bug isn't solved, it would be good to unselect in the Viewer.remove() method.

My CRUD framework based on JFace/SWT is working quite well.  :-)
Comment 18 Boris Bokowski CLA 2008-09-15 15:30:08 EDT
Created attachment 112591 [details]
patch
Comment 19 Boris Bokowski CLA 2008-09-15 15:31:02 EDT
I would like to avoid releasing this patch so close to M2. Moving to M3.
Comment 20 Boris Bokowski CLA 2008-10-26 09:40:05 EDT
Released to HEAD.
Comment 21 Boris Bokowski CLA 2008-10-28 16:48:01 EDT
Verified by code inspection using I20081028-0100, and by using the steps from comment 0.