Bugzilla – Full Text Bug Listing
| Summary: | 3 dots null items added on a TreeViewer | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [RT] RAP | Reporter: | Arnaud MERGEY <a_mergey> | ||||||||||||||
| Component: | RWT | Assignee: | Project Inbox <rap-inbox> | ||||||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||||||
| Severity: | normal | ||||||||||||||||
| Priority: | P1 | CC: | tbuschto | ||||||||||||||
| Version: | 1.4 | ||||||||||||||||
| Target Milestone: | 1.5 M1 | ||||||||||||||||
| Hardware: | All | ||||||||||||||||
| OS: | All | ||||||||||||||||
| Whiteboard: | sr141 | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Arnaud MERGEY
Created attachment 196117 [details]
snippet to reproduce the issue
- Launch snippet.tree application
- double click on one item on the left tree
- then double click on "next" item on the right tree, you should see the "3 dots items" added
-then you can double click several times on "previous " item, you will see that the number of "3 dots" items is equals to the number of "normal" items
If you launche the snippet against RC1 you should see the issue, against M7 it should work as expected
I saw the issue with chrome 11 and firefox 4
I can reproduce the issue as well. Created attachment 196215 [details]
Proposed patch
itemCount was increased by TreeItem.js#_add. Fixed by removing the last item on add.
Comment on attachment 196215 [details]
Proposed patch
This patch is very much approved.
Applied patch to CVS HEAD and v14_Maintenance branch. Since the patch pops off the last item from the array, it has the potential of deleting created items. So what happens if the _children array is already filled? In this case creating a new item with an index would silently delete the last item. This wonder if this is an acceptable client-side API. However, if we can ensure that the server will always call Tree#setItemCount() *before* new TreeItem(), I'm ok with the fix for RC2. Yes... the server always calls setItemCount before the creation of new child TreeItem(). This is how WidgetTreeVisitor works - first visit the parent (tree)/ parentItem and call setItemsCount on them and afterwards - create the new child items. Just for the record - we discussed with Tim more clear implementation (to set itemCount only from the server and not modify it on the client (call push/pop)), which requires more refactorings, especially in tests. This will be done with Tree/Table client side merge. Well yes, my thoughts went into the same direction... Hello, This bug does not seems to be totally fixed and some issues are still reproducible. With my previous snippet, you can try following: - Launch snippet.tree application - double click on one item on the left tree - then double click on one element on the outline tree (not next or previous) the next node disappear, even it should not -then you can double click several times on "previous " item, you will see the "next" item replaced by a "3 dots" item Created attachment 197067 [details]
Proposed patch
With the previous patch we fixed the TreeItem.js#_add method to not change the item count on the client, but didn't fixed the TreeItem.js#_remove method. With this patch the _remove method doesn't change the item count on the client anymore.
Comment on attachment 197067 [details]
Proposed patch
The patch looks safe to me and from my understanding the issue is critical enough to include the fix into the release
Comment on attachment 197067 [details]
Proposed patch
We worked on it together.
Ported and committed the second patch to the "v14_Tree_Table_Merge" branch. Applied second patch to CVS HEAD. Applied second patch to 1.4 Maintenance branch. the fix will be available with RC3. Sorry to reopen it again, but by testing RC3, I found another issue It can be reproduced with my snippet, but in order to reproduce it faster, the code of my snippet can be changed like this on top of snippet.tree.outline.MyContentOutlinePage class replace "private final static int SIZE = 20"; by "private final static int SIZE = 2;" - Launch snippet.tree application - double click on one item on the left tree - then double click on "previous" item on the tree of the outline view -> previous item is displayed twice If you do not change the code of my snippet, to reproduce the issue you have to click on "previous" item, until there is only one element in the tree Hi Arnaud, but I think that this is completely different bug... nothing related to "..." :-). Is this working with M7? Hi Ivan, I have just tested this bug is also reproducible with M7 As the way this issue occurs, I (probably wrongly) thought that this issue was related to the "..." ones. Do you want I create another bug ? Arnaud, I think that there is a problem in your code. Look at MyContentOutlinePage double click listener. Removing the:
if(size==1){
lists[0].add(PREVIOUS);
}
fixes the problem, but introduce a new one - clicking on "next" item on empty tree does not add "previouse" item. Could you check your code first (test against RCP if needed)? If you still think that this is a bug please open another bugzilla. I will close this bug as fixed.
No you are right, the issue was in my code, I am really sorry, I should have been more carreful Created attachment 197436 [details]
updated snippet
I found another "..." issue. I am quite sure it is not related to my code this time, as the issue occurs with 1.4 RC3, I have tested with M7, and there is no "..." issue. In order to reproduce the issue, with the updated snippet, - Launch snippet.tree application - double click on one item on the left tree - on outline view expand some items until vertical scrollbar appears - then scroll down - then click on the action item on the oultine view (displayed with ISharedImages.IMG_ELCL_SYNCED icon) -> you should see almost all items of the tree replaced with "..." items then there also seems to be some refresh issue, by scrolling in this state. The action allowing to reproduce the issue, set input on the outline treeviewer and try to restore old expansion state and selection. It seems to be related to this, as this issue occurs everytime I do this kind of code: TreePath[] oldTreePaths = getTreeViewer().getExpandedTreePaths(); ISelection oldSelection = getTreeViewer().getSelection(); myLists=new MyLists(); getTreeViewer().setInput(myLists.list); getTreeViewer().setExpandedTreePaths(oldTreePaths); getTreeViewer().setSelection(oldSelection); Created attachment 197455 [details]
updated snippet
Arnaud, we can reproduce the issue and are working on a fix. It's too late to be fixed in 1.4. Will be fixed in HEAD. I fixed the issue in the v14_Tree_Table_Merge branch. The fixed can not be easily ported to HEAD or 1.4, but will be solved when the branch in merged with HEAD soon. Created attachment 197601 [details]
Ported fix to 1.4 Maintenance branch
Merged v14_Tree_Table_Merge" branch into CVS HEAD (See Bug 332524), which fixes this bug. (In reply to comment #27) > Created attachment 197601 [details] > Ported fix to 1.4 Maintenance branch Applied patch to v14_Maintenance branch. |