gwtproject / gwt

GWT Open Source Project
http://www.gwtproject.org
1.53k stars 377 forks source link

CellTree disappeared when clicking on the space between two top level nodes (due to possible bug in animation) #6358

Closed dankurka closed 9 years ago

dankurka commented 9 years ago

Originally reported on Google Code with ID 6359

Found in GWT Release (e.g. 1.5.3, 1.6 RC):

Since 2.3, including 2.4-beta. 2.2 does not have this problem.

Encountered on OS / Browser (e.g. WinXP, IE6-7, FF3):

Only tested on Linux with Firefox 4.0 and Chrome 12 dev.

Detailed description (please be as specific as possible):

Create a CellTree with default resources. If mouse-click the space between two top
level tree nodes (when mouse pointer is arrow instead of pointer), the whole tree is
gone. An easy way to show this is to launch the Showcase sample app for the specific
versions of GWT (2.3 and 2.4 beta) and perform the action on the left side tree. I
tried to pinpoint the bug in the CellTree code but didn't have enough time. It seemed
that there is a bug in the animation code. The tree disappears due to a clean up before
the animation. Note that this happens even the tree animation is disabled.

Shortest code snippet which demonstrates issue (please indicate where
actual result differs from expected result):

Using the Showcase sample.

Workaround if you have one:

If the space between the tree nodes are hidden, we can prevent this from happening.
The easiest way is to use the BasicResource of the CellTree which essentially removes
the spaces.

Links to relevant GWT Developer Forum posts:

Reported by wenye04 on 2011-05-15 19:53:19

dankurka commented 9 years ago
I meet same problem on Win7 Chrome Browser. and dig into the source code, seems is bellow
lines cause this problem:

