On 2012/09/10 23:15:08, tbroyer wrote:
Brian: Unnur thinks you'd review that patch faster than her ;-)


https://gwt-code-reviews.appspot.com/1827803/diff/1/user/src/com/google/gwt/user/cellview/client/CellTree.java
File user/src/com/google/gwt/user/cellview/client/CellTree.java
(right):


https://gwt-code-reviews.appspot.com/1827803/diff/1/user/src/com/google/gwt/user/cellview/client/CellTree.java#newcode743
user/src/com/google/gwt/user/cellview/client/CellTree.java:743: if
(!nodeView.isRootNode() &&
nodeView.getImageElement().isOrHasChild(target)) {
On 2012/09/10 23:02:41, unnurg wrote:
> Does the root node never need to be "opened"?  Or does setNode()
only add that
> CSS that we're trying to get rid of?

Actually, the CSS is not the issue, it's only what triggers it: the
actual issue
is that the root node is closed when receiving a mousedown event; the
margin:20px makes it so that there are 20px high regions between
top-level nodes
where clicks target the root node.
And the real issue is that getImageElement accesses some specific
element in the
DOM, assuming the node's element contains a first child for the node's
value and
disclosure image (the one that getImageElement returns) and a second
one with
the children. Except that for the root node, there's no visible value
(it's a
"virtual" node) so the second child is actually the first, and
getImageElement
returns a DIV that contains all children (so when clicking in between
child
nodes, it targets getImageElement, producing a false positive here).

Yes, it's a bit of a complex thing ;-)

The root node is opened in the constructor and should never ever be
closed
(remember: it's "invisible", it's only there to contain the top-level
nodes).

LGTM

Great - thanks for the explanation (and the patch).  I'll add this to
the list of your patches that are going in (Brian will probably do the
actual patching and submitting unless he hasn't gotten to it by Friday,
in which case I'll probably go through and do them)

https://gwt-code-reviews.appspot.com/1827803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to