Opened 8 years ago

Closed 8 years ago

#12291 closed defect (fixed)

dijit._Container removeChild(), removal of child of index o (zero)

Reported by: aedart@… Owned by: bill
Priority: low Milestone: 1.6
Component: Dijit Version: 1.6.0b1
Keywords: dijit._Container, removeChild Cc:
Blocked By: Blocking:

Description

If one want to use the method removeChild, inside dijit._Container, and pass in a number as argument, one cannot remove the first child added to the container - given there is one. The reason seems to be due to the following line, inside the removeChild method:

if(typeof widget == "number" && widget > 0){
	widget = this.getChildren()[widget];
}

So, given that I have added some child widgets to my container, and I need to remove the first element, by invoking removeChild(0), then the method will not do as expected.

A solution could be to alter the if condition to allow o (zero) as a valid input. E.g:

		removeChild: function(/*Widget or int*/ widget){
			// summary:
			//		Removes the passed widget instance from this widget but does
			//		not destroy it.  You can also pass in an integer indicating
			//		the index within the container to remove

			if(typeof widget == "number" && widget >= 0){
				widget = this.getChildren()[widget];
			}

			if(widget){
				var node = widget.domNode;
				if(node && node.parentNode){
					node.parentNode.removeChild(node); // detach but don't destroy
				}
			}
		},

However, note that I cannot argue as how this would change other parts of dijit, that rely on this class/method.

Change History (2)

comment:1 Changed 8 years ago by bill

Component: GeneralDijit
Milestone: tbd1.6
Owner: changed from anonymous to bill
Status: newassigned

I traced that code back to [14163] but I agree, the check seems wrong. Actually we don't generally do bounds checking in dojo at all, because of the added code bloat.

I'll remove that check, after running the regression tests.

Note that removeChild(index) doesn't work for all widgets, for example StackContainer (and subclasses) only handle removeChild(widget).

comment:2 Changed 8 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [23787]) Remove redundant and incorrect bounds checking, fixes #12291 !strict.

Note: See TracTickets for help on using tickets.