in 2.3.0 source code, 
com.google.gwt.user.cellview.client.CellTree class have onBrowserEvent() as bellow:

  @Override
  public void onBrowserEvent(Event event) {
...
    final CellTreeNodeView<?> nodeView = findItemByChain(chain, 0, rootNode);
    if (nodeView != null) {     // <------- this line cause the problem
...

in 2.2.0 source code, the code as bellow:

  @Override
  public void onBrowserEvent(Event event) {
...
   final CellTreeNodeView<?> nodeView = findItemByChain(chain, 0, rootNode);
    if (nodeView != null && nodeView != rootNode ) {       // <------- this line cause
the problem
...

because of not check current nodeView is a rootNode, so the code close whole rootNode,
which cause the tree disappear , I am not sure how to fix this. Anybody can help? thanks.

Reported by Santal.Li on 2011-05-31 08:14:12

dankurka commented 9 years ago
The line mentioned in the comment above by Santal was introduced with issue 5546 (http://code.google.com/p/google-web-toolkit/issues/detail?id=5547).

This has been integrated with r9997 and was reviewed at http://gwt-code-reviews.appspot.com/1420801.

Santal, did changing the line back solve the problem?

Reported by felixjmoeller on 2011-07-06 22:03:38

dankurka commented 9 years ago
how did you solve ???

Reported by fenster.ben on 2011-08-28 22:02:57

dankurka commented 9 years ago
I was able to suppress this issue, in a debug-only environment, using css alone. 

I did so by disabling the rule, which applies to all CellTree rows regardless of node
level, "margin-top: 20px" in the obfuscated class "GDN1L2BCGF". 

It seems that clicking in that margin areas anywhere within the tree causes the entire
tree to disappear. And once that rule had been removed, using Chrome's developer tools,
I was unable to replicate the original issue. 

Note Better: Because I used Chrome's developer tools to investigate and test my theory,
I have no real reason to believe my observations will lead to a solution. I only offer
my observations in hopes that they will help resolve this issue.

For what it's worth, it would appear the offending class, if there is one, is cellTreeTopItem
(which seeminly applies to all rows within the celltree).

Reported by icc.goicc on 2011-08-31 11:49:04

dankurka commented 9 years ago
I was able to replicate the issue in gwt showcase website http://gwt.google.com/samples/Showcase/Showcase.html#!CwCheckBox
if you click in the space between any of the top items the CellTree will disappear.

Reported by aelgali on 2011-10-04 07:03:13

dankurka commented 9 years ago
Issue 6384 has been merged into this issue.

Reported by t.broyer on 2011-10-18 16:21:30

dankurka commented 9 years ago
Issue 6904 has been merged into this issue.

Reported by t.broyer on 2011-10-18 16:21:51

dankurka commented 9 years ago
I was able to get around this issue with the following code:

Resource resource = GWT.create(Resources.class)
StyleInjector.injectAtEnd("."+resource.cellTreeStyle().cellTreeTopItem()+" {margin-top:
0px;}"); // Not the best code
CellTree cellTree = new CellTree(treeViewModel, null, resource);

As stated before, the margin-top: 20px is causing the problem when the margin is clicked.
 The code overwrites the margin-top value from all top level nodes.

Reported by meyer.rob on 2011-10-18 21:31:55

dankurka commented 9 years ago
Glad I found this - I spent some time trying to work out what was happening.
@Meyer - thank you for this - very useful. One totally minor point - there's a trivial
typo in #8 - it should be:

Resources resource = GWT.create(Resources.class);
        ^                                       ^^ 

The workaround does seem to achieve the purpose, though!

Reported by alan@mechnicality.com on 2011-10-29 20:18:18

dankurka commented 9 years ago
Comments indicate this has been fixed in trunk--2.4.0 does not seem to have the fix.

If anyone using trunk is still having problems, please leave a comment and we can reopen
it.

Reported by stephen.haberman on 2011-10-30 02:13:57

dankurka commented 9 years ago
Hi guys!

We're using GWT 2.4.0 in our project and this bug still persists.

However, in this bug status, you can see:

Status: FixedNotReleased
Labels: Category-UI Milestone-2_5

So, I guess we have to wait to GWT 2.5 release.

Reported by martins.tuga on 2011-11-04 12:46:06

dankurka commented 9 years ago
Issue 7218 has been merged into this issue.

Reported by t.broyer on 2012-02-29 15:42:50

dankurka commented 9 years ago
Bulk edit: should be fixed in the GWT 2.5 release candidate.

Reported by skybrian@google.com on 2012-06-27 03:38:12

dankurka commented 9 years ago
I'm using 2.5rc1 now and the bug is still there.

Reported by radioanonzoi on 2012-07-31 09:27:21

dankurka commented 9 years ago
Reopening, this has not been fixed.

Reported by t.broyer on 2012-09-08 09:50:32

dankurka commented 9 years ago
Does anyone have a fix patch for this issue?  If so, please send it to me before Thursday
and I'll try and get it into GWT 2.5 - I'm not familiar enough with this code to fix
it myself in time.

Reported by unnurg on 2012-09-10 19:10:38

dankurka commented 9 years ago
Read the thread carefully, the workaround is in 8th comment.

Reported by radioanonzoi on 2012-09-10 19:17:39

dankurka commented 9 years ago
Yup - and the workaround seems to work fine, so unless someone has a fix for the library
code handy, I'm not planning on prioritizing this for the RC2.  If someone does have
a (non hacky and submittable) fix, please feel free to send for review.  Thanks!

Reported by unnurg on 2012-09-10 19:33:56

dankurka commented 9 years ago
I played a bit with the Chrome Developer Tools and found that the behavior is triggered
by a mousedown event.
Looking at the code, it looks like the fix should be as easy as special-casing nodeView.isRootNode()
(or nodeView == rootNode) in the "if (nodeView.getImageElement().isOrHasChild(target))"
test. This is because the (virtual, hidden) root node has no label, so getImageElement()
actually looks into the DIV that contains the child items (that element matches with
the target, so the root node is closed, leading to an emptied/cleared CellTree).

Reported by t.broyer on 2012-09-10 21:05:33

dankurka commented 9 years ago
I spent a few minutes verifying my theory and testing the proposed change. It seems
to work (aka "worked for me"). Patch sent for review at https://gwt-code-reviews.appspot.com/1827803/

Reported by t.broyer on 2012-09-10 21:34:58

dankurka commented 9 years ago
This issue was closed by revision r11265.

Reported by gwt.mirrorbot on 2012-09-12 01:45:24

dankurka commented 9 years ago

Reported by t.broyer on 2012-09-12 06:47:02

dankurka commented 9 years ago

Reported by skybrian@google.com on 2012-10-03 00:08:28

dankurka commented 9 years ago

Reported by skybrian@google.com on 2012-10-28 23:14:34