Bugzilla – Full Text Bug Listing
| Summary: | [Viewers] Refreshing ContainerCheckedTreeViewer does not restore checked/grayed states correctly. | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | orders.lightfoot | ||||
| Component: | UI | Assignee: | Platform UI Triaged <platform-ui-triaged> | ||||
| Status: | CLOSED WONTFIX | QA Contact: | |||||
| Severity: | major | ||||||
| Priority: | P3 | CC: | jayn, kyle.baardson, moritz.kreutzer, storaskar, susan, tom.schindl, wuut, xavier.raynaud | ||||
| Version: | 3.2.1 | Keywords: | helpwanted | ||||
| Target Milestone: | --- | ||||||
| Hardware: | PC | ||||||
| OS: | Windows XP | ||||||
| Whiteboard: | stalebug | ||||||
| Attachments: |
|
||||||
|
Description
orders.lightfoot
Created attachment 56924 [details]
ContainerCheckedTreeViewer Refresh Problem Demonstrated
Sorry, but I won't have time for this in 3.3. Are there any efforts for fixing this in 3.4? Would you be able to contribute a patch? I don't think so.. But anyway, I could have a look. for what it's worth, I can see why this happens, but I don't know the tree viewer code well enough to propose a solution.
I believe the problem to be in CheckboxTreeViewer.applyState(...)
The code that records the checked state uses the elements to remember what checkmarks should be restored, and then attempts to traverse the tree items and reset their check/gray data if their element was remembered in the saved state. But non-expanded elements are typically dummy items that to not have their element set into their data. So their check state is never restored.
private void applyState(CustomHashtable checked, CustomHashtable grayed,
Widget widget) {
Item[] items = getChildren(widget);
for (int i = 0; i < items.length; i++) {
Item item = items[i];
if (item instanceof TreeItem) {
Object data = item.getData();
>>> if (data != null) {
TreeItem ti = (TreeItem) item;
ti.setChecked(checked.containsKey(data));
ti.setGrayed(grayed.containsKey(data));
}
}
applyState(checked, grayed, item);
}
}
I attempted to force the children to get updated using updateChildren(Widget widget, Object parent, Object[] elementChildren) but this didn't seem to help either.
I really don't know what I'm doing at this point but I believe this is part of the problem. I will probably work around this problem by recording the check state myself and then resetting them.
See bug #207064. PDE implemented FilteredCheckboxTree to solve many of these problems. We might consider adopting this in platform UI. It has some assumptions/limitations but even if it only works for certain kinds of models, it would be useful to have. In Class AbstractTreeViewer. There seems to be the method that throws away all the data Objects. This happens when you collapse the Tree for example.
I am not familiar with the TreeView code but this make no sense to me.
Is there a way to turn this off by flag?
private void updateChildren(Widget widget, Object parent,
Object[] elementChildren, boolean updateLabels) {
// optimization! prune collapsed subtrees
if (widget instanceof Item) {
Item ti = (Item) widget;
if (!getExpanded(ti)) {
// need a dummy node if element is expandable;
// but try to avoid recreating the dummy node
boolean needDummy = isExpandable(ti, null, parent);
boolean haveDummy = false;
// remove all children
Item[] items = getItems(ti);
for (int i = 0; i < items.length; i++) {
if (items[i].getData() != null) {
disassociate(items[i]);
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> [[[items]]][i].dispose();
} else {
if (needDummy && !haveDummy) {
haveDummy = true;
} else {
items[i].dispose();
}
}
}
if (needDummy && !haveDummy) {
newItem(ti, SWT.NULL, -1);
}
return;
}
}
(In reply to comment #8) > I am not familiar with the TreeView code but this make no sense to me. > Is there a way to turn this off by flag? No, I don't think you can turn this off. When the tree is refreshed, we prune collapsed subtrees. How does this make no sense? (In reply to comment #9) > (In reply to comment #8) > > I am not familiar with the TreeView code but this make no sense to me. > > Is there a way to turn this off by flag? > > No, I don't think you can turn this off. When the tree is refreshed, we prune > collapsed subtrees. How does this make no sense? > When i add an item to a collapsed tree (not to the root), i have to do a refresh, so the added item show up. After this refresh all collapsed items loose their check - flags. I thought that the data Object become removed after the collapse action. And because this, the check - flags does not become restored correctly. Did i miss something? (In reply to comment #10) > When i add an item to a collapsed tree (not to the root), i have to do a > refresh, so the added item show up. Can you not call AbstractTreeViewer.add(Object parent, Object element) instead of refresh? The problem for me was that filtered tree calls refresh. (In reply to comment #11) > (In reply to comment #10) > > When i add an item to a collapsed tree (not to the root), i have to do a > > refresh, so the added item show up. > > Can you not call AbstractTreeViewer.add(Object parent, Object element) instead > of refresh? > I've tied this also. The new item do not show up with the AbstractTreeViewer.add( method. Whatever i do, seems i always need to call refresh in order to see the new items. Would using an ICheckStateProvider (new API in 3.5) help with this? Hitesh is now responsible for watching bugs in the [Viewers] component area. I had the same issue in gprof viewer in linux tools project. For info, you can find here how I workaround this issue: https://dev.eclipse.org/svnroot/technology/org.eclipse.linuxtools/gprof/trunk/org.eclipse.linuxtools.dataviewers/src/org/eclipse/linuxtools/dataviewers/abstractviewers/AbstractSTTreeViewer.java Perhaps it can help ? (In reply to comment #14) > Would using an ICheckStateProvider (new API in 3.5) help with this? I did just that, but ended up re-writing the treeViewer anyway. My leafnode data was from an emf model, and I found it much faster to update all the tree nodes based on the subtree of leaf node data provided by the emf model. The problem I see with CheckboxTreeViewer is the code below from doUpdateItem setChecked(element, checkStateProvider.isChecked(element)); setGrayed(element, checkStateProvider.isGrayed(element)); After experimenting in a new viewer I found that using the following code consistently will update the tree checked and grey state correctly in the gui on Windows. To explain, I found that if you set the gray state true, you also have to set checked to true. Otherwise you have to set gray state to false and the checked value to the chkstate. The above code from doUpdateItem sets checked and grayed separately, and I believe the ContainerCheckedTreeViewer even sets grayed to false within its setChecked. All that was unreliable, at least on Windows. public boolean setGrayChecked(Object element, boolean chkState, boolean grState) { Assert.isNotNull(element); Widget widget = internalExpand(element, false); if (widget instanceof TreeItem) { TreeItem item = (TreeItem) widget; if (grState){ item.setChecked(true); item.setGrayed(true); } else{ item.setGrayed(false); item.setChecked(chkState); } return true; } return false; } (In reply to comment #17) So, looking at the ContainerCheckedTreeViewer code again, I decided to try to use it and change my checkStateProvider code to always return isChecked true if isGrayed is true, i.e. when some of the children are checked and some not. That seems to fix the update of the grayed state. @Override public boolean isChecked(Object element) { if (isGrayed(element)){ return true; } else ... 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. If the bug is still relevant, please remove the "stalebug" whiteboard tag. |