This project is archived and is in readonly mode.

#424 ✓resolved
Daniel Steigerwald

empty() should NOT unextend extended children

Reported by Daniel Steigerwald | October 14th, 2008 @ 02:15 AM | in 1.2.1

Serious bug to fix before 1.2.1 release.

IE method clearAttributes, is defacto un$. It remove also all moo element methods, for example setStyle, which is problem. Imagine hover animation on rows, and removing element by click. Because Element.destroy or Element.empty call clearAttributes, current animation will fail -> method .setStyle is undefined. So, clearAttributes is safe only for unload and clone.

Fix:



destroy: function() {
	this.removeEvents().empty().dispose();
	return null;
},

empty: function() {
	$A(this.childNodes).each(function(node) {
		if (node.removeEvents) node.removeEvents();
		Element.empty(node);
		Element.dispose(node);
	});
	return this;
}

Comments and changes to this ticket

  • Thomas Aylott

    Thomas Aylott October 14th, 2008 @ 05:02 AM

    • Assigned user changed from “Valerio” to “Tom Occhino”
    • State changed from “new” to “open”
    • Milestone changed from 2.0 to 1.2.1
    • Tag set to bug, clean, clearattributes, critical, destroy, dispose, empty, error, removeevents
  • Thomas Aylott

    Thomas Aylott October 14th, 2008 @ 05:03 AM

    • Title changed from “clearAttributes catch!” to “destroy /empty calling clearAttributes causing JS Errors”
  • Valerio

    Valerio October 14th, 2008 @ 03:08 PM

    • State changed from “open” to “invalid”

    Destroying while animating an element is not supported by MooTools.

    Also, a diff is always appreciated.

  • Daniel Steigerwald

    Daniel Steigerwald October 14th, 2008 @ 11:20 PM

    I am convinced you did wrong decision Valerio. Same as IE detection, or new Document cleaning.

  • Daniel Steigerwald

    Daniel Steigerwald October 14th, 2008 @ 11:22 PM

    ..anyway, "Destroying while animating an element" always works, until moo incorporated clearAttributes. And will work forever, except Internet Explorer, so you are creating new bugs, thanx.

  • Daniel Steigerwald
  • Thomas Aylott

    Thomas Aylott October 15th, 2008 @ 08:10 PM

    • Assigned user changed from “Tom Occhino” to “Thomas Aylott”
    • Title changed from “destroy /empty calling clearAttributes causing JS Errors” to “empty() should NOT unextend extended children”
    • State changed from “invalid” to “open”
    • Tag changed from bug, clean, clearattributes, critical, destroy, dispose, empty, error, removeevents to bug, clean, critical, destroy, dispose, empty, error, regression

    Fix is here: http://github.com/subtleGradient...

    There is no way in javascript to really destroy an element. You can only remove pointers to it and hope the garbage collector collects it.

    Causing an extended element to stop being extended in IE is unexpected behavior. This is a major regression since there is no workaround possible in user-code. Any user-code fix would require major refactoring.

    If we want to change this behavior, we'll have to wait for 1.3 since this is a breaking change.

    Ideally empty() should completely wipe out any trace of its children, but that's not technically possible in javascript.

  • Tom Occhino

    Tom Occhino October 16th, 2008 @ 06:47 PM

    • State changed from “open” to “resolved”

    fixed for 1.2.1

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Shared Ticket Bins

Pages