From 7d1e03545f98cdb2c473d883bfc12d61a0dacd88 Mon Sep 17 00:00:00 2001 From: diarmidmackenzie Date: Sat, 5 Feb 2022 10:43:25 +0000 Subject: [PATCH 1/4] Possible fix for 4973 --- src/core/a-entity.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/core/a-entity.js b/src/core/a-entity.js index 48d207947cb..879af559189 100644 --- a/src/core/a-entity.js +++ b/src/core/a-entity.js @@ -387,9 +387,13 @@ var proto = Object.create(ANode.prototype, { // Wait for component to initialize. if (!component.initialized) { + component.pendingRemoval = true; this.addEventListener('componentinitialized', function tryRemoveLater (evt) { if (evt.detail.name !== name) { return; } - this.removeComponent(name, destroy); + if (component.pendingRemoval) { + this.removeComponent(name, destroy); + component.pendingRemoval = false; + } this.removeEventListener('componentinitialized', tryRemoveLater); }); return; @@ -486,8 +490,16 @@ var proto = Object.create(ANode.prototype, { this.removeComponent(attr, true); return; } - // Component already initialized. Update component. + // Component initialization already started. Update component. component.updateProperties(attrValue, clobber); + + // Component has been initialized, but is pending removal + if (component.pendingRemoval) { + // This new update should cancel any pending removal, and reinstate the attribute + // (which will have been removed, synchronously, when the component removal was initiated) + window.HTMLElement.prototype.setAttribute.call(this, attr, ''); + component.pendingRemoval = false; + } return; } From d51fffaa387bfbe8f8baa72759df03dcf26192fe Mon Sep 17 00:00:00 2001 From: diarmidmackenzie Date: Sat, 19 Nov 2022 16:05:49 +0000 Subject: [PATCH 2/4] Fix lint errors after merge --- src/core/a-entity.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/core/a-entity.js b/src/core/a-entity.js index 522e6d0241e..19ac59e0020 100644 --- a/src/core/a-entity.js +++ b/src/core/a-entity.js @@ -369,13 +369,13 @@ class AEntity extends ANode { // Wait for component to initialize. if (!component.initialized) { - component.pendingRemoval = true; + component.pendingRemoval = true; this.addEventListener('componentinitialized', function tryRemoveLater (evt) { if (evt.detail.name !== name) { return; } - if (component.pendingRemoval) { + if (component.pendingRemoval) { this.removeComponent(name, destroy); - component.pendingRemoval = false; - } + component.pendingRemoval = false; + } this.removeEventListener('componentinitialized', tryRemoveLater); }); return; @@ -469,13 +469,13 @@ class AEntity extends ANode { // Component initialization already started. Update component. component.updateProperties(attrValue, clobber); - // Component has been initialized, but is pending removal - if (component.pendingRemoval) { - // This new update should cancel any pending removal, and reinstate the attribute - // (which will have been removed, synchronously, when the component removal was initiated) - window.HTMLElement.prototype.setAttribute.call(this, attr, ''); - component.pendingRemoval = false; - } + // Component has been initialized, but is pending removal + if (component.pendingRemoval) { + // This new update should cancel any pending removal, and reinstate the attribute + // (which will have been removed, synchronously, when the component removal was initiated) + window.HTMLElement.prototype.setAttribute.call(this, attr, ''); + component.pendingRemoval = false; + } return; } From 59345149462bd22fbfd352145645d275d5bbf2ed Mon Sep 17 00:00:00 2001 From: diarmidmackenzie Date: Sun, 20 Nov 2022 10:38:02 +0000 Subject: [PATCH 3/4] Begin UT suite for 4973 --- tests/core/a-entity.test.js | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/core/a-entity.test.js b/tests/core/a-entity.test.js index deaa5585c63..18499377011 100644 --- a/tests/core/a-entity.test.js +++ b/tests/core/a-entity.test.js @@ -1693,3 +1693,27 @@ suite('a-entity component dependency management', function () { }); }); }); + +suite('a-entity attribute operations prior to adding to DOM', function () { + var scene; + setup(function (done) { + this.TestComponent = registerComponent('test', TestComponent); + scene = document.createElement('a-scene'); + document.body.appendChild(scene); + scene.addEventListener('loaded', () => { + done(); + }); + }); + + test('Add component before element added to DOM', function () { + var TestComponent = this.TestComponent.prototype; + this.sinon.spy(TestComponent, 'init'); + const el = document.createElement('a-entity'); + el.setAttribute('test', ''); + sinon.assert.notCalled(TestComponent.init); + el.addEventListener('loaded', function () { + sinon.assert.called(TestComponent.init); + }); + scene.appendChild(el); + }); +}); From 16b6cff2d27f5f2dc9494394acba1083b7a9f054 Mon Sep 17 00:00:00 2001 From: diarmidmackenzie Date: Sun, 20 Nov 2022 10:40:31 +0000 Subject: [PATCH 4/4] Extend UTs for 4973 --- tests/core/a-entity.test.js | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/core/a-entity.test.js b/tests/core/a-entity.test.js index 18499377011..c8c517faf04 100644 --- a/tests/core/a-entity.test.js +++ b/tests/core/a-entity.test.js @@ -1716,4 +1716,31 @@ suite('a-entity attribute operations prior to adding to DOM', function () { }); scene.appendChild(el); }); + + test('Add and remove component before element added to DOM', function () { + var TestComponent = this.TestComponent.prototype; + this.sinon.spy(TestComponent, 'init'); + const el = document.createElement('a-entity'); + el.setAttribute('test', ''); + sinon.assert.notCalled(TestComponent.init); + el.removeAttribute('test'); + el.addEventListener('loaded', function () { + sinon.assert.notCalled(TestComponent.init); + }); + scene.appendChild(el); + }); + + test('Add, remove & re-add component before element added to DOM', function () { + var TestComponent = this.TestComponent.prototype; + this.sinon.spy(TestComponent, 'init'); + const el = document.createElement('a-entity'); + el.setAttribute('test', ''); + sinon.assert.notCalled(TestComponent.init); + el.removeAttribute('test'); + el.setAttribute('test', ''); + el.addEventListener('loaded', function () { + sinon.assert.called(TestComponent.init); + }); + scene.appendChild(el); + }); });