This project is archived and is in readonly mode.

#332 ✓resolved
Daniel Steigerwald

Moo Element clone patch fix - release canditade

Reported by Daniel Steigerwald | August 25th, 2008 @ 05:15 AM | in 1.2.1

Current issues:

  • very slow
  • cloning doesn't work in others scopes / another documents (should be used .newElement instead of new Element, ie need proper document to element creating)
  • there is some textnode cloning also, but textnodes has no own native, and Element.clone(someTextnode) looks a bit weird :)

Slowness is caused by attribute cloning, as a workaround for IE event issue.

jQuery explanation on IE issue:

IE copies events bound via attachEvent when using cloneNode. Calling detachEvent on the clone will also remove the events from the orignal. In order to get around this, we use innerHTML. Unfortunately, this means some modifications to attributes in IE that are actually only stored as properties will not be copied (such as the the name attribute on an input).

But this fix use another technique.

  • el.clearAttributes clears surprisingly also all attached events (inline and via attachEvent too)
  • el.mergeAttributes surprisingly clones all attributes, but no events

I tested this fix both in with IE6/7.


clone: function(contents, keepid){
	var clone = this.cloneNode(!!contents);
	clone.uid = null;
	!keepid && clone.removeAttribute('id');
	if (Browser.Engine.trident) {
            clone.clearAttributes();
            clone.mergeAttributes(this.cloneNode(false));
	}
	return $(clone);
}

Something like testcase:


window.addEvent('load', function() {
            var el1 = new Element('div', {
                id: 'draganDabic',
                text: 'mouse over me',
                style: 'color: red',
                title: 'will this title be clonned?',
                events: {
                    mouseover: function() { document.title = $time() + '|' + this.uid }
                }
            }).inject(document.body);
            el1.clone(true, true)
                .addEvent('mouseover', function() { document.title = this.id + '|' + $time() })
                .inject(document.body);
        });

