Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#11424 closed defect (fixed)

[patch] [cla] Tree: dnd cause the browser crash

Reported by: rat9912345 Owned by: bill
Priority: high Milestone: 1.6
Component: Dijit Version: 1.5.0b2
Keywords: Cc:
Blocked By: Blocking:

Description

Test case :/dijit/tests/tree/test_Tree_Programmatic.html

Defect 1: If I drag and drop the items a little quickly, the browser will be crashed,the browser will no response ,and then will output the errors(IE is stackoverflow,ff is too much recursion),then many avatars can not be hidden,and some items disappeared.

Defect 2: If the tree is too width or height ,there will be a scroll bar. I drag the scroll bar by accident .The avatar pointed item is the last selected item ,and the avatar will exist all the time.

I will upload the gif demos for them.

Attachments (5)

one.7z (130.7 KB) - added by rat9912345 9 years ago.
defect one
two.7z (196.7 KB) - added by rat9912345 9 years ago.
defect two
one.gif (137.7 KB) - added by rat9912345 9 years ago.
two.gif (249.8 KB) - added by rat9912345 9 years ago.
dragTopItem.patch (1.7 KB) - added by bill 9 years ago.
make sure dragging a top level item and dropping into a subnode removes the item from list of top-level items

Download all attachments as: .zip

Change History (17)

Changed 9 years ago by rat9912345

Attachment: one.7z added

defect one

Changed 9 years ago by rat9912345

Attachment: two.7z added

defect two

comment:1 Changed 9 years ago by bill

Priority: highestnormal
severity: blockernormal

I can't acess those "one.7z" and "two.7z" files you uploaded, are those screenshots? You should upload them in a standard format: PNG, GIF, JPEG, TIFF, etc.

Judging from your description (of "defect #1") you aren't getting a browser crash but just a javascript exception. It sounds like #7971/#10587 but I can't reproduce those tickets or this one. You might also just be seeing #6346 or #9475.

I don't understand what your "defect #2" is about. I can't get a vertical scrollbar on test_Tree_programmatic regardless of browser window size... there is just the browser's scrollbar itself.

So, you'll need to provide a lot more information.

comment:2 Changed 9 years ago by rat9912345

Oh sorry ,I set the width in defect two.

Changed 9 years ago by rat9912345

Attachment: one.gif added

Changed 9 years ago by rat9912345

Attachment: two.gif added

comment:3 Changed 9 years ago by Adam Peller

you should also be very specific about which browsers (and versions) you are able to reproduce this on

comment:4 Changed 9 years ago by rat9912345

both of FireFox3.6 and IE8

comment:5 Changed 9 years ago by Adam Peller

it might be helpful also to see the stack overflow, or at least the top of it, if you are able to see it in the console or debugger

comment:6 in reply to:  1 Changed 9 years ago by rat9912345

Replying to bill:

I can't acess those "one.7z" and "two.7z" files you uploaded, are those screenshots? You should upload them in a standard format: PNG, GIF, JPEG, TIFF, etc.

Judging from your description (of "defect #1") you aren't getting a browser crash but just a javascript exception. It sounds like #7971/#10587 but I can't reproduce those tickets or this one. You might also just be seeing #6346 or #9475.

I don't understand what your "defect #2" is about. I can't get a vertical scrollbar on test_Tree_programmatic regardless of browser window size... there is just the browser's scrollbar itself.

So, you'll need to provide a lot more information.

Hi bill ,I found an easy method to reproduce the defect. use link: http://archive.dojotoolkit.org/nightly/dojotoolkit/dijit/tests/tree/test_Tree_Programmatic.html

Step 1 - Drag Africa folder node and drop to Asia node, then a copy of Africa node is added under Asia, the correct behavior should cut not copy.

Step 2 - Drag the new Africa node(Asia) back to the previous Africa, then the browser will crash.

comment:7 Changed 9 years ago by bill

Milestone: tbd1.6
Summary: Tree dnd cause the browser crash[patch] [cla] Tree: dnd cause the browser crash

Cool, thanks for the test case. Dropping an item onto itself definitely won't work but the copy-rather-than-cut problem is a bug. Actually there's also a problem with the test case itself, since users of ForestStoreModel are required to implement onLeaveRoot and onAddToRoot.

I've got a patch for this which I'll attach here, and checkin for 1.6, after some more testing.

comment:8 Changed 9 years ago by bill

Owner: set to bill

Changed 9 years ago by bill

Attachment: dragTopItem.patch added

make sure dragging a top level item and dropping into a subnode removes the item from list of top-level items

comment:9 in reply to:  8 Changed 9 years ago by rat9912345

Replying to bill: Thank you bill for so quick fix. Next ,help you to reproduce the second defect. Use the same test case ,and set the tree's width to 200px,like

myTree = new dijit.Tree({

id: "myTree", model: myModel, dndController: "dijit.tree.dndSource", style: "width:200px;overflow:auto;"

});

Step 1: Expand the tree to enough width ,there will appear the horizontal scroll bar.

Step 2: Try to drag the scroll bar then release the mouse at any palce, avatar item is the last selected item ,these step everything is normal except the avatar shouldn't appear.

Step 3: Now drag the scroll bar once again.Different with step 2, these time the error item can be drag and drop to the tree ,but after drop the avatar always exist.

comment:10 Changed 9 years ago by bill

You should file each bug as a separate ticket. But this time it's not necessary, I think the second defect you found is already filed as #10585, and also #10903.

comment:11 Changed 9 years ago by bill

Resolution: fixed
Status: newclosed

(In [22495]) Any change to any item may make it start/stop matching ForestStoreModel's query for the top level items in the tree. Thus, need to re-query the top level items. Fixes #11424 !strict.

Also added a bunch of warning comments that the developer must define certain callbacks when using ForestStoreModel.

comment:12 Changed 9 years ago by bill

(In [22496]) Add tests that ForestStoreModel noticed when modified items start/stop matching query for top level items in the tree. Refs #11424.

Note: See TracTickets for help on using tickets.