thanhlong203 / closure-library

Automatically exported from code.google.com/p/closure-library
0 stars 0 forks source link

Strange behavior in goog.ui.Component.prototype.addChildAt #182

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
Consider the following lines in addChildAt():

  child.setParent(this);
  goog.array.insertAt(this.children_, child, index);

  if (child.inDocument_ && this.inDocument_ && child.getParent() == this) {

Given how this code works, I don't understand how there could be a case where 
child.getParent() == this is false.

The problem is that this kicks you into the case where it assumes you are 
moving a child into a different position among a parent. Unfortunately, if you 
do the following:

child.decorate(el);
parent.addChild(child);

Then this ends up reparenting child, which is probably not the intended 
behavior. Fortunately, if you swap the statements, then it will work as 
intended:

parent.addChild(child);
child.decorate(el);

So it seems that something like this would be better:

  var oldParent = child.getParent();
  child.setParent(this);
  goog.array.insertAt(this.children_, child, index);

  if (child.inDocument_ && this.inDocument_ && child.getParent() == oldParent) {

so that it would only reparent the DOM of the child if it were clearly a move 
among the child's siblings.

Original issue reported on code.google.com by bolinf...@gmail.com on 2 Jul 2010 at 9:04