Merge pull request aframevr#1928 from donmccurdy/bug-entity-detach

Remove parentEl references when entity is detached. (fixes aframevr#1444)
pamddg · Sep 26, 2016 · 0415d0d · 0415d0d
2 parents f9a48e0 + 8efc4bc
commit 0415d0d
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 3 deletions.
diff --git a/src/core/a-entity.js b/src/core/a-entity.js
@@ -102,7 +102,8 @@ var proto = Object.create(ANode.prototype, {
       if (!this.parentEl || this.isScene) { return; }
       // Remove components.
       Object.keys(this.components).forEach(bind(this.removeComponent, this));
-      this.parentEl.remove(this);
+      this.removeFromParent();
+      ANode.prototype.detachedCallback.call(this);
     }
   },
 
@@ -236,6 +237,17 @@ var proto = Object.create(ANode.prototype, {
     }
   },
 
+  /**
+   * Tell parentNode to remove this entity from itself.
+   */
+  removeFromParent: {
+    value: function () {
+      this.parentEl.remove(this);
+      this.parentEl = this.parentNode = null;
+      this.attachedToParent = false;
+    }
+  },
+
   load: {
     value: function () {
       var self = this;

diff --git a/src/core/a-node.js b/src/core/a-node.js
@@ -23,9 +23,13 @@ module.exports = registerElement('a-node', {
 
     attachedCallback: {
       value: function () {
-        var mixins = this.getAttribute('mixin');
+        var mixins;
+
+        this.hasLoaded = false;
         this.sceneEl = this.closestScene();
         this.emit('nodeready', {}, false);
+
+        mixins = this.getAttribute('mixin');
         if (mixins) { this.updateMixins(mixins); }
       },
       writable: window.debug
@@ -72,7 +76,9 @@ module.exports = registerElement('a-node', {
     },
 
     detachedCallback: {
-      value: function () { /* no-op */ }
+      value: function () {
+        this.hasLoaded = false;
+      }
     },
 
     /**

diff --git a/tests/core/a-entity.test.js b/tests/core/a-entity.test.js
@@ -47,6 +47,8 @@ suite('a-entity', function () {
     parentEl.appendChild(el);
     el.addEventListener('loaded', function () {
       assert.equal(parentEl.object3D.children[0].uuid, el.object3D.uuid);
+      assert.ok(el.parentEl);
+      assert.ok(el.parentNode);
       done();
     });
   });
@@ -72,6 +74,51 @@ suite('a-entity', function () {
     el.removeAttribute('geometry');
   });
 
+  test('can be detached and re-attached safely', function (done) {
+    var el = document.createElement('a-entity');
+    var parentEl = this.el;
+    el.object3D = new THREE.Mesh();
+    el.setAttribute('geometry', 'primitive:plane');
+    parentEl.appendChild(el);
+    el.addEventListener('loaded', afterFirstAttachment);
+
+    function afterFirstAttachment () {
+      el.removeEventListener('loaded', afterFirstAttachment);
+
+      assert.equal(parentEl.object3D.children[0].uuid, el.object3D.uuid);
+      assert.ok(el.parentEl);
+      assert.ok(el.parentNode);
+      assert.ok(el.components.geometry);
+      assert.isTrue(el.hasLoaded);
+
+      parentEl.removeChild(el);
+      process.nextTick(afterDetachment);
+    }
+
+    function afterDetachment () {
+      assert.equal(parentEl.object3D.children.length, 0);
+      assert.notOk(el.parentEl);
+      assert.notOk(el.parentNode);
+      assert.notOk(el.components.geometry);
+      assert.isFalse(el.hasLoaded);
+
+      parentEl.appendChild(el);
+      el.addEventListener('loaded', afterSecondAttachment);
+    }
+
+    function afterSecondAttachment () {
+      el.removeEventListener('loaded', afterSecondAttachment);
+
+      assert.equal(parentEl.object3D.children[0].uuid, el.object3D.uuid);
+      assert.ok(el.parentEl);
+      assert.ok(el.parentNode);
+      assert.ok(el.components.geometry);
+      assert.isTrue(el.hasLoaded);
+
+      done();
+    }
+  });
+
   suite('attachedCallback', function () {
     test('initializes 3D object', function (done) {
       var el = entityFactory();
@@ -361,6 +408,8 @@ suite('a-entity', function () {
         parentEl.removeChild(el);
         process.nextTick(function () {
           assert.equal(parentEl.object3D.children.length, 0);
+          assert.notOk(el.parentEl);
+          assert.notOk(el.parentNode);
           done();
         });
       });