Description Pascal Muetschard 2020-05-15 14:28:08 EDT

This becomes apparent in trying out: https://git.eclipse.org/c/platform/eclipse.platform.swt.git/tree/examples/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet254.java

Steps to reproduce:
1) run the above snippet
2) select the last item visible
3) click the button

Expected Result:
The button should say that 10 items are visible.

Actual Result:
The button says that only 1 item is visible.

This is due to the fact that getTopItem(), which is called when the button is clicked, returns the selected item, instead of the expected top item.

Looking at the code, I see these issues:
1) The first thing done in getTopItem() is comparing the current scroll offset to a cached offset stored in a member variable. This cached offset is only set in setTopItem(). However, since the value is default initialized to 0, if the view is not scrolled, it will match the the current offset, even if setTopItem is never called. The code then incorrectly uses _getCachedTopItem.

2) In _getCachedTopItem, if there is a selected item, it is retrieved and returned. The following code snippet:
if (topItem == treeSelection) {
  return topItem;
} else {
  return treeSelection;
}
Is equivalent to "return treeSelection;" and doesn't make sense. Why does it return the selected item?

3) The assumption that caching the last scroll offset from when setTopItem() was called is invalid: imagine setTopItem is called, the user scrolls up, expands an item above the item from setTopItem before, and scrolls back to a spot with the same offset as was cached. The cached topItem is no longer the top item, but would be returned (if there is no selection, or if 2) gets fixed).

4) Caching the top item from setTopItem is invalid: imagine a tree that is fully visible, calling setTopItem on any but the first item will cache the wrong top item. Since the tree is fully visible, calling setTopItem does not scroll the tree and has no effect, thus does not affect the top item.

The Windows and MacOS versions of the Tree do not have this issue and they all deal correctly with 4).