DOM nesting breaks when I add, remove, then add a Node

[arkadyp]

I pushed up this example of this code here.

'use strict';

// Famous dependencies
var DOMElement = require('famous/dom-renderables/DOMElement');
var FamousEngine = require('famous/core/FamousEngine');

// Boilerplate code to make your life easier
FamousEngine.init();

// Initialize with a scene; then, add a 'node' to the scene root
var scene = FamousEngine.createScene().addChild();

var outerNode = scene.addChild();
outerNode.setSizeMode('absolute', 'absolute', 'absolute');
outerNode.setAbsoluteSize(300, 300);
outerNode.setPosition(100, 100);
var outerNodeDOM = new DOMElement(outerNode);
outerNodeDOM.setProperty('border', '1px solid black');

var modifierNodeSync;
var innerNodeSync;
var innerNodeSyncDOM;
function addModifierPlusNodeSync() {
    modifierNodeSync = outerNode.addChild();
    innerNodeSync = modifierNodeSync.addChild();
    innerNodeSync.setSizeMode('absolute', 'absolute', 'absolute');
    innerNodeSync.setAbsoluteSize(100, 100);
    innerNodeSyncDOM = new DOMElement(innerNodeSync);
    innerNodeSyncDOM.setProperty('background-color', 'red');
    innerNodeSyncDOM.setContent('I should be inside the box [sync]');
}

addModifierPlusNodeSync();
outerNode.removeChild(modifierNodeSync);
addModifierPlusNodeSync();

var modifierNodeAsync;
var innerNodeAsync;
var innerNodeAsyncDOM;
function addModifierPlusNodeAsync() {
    modifierNodeAsync = outerNode.addChild();
    innerNodeAsync = modifierNodeAsync.addChild();
    innerNodeAsync.setSizeMode('absolute', 'absolute', 'absolute');
    innerNodeAsync.setAbsoluteSize(100, 100);
    innerNodeAsync.setPosition(200, 0);
    innerNodeAsyncDOM = new DOMElement(innerNodeAsync);
    innerNodeAsyncDOM.setProperty('background-color', 'green');
    innerNodeAsyncDOM.setContent('I should be inside the box [async]');
}

addModifierPlusNodeAsync();
setTimeout(function(){
    outerNode.removeChild(modifierNodeAsync);
    addModifierPlusNodeAsync();
}, 1);

[michaelobriena]

@adnan-wahab is looking into this

[dansimco]

+1

[CompSciFutures]

Try recursively dismounting all its children.

I think there is a fix waiting in the pull requests.

Sent from my iPhone

On 20 Sep 2015, at 6:19 pm, 42experts notifications@github.com wrote:

this one is really wrecking havoc in our app - and I am sure it will in any serious app, as long as it removes nodes with DOM elements in them. IMHO, this single bug renders the entire engine unusable in the real world - and it is indeed marked as high priority. The last movement on this is seen on July 8. It's more than two months back. Is Famous engine dead on arrival? I've taken the risk of building upon it - should I already regret that decision? I'd rather know if I need to write off the development costs and move on to some other product that has a team that actually cares.

—
Reply to this email directly or view it on GitHub.

[trusktr]

@42experts That's caused by this: #487

@Aprender I tried merging the fix locally, but it's got merge conflicts since it's been sitting so long.

[CompSciFutures]

@42experts do you mind if I see your patch? Curious to understand the problem.

[CompSciFutures]

Try this:

var Dismount = function(node) {
            var aNodes = [];
            if ( !(node instanceof famous.core.Node) )
                throw "node not a Famous#Node";

            var f = function(current) {
                aNodes.push(current);
                var aChildren = current.getChildren();
                for ( var i in aChildren )
                    f(aChildren[i]);
            };

            f(node);

            while ( aNodes.length ) {
                var x = aNodes.pop();
                if ( x.isMounted())
                    x.dismount();
            }
        }

[CompSciFutures]

Hmm, I found some interesting edge cases while poking this thing. It seems that the problem is related to having multiple leaves connected at different levels in the tree.

In some of these instances, dismounting while doing a DFS search isn't sufficient as the tree is changing mid flight and causing nodes to be overlooked. Using DFS traversal to first build up a stack of nodes to remove then iterating over that said stack to do the removal (once tree traversal is complete) is working a little better.