Opened 11 years ago

Closed 11 years ago

#6899 closed defect (fixed)

Tree: error if delete item, then recreate item w/same id

Reported by: guest Owned by: bill
Priority: high Milestone: 1.2
Component: Dijit Version: 1.1.1
Keywords: Cc:
Blocked By: Blocking:

Description

I am trying to reload a dijit.tree tree node. I created a refresh node method that deletes all of the children in a dijit tree node.

After I recursively remove the items in a node I can't re add them with the same id.

I get an invalid item exception even though I tried to do a store.save() after the deleteItem;

[Exception... "'Error: dojo.data.ItemFileReadStore?: Invalid item argument.' when calling method: [nsIDOMEventListener::handleEvent]" nsresult: "0x8057001c (NS_ERROR_XPC_JS_THREW_JS_OBJECT)" location: "<unknown>" data: no]

Attachments (3)

test_Tree_Programmatic_recursive_delete2.html (5.2 KB) - added by guest 11 years ago.
test_Tree_Programmatic_recursive_delete.html (5.5 KB) - added by guest 11 years ago.
deleteReadd.html (3.4 KB) - added by bill 11 years ago.
simplified testcase, goes in dijit/tests/tree

Download all attachments as: .zip

Change History (10)

comment:1 Changed 11 years ago by guest

When I use the test file test_Tree_Programmatic_recursive_delete.html I can add children.

The first time I can recursively delete them with the deleteItems button.

The program then readds them.

The second time I do a deleteItems it gives me an invalid argument error.

comment:2 Changed 11 years ago by bill

I looked at your test program but it doesn't even seem to be syntactically valid javascript. You have these lines *not* inside of an object:

23	          tree:        null;
24	          myModel:    null;

It's also very hard to read due to the inconsistent tabbing/spacing that causes random indentation.

Also, these lines in deleteItem() are strange:

107	                function deleteItems () {
108	                          var children = 0;
109	              children = this.node.getChildren();       

What is "this", and why are you setting children to the scalar 0 and then immediately resetting it to an array?

I don't think any of these things are the root of your problem but you clean up the testcase some before we look at it, at least addressing these issues?

comment:3 Changed 11 years ago by guest

I cleaned up the testcase file.

The following code lines are just object references to the tree node and model. The methods in a complex tree can reference them as objects when inside another object to add or delete references.

23	          tree:        null;
24	          myModel:     null;

I initialized the var children to zero. There was a case where the recursive delete would only delete the first node if it was unitialized. If the var children is set to zero the array length would get returned correctly.

107	         function deleteItems () {
108	                  var children = 0;
109	                  children = this.node.getChildren(); 

Changed 11 years ago by bill

Attachment: deleteReadd.html added

simplified testcase, goes in dijit/tests/tree

comment:4 Changed 11 years ago by bill

Milestone: 1.4
Owner: set to bill

Ah OK, looks like there's a problem w/Tree because it caches old TreeNodes (for items that have been orphaned, or in this case, deleted), and then reuses them if their TreeNode?.item.id == newItem.id:

var id = model.getIdentity(item),
	existingNode = tree._itemNodeMap[id],

The model should (but doesn't) have a notification when an item has been deleted, so that Tree can remove any corresponding TreeNode? and not keep a cached copy w/a pointer to a deleted item.

Probably as a workaround you can delete myTree._itemNodeMap[id] for every time an item is deleted, but I'll work on a real solution.

That code above still strange. You should use "var" to declare the variables and stop using this.. I'm attaching a simplified test case. As before, pressing "delete and re-add items" twice shows the problem, which occurs on the second deleteItem() call.

comment:5 Changed 11 years ago by bill

Summary: refresh treenodeTree: error if delete item, then recreate item w/same id

Bug can be seen in test_Tree_Notification_API_Support.html, after fix to #6910:

  1. select node5
  2. create new item with id=node6
  3. select node6 and delete it
  4. create new item with id=node6

comment:6 Changed 11 years ago by bill

Milestone: 1.41.2
Status: newassigned

comment:7 Changed 11 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [13923]) Fixes #6899: Tree problem when deleting an item then creating new item w/same id. Tree now correctly removes nodes from cache for deleted items. (Cache is used for DnD, because when an item is moved it is first orphaned, then reparented, as two separate notifications.) Also added store.save() call to test files as ItemFileWriteStore? gets confused otherwise (when deleting item and then recreating item w/same id). !strict

Note: See TracTickets for help on using tickets.