Comments and changes to this ticket

  • Tom Occhino

    Tom Occhino August 25th, 2008 @ 03:56 PM

    • Assigned user changed from “Valerio” to “Tom Occhino”
    • Milestone changed from 2.0 to 1.2.1

    Fantastic Daniel.

    Not only does your patch work perfectly, but it's one fourth the size of the current, error filled implementation. This patch has already made it into my fork, and will be merged into trunk after some more testing.

    Thanks very much for your research.

  • Daniel Steigerwald

    Daniel Steigerwald August 25th, 2008 @ 04:27 PM

    I know, this is not right place, but I don't know better :) Please, merge also:

    • #320 (current moo leaks a lot!)
    • #330 (extremely useful)
    • #324 (digitarald ask me for fork, but I have no time to learn and install GIT)
    • #281 (extremely useful, we want pixels in IE to!)
    • #270 (it's simple, so why not fix it)
    • #240 (sweet jesus zombie, just swap two lines!!)
    • #264 (it's much faster and reliable, jquery uses)

    These patches are tested, and ready to copy&paste.

  • Daniel Steigerwald

    Daniel Steigerwald August 25th, 2008 @ 04:42 PM

    • Title changed from “Moo Element clone patch fix” to “Moo Element clone patch fix - updated”
    
    clone: function(contents, keepid){
        var clone = this.cloneNode(!!contents);
        clone.uid = null;
        !keepid && clone.removeAttribute('id');
        if (Browser.Engine.trident) {
            clone.clearAttributes();
            clone.mergeAttributes(this);
        }
        return $(clone);
    },
    

    Second cloning was useless.

  • Daniel Steigerwald

    Daniel Steigerwald August 25th, 2008 @ 05:26 PM

    TODO:

    • what about childs events and ids ?
    • has cloneNode another issues?
  • Daniel Steigerwald

    Daniel Steigerwald August 25th, 2008 @ 05:32 PM

    .. not only ids, but uids to.

  • Daniel Steigerwald

    Daniel Steigerwald August 25th, 2008 @ 06:11 PM

    IE cloneNode issues:

    Hell is other browsers - Sartre

  • Daniel Steigerwald

    Daniel Steigerwald August 26th, 2008 @ 12:55 AM

    • Title changed from “Moo Element clone patch fix - updated” to “Moo Element clone patch fix - release canditade”

    Documentation notes:

    • select element checked state is not cloned (crossbrowser behaviour)
    • IE has two issues: cloned element can't be appended to another document, radio buttons checked state isn't cloned too, because checked state could be setted only for radio in DOM
    
    
    // checked state on inputs is clonned, selected options no (like ff and webkit by default)
        // ie forget radio checked state
    	clone: Browser.Engine.trident ?
    	    function(contents, keepid) {
    	        var clone = this.cloneNode(!!contents);
    	        function clean(el) {
    	            el.uid = null;
    	            if (!keepid) el.removeAttribute('id');
    	            var shallowClone = el.cloneNode(false);
    	            el.clearAttributes();
    	            el.mergeAttributes(shallowClone);
    	            return el;
    	        }
    	        if (contents) {
    	            var tEls = this.getElementsByTagName('*'), cEls = clone.getElementsByTagName('*');
    	            for (var i = tEls.length; i--; ) {
    	                clean(tEls[i]);
    	                if (tEls[i].tagName.toLowerCase) cEls[i].checked = tEls[i].checked;
    	            }
    	        }
    	        return $(clean(clone));
    	    } :
    
    	    function(contents, keepid) {
    	        function clean(el) {
    	            el.uid = null;
    	            if (!keepid) el.removeAttribute('id');
    	            return el;
    	        }
    	        var els = this.getElementsByTagName('*');
    	        for (var i = els.length; i--; ) clean(els[i]);
    	        return $(clean(this.cloneNode(!!contents)));
    	    }
    	,
    
    
  • Daniel Steigerwald

    Daniel Steigerwald August 26th, 2008 @ 12:58 AM

    stupid typo

    
    
    clone: Browser.Engine.trident ?
    	    function(contents, keepid) {
    	        var clone = this.cloneNode(!!contents);
    	        function clean(el) {
    	            el.uid = null;
    	            if (!keepid) el.removeAttribute('id');
    	            var shallowClone = el.cloneNode(false);
    	            el.clearAttributes();
    	            el.mergeAttributes(shallowClone);
    	            return el;
    	        }
    	        if (contents) {
    	            var tEls = this.getElementsByTagName('*'), cEls = clone.getElementsByTagName('*');
    	            for (var i = tEls.length; i--; ) {
    	                clean(tEls[i]);
    	                if (tEls[i].tagName.toLowerCase() == 'input' && tEls[i].type == 'checkbox')
    	                    cEls[i].checked = tEls[i].checked;
    	            }
    	        }
    	        return $(clean(clone));
    	    } :
    
    	    function(contents, keepid) {
    	        function clean(el) {
    	            el.uid = null;
    	            if (!keepid) el.removeAttribute('id');
    	            return el;
    	        }
    	        var els = this.getElementsByTagName('*');
    	        for (var i = els.length; i--; ) clean(els[i]);
    	        return $(clean(this.cloneNode(!!contents)));
    	    }
    	,
    
  • Daniel Steigerwald

    Daniel Steigerwald August 26th, 2008 @ 01:52 AM

    Hell again

    Because ie6 ignore checkbox and radio fix, and because ie7 radion button loses check state when el is removed from dom, I have decide to unfixing these behaviour, just documentation will be updated :)

    
    
    clone: Browser.Engine.trident ?
    	    function(contents, keepid) {
    	        var clone = this.cloneNode(!!contents);
    	        function cleanAndFix(cloned, orig) {
    	            cloned.uid = null;
    	            if (!keepid) cloned.removeAttribute('id');
    	            var shallowClone = orig.cloneNode(false);
    	            cloned.clearAttributes();
    	            cloned.mergeAttributes(shallowClone);
    	            return cloned;
    	        }
    	        if (contents) {
    	            var cEls = clone.getElementsByTagName('*'), tEls = this.getElementsByTagName('*');
    	            for (var i = cEls.length; i--; ) cleanAndFix(cEls[i], tEls[i]);
    	        }
    	        return $(cleanAndFix(clone, this));
    	    } :
    	    function(contents, keepid) {
    	        var clone = this.cloneNode(!!contents);
    	        function cleanAndFix(cloned, orig) {
    	            cloned.uid = null;
    	            if (!keepid) cloned.removeAttribute('id');
    	            return cloned;
    	        }
    	        if (contents) {
    	            var cEls = clone.getElementsByTagName('*');
    	            for (var i = cEls.length; i--; ) cleanAndFix(cEls[i]);
    	        }
    	        return $(cleanAndFix(clone, this));
    	    },
    
    
  • Daniel Steigerwald

    Daniel Steigerwald August 26th, 2008 @ 02:00 AM

    minimized as tomocchino wished :)

    
    
    clone: function(contents, keepid) {
    	    var clone = this.cloneNode(!!contents);
    	    function cleanAndFix(cloned, orig) {
    	        cloned.uid = null;
    	        if (!keepid) cloned.removeAttribute('id');
    	        if (Browser.Engine.trident) {
    	            var shallowClone = orig.cloneNode(false);
    	            cloned.clearAttributes();
    	            cloned.mergeAttributes(shallowClone);
    	        }
    	        return cloned;
    	    }
    	    if (contents) {
    	        var cEls = clone.getElementsByTagName('*'),
                    tEls = Browser.Engine.trident && this.getElementsByTagName('*');
    	        for (var i = cEls.length; i--; ) cleanAndFix(cEls[i], tEls && tEls[i]);
    	    }
    	    return $(cleanAndFix(clone, this));
    	},
    
    
  • Tom Occhino

    Tom Occhino August 31st, 2008 @ 07:21 PM

    • State changed from “new” to “resolved”

    after many revisions, and a lot of testing... new Element.clone is committed

Create your profile

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

Shared Ticket Bins

People watching this ticket

Referenced by

Pages