This project is archived and is in readonly mode.

#661 ✓invalid
csuwldcat

Cloning a node fails to clone Element Storage properties of target node

Reported by csuwldcat | May 6th, 2009 @ 07:22 PM | in 2.0 (closed)

The current clone() does not account for the fact an element targeted for cloning has variables stored to it via Element Storage. This can be accounted for in code buy doing a retrieve on all the stored vars of the clone target then storing to the newly cloned element, but it is quite messy and inelegant. To be a true clone the cloned element should take on the same storage properties and values as the clone target.

Any thoughts Mootooligans?

Comments and changes to this ticket

  • Chris the Developer

    Chris the Developer May 6th, 2009 @ 08:49 PM

    I was about to say that this would have been a good reason to keep Storage as a Hash, but I am surprised I hadn't noticed: Hash doesn't have a clone method!

    so...what if we write up a silly little object clone function:

    
    Object.prototype.clone = function() {
    	var buffer = {};
    	for (key in this) buffer[key] = this[key];
    	return buffer;
    }
    

    and then add a storage clone function (between store and retrieve):

    
    retrieve: function(property, dflt){
    		//some stuff
    	},
    	
    	cloneStorage: function() {
    		return this.get(this.uid).clone();
    	},
    
    	store: function(property, value){
    		//some stuff
    	},
    
  • Chris the Developer

    Chris the Developer May 6th, 2009 @ 08:50 PM

    erm...i mean:

    
    return get(this.uid).clone();
    
  • Scott Kyle

    Scott Kyle May 6th, 2009 @ 09:19 PM

    Object.prototype?! My eyes!!! Noooooo!!!!!!

  • csuwldcat

    csuwldcat May 6th, 2009 @ 10:12 PM

    What about enhancing storage with options? I guess what I am thinking would work out to this syntax:

    This first way you could pass in an array of storage properties to have cloned over to the new node

    someEl.clone({storage: ['color', 'flavor', 'taste']});

    If you could passed it an "all" string it could retrieve all stored properties from the clone target and store them to the new node

    someEl.clone({storage: 'all']});

    God this would be useful for other cloning cherry picking as well.

    Just a thought.

  • Sebastian Markbåge

    Sebastian Markbåge May 6th, 2009 @ 10:28 PM

    This is a duplicate of: https://mootools.lighthouseapp.c...

    Element Storage could have things like Classes (like Fx) that point to the original element. Even if you would do deep clone you could have things like circular references or other weird stuff that can't possibly be resolved. Element.clone should remain a simple DOM element clone.

  • csuwldcat

    csuwldcat May 6th, 2009 @ 10:44 PM

    Wow I can't disagree more. Most of the time you do element storage (for me anyway) is to store actionable small data vars to a node that they relate to to access through events. What use is a stupid clone of a node without this when I have a delegated listener over the container and that one cloned node is going to fail to operate correctly when the delegate event is triggered?

    How in the heck is cloning the style properties of an element different than cloning its other crucial properties? I have a shiny new copy that looks just a great as the old one, but has the brain of Barney Fife? Right...

    So in closing, there is a way around this, and that is to clone a node, and then do .retrieve() on the target node for every prop you want to get, then do a .store() on the new clone for them all too. That is super lame however.

  • Scott Kyle

    Scott Kyle May 7th, 2009 @ 12:04 AM

    @csusldcat

    No need to get angry. I hope you read the reasoning that was given in the thread on issue #350.

    Here's an example of that:

    If you run element.tween() an Fx.Tween object gets added to that element's storage. That object contains a reference to the element. If you cloned that element AND its storage, the new clone would have that same Fx.Tween object in its storage, but that object would be referencing the original element and not the new clone.

    That is just one example of why this is a bad idea. If you like, you could create your own deepClone() method that copied all the storage except objects for instance. Take a look at the cloneStorage() method in #350 and you can adapt it from there.

  • Chris the Developer

    Chris the Developer May 7th, 2009 @ 05:59 AM

    @Scott: I didn't actually mean pushing a method onto Object.prototype - it was just to illustrate a cleaner attempt at storage cloning... :)

  • Scott Kyle

    Scott Kyle September 4th, 2009 @ 03:46 AM

    • State changed from “new” to “invalid”

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

Tags

Pages