Bug 219887 - [Viewers] ISelectionProvider events not triggered by TableViewer.setItemCount()
Summary: [Viewers] ISelectionProvider events not triggered by TableViewer.setItemCount()
Status: ASSIGNED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3.1   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Platform UI Triaged CLA
QA Contact:
URL:
Whiteboard:
Keywords: helpwanted
Depends on:
Blocks:
 
Reported: 2008-02-21 18:40 EST by Peter Centgraf CLA
Modified: 2019-09-06 16:16 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Centgraf CLA 2008-02-21 18:40:56 EST
Tested under 3.3.1.1:

When using an SWT.VIRTUAL style Table with a TableViewer and ILazyContentProvider, the usage pattern recommended in the JavaDoc is for the content provider to call TableViewer.setItemCount() during updateElement(), when necessary.  This sequence is triggered by TableViewer.setInput() and TableViewer.refresh().

If the TableViewer has selected elements, and one or more of these elements is no longer present after the refresh(), the TableViewer should fire ISelectionChangedListener.selectionChanged() with the updated selection.  The trivial case is setItemCount(0), which should fire an event with StructuredSelection.EMPTY.

In fact, no event is fired.

I have traced this to a faulty assumption in the implementation of StructuredViewer.preservingSelection().  This is the code:

// send out notification if old and new differ
ISelection newSelection = getSelection();
if (!newSelection.equals(oldSelection)) {
    handleInvalidSelection(oldSelection, newSelection);
}

The problem is that this code is running on the UI thread during the same Windows event that caused TableViewer.refresh() -> TableViewer.clearAll() -> Table.clearAll().  At this point, all of the old TableItems are still present, with the old selection state and data field contents, but cleared text and image fields.  Therefore, getSelection() is guaranteed to return the same contents as the oldSelection, and the selection event will never be fired.

At some point after the UI thread finishes processing the current event and the main event pump loops again, Windows will attempt to redraw the Table, which will trigger SWT.SetData -> ILazyContentProvider.updateElement() -> TableViewer.setItemCount().  At this point, we should expect Windows to notify us that the selection has changed (if it indeed has changed), which would propagate up through Table to TableViewer and then to our ISelectionListeners.  Apparently, Windows fails to notify SWT that a selection change has occurred in this case.

It's unclear to me whether this is a JFace or an SWT bug.  I suspect SWT is the problem, but I have only observed the effects via JFace API.  Perhaps Table.setItemCount() needs to explicitly fire selection events in this case, since Windows is failing us here.  Perhaps TableViewer.setItemCount() needs to verify the selection state and fire the events at that level.
Comment 1 Boris Bokowski CLA 2008-02-25 23:24:44 EST
I think a fix for this should be in JFace where we know that we are changing the number of items. It might be hard to implement though - we don't necessarily know the mapping between items and elements at the time we would want to fire a selection change.
Comment 2 Peter Centgraf CLA 2008-02-26 09:16:43 EST
If the selection changes because of setItemCount(), there must be strictly fewer items in the selection than before.  At the point this method is fired, we can determine which TableItems will be selected easily enough by using the new count as a threshold.  The viewer can either assume that the elements mapped to those items will be unchanged (and use getData() immediately), or verify the mappings with the content provider first.

I'm not sure which of those two approaches is preferable.  As I mentioned, it is common for setItemCount() to be called by the content provider during updateElement() as a result of refresh() or setInput().  Therefore, it is likely for the Item-element mapping to be stale.  On the other hand, I dislike the idea of reentrant calls to updateElement().  The updateElement() API can also be implemented asynchronously, so there's no guarantee that the mapping will be up to date before the call returns.

Perhaps the better approach would be to report the selection changes at the finest possible granularity.  When setItemCount() is called, report the selection at that time, ignoring any changes that updateElement() may or may not cause.  When/if updateElement() results in a call to replace() for a selected Item, report that as a new selection.
Comment 3 Boris Bokowski CLA 2009-11-26 09:48:29 EST
Hitesh is now responsible for watching bugs in the [Viewers] component area.
Comment 4 Eclipse Webmaster CLA 2019-09-06 16:16:56 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.