43908 - Need to check where drag initiates in bookmarks tree

Status: VERIFIED FIXED

(SeaMonkey :: Bookmarks & History, defect, P2)

Description

[Blake Ross]

Build ID: 2000062608

In the bookmarks tab of the sidebar, you can click plain white space (without 
selecting an item) and drag and a drag session will be initiated.  Similarly, 
you can select an item and then just starting from pretty much anywhere else in 
the tree and the drag still starts.  The same holds true for the tree in the 
Manage Bookmarks window.  We need to ensure that the drag is actually 
initiating from the item itself that's being dragged. I don't know if this is 
specific to bookmarks, or rather the general tree dragging code that seems to 
be reused in various places.

Comment 1

[John Morrison]

FYI, here's hyatt's comments (see bug 43367 and bug 43134):
  "These are bugs in the event handlers of individual trees. ... It was
  incorrect of me to hide clicks on the scrollbar from the DOM.  I fixed
  this when I moved to the new tree.  I'm not doing hiding scrollbar
  events any more, and now people have to patch their tree event handlers
  to check to see if a click actually occurred on a cell. Clicking on the
  blank space in a tree would have exhibited the same problem."

Comment 2

[Blake Ross]

Yep, I didn't see that. Thanks John.

RJC: since slamm's on sabbatical, would this be easy for you to fix?  If not, 
feel free to remove yourself from whiteboard/cc -- it's not critical at all.

Comment 3

[Chris McAfee]

over to rjc

Comment 4

[Robert John Churchill]

Keep this on slamm's list... with [rjc] in the status area of the bug.

Comment 5

[Blake Ross]

this was fixed.

Comment 6

[John Morrison]

Actually, this throws an error, which does abort the drag, but not in the best 
way.

  JavaScript error: chrome://communicator/content/bookmarks/bookmarksDD.js line 
80: 
    event.target.parentNode.parentNode.parentNode.parentNode has no properties

I think that this is the right fix

Index: mozilla/xpfe/components/bookmarks/resources/bookmarksDD.js
===================================================================
RCS file: /cvsroot/mozilla/xpfe/components/bookmarks/resources/bookmarksDD.js,v
retrieving revision 1.12
diff -u -r1.12 bookmarksDD.js
--- mozilla/xpfe/components/bookmarks/resources/bookmarksDD.js  2000/08/24 
00:17:28     1.12
+++ mozilla/xpfe/components/bookmarks/resources/bookmarksDD.js  2000/09/11 
03:09:05
@@ -42,7 +42,7 @@
   // if the click is on the tree proper, ignore it. We only care about clicks 
on items.
 
   var tree = document.getElementById("bookmarksTree");
-  if ( event.target == tree )
+  if ( event.target == tree || event.target.localName == "treechildren" )
     return(true);         // continue propagating the event
     
   var childWithDatabase = tree;


Does that look correct, Steve "5 days and counting" Lamm.

Comment 7

[John Morrison]

Attachment: patch; check for treechildren too ...

Comment 8

[Blake Ross]

thanks for digging deeper.  it looks good to me (r=blake).  I don't know how to 
check this in, though...I didn't write the patch, it's from a netscape 
employee, but the bug isn't nsbeta3+.  Brendan, can I check this in with your 
approval?

Comment 9

[John Morrison]

Pending r=slamm, nominating nsbeta3 to cover the Netscape side of procedures.
There is a patch, which is very straightforward and (if judged correct) has 
minimal risk, snuffing out an obvious error.

(Heh. Perhaps I should just drop hints in the future ...).

Comment 10

[Brendan Eich [:brendan]]

Do the right thing: check the patch in.

/be

Comment 11

[johng]

nav triage team:
OK, wise people have a patch and say the patch is good.  Check it in this week 
or forever hold your peace.  +, P2

Comment 12

[Blake Ross]

fix checked in, but I think ray's progid changes broke bookmarks dragging (so 
may be hard to verify)

Comment 13

[Blake Ross]

okay, ray's second batch of progid->contractID checkins fixed dragging, and 
this is now working properly (dragging only initiates in the proper place, and 
no more console errors).

vrfy fixed in new win98 build