Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix parentNode error on load callback (fixes #1471) #1483

Merged
merged 1 commit into from May 20, 2016

Conversation

@ngokevin
Copy link
Member

@ngokevin ngokevin commented May 19, 2016

Description:

@dmarcos and I don't know why this is needed. Before the callback, parentNode is defined. At the callback parentNode is briefly null. parentEl works for some reason. Have no idea what is happening to the read-only parentNode.

Changes proposed:

@@ -229,7 +229,7 @@ var proto = Object.create(ANode.prototype, {
// Entity load.
function entityLoadCallback () {
self.updateComponents();
if (self.parentNode.isPlaying) { self.play(); }
if (self.parentEl.isPlaying) { self.play(); }

This comment has been minimized.

@dmarcos

dmarcos May 19, 2016
Member

Let's add a comment here.

@ngokevin ngokevin force-pushed the ngokevin:parentel branch from bacd0d2 to e3c0f00 May 20, 2016
@dmarcos dmarcos merged commit 3dc048e into aframevr:master May 20, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@darkwing
Copy link
Contributor

@darkwing darkwing commented Jul 19, 2016

The logo demo on master is currently broken and parentEl is null, so looks like this is still an issue

@darkwing
Copy link
Contributor

@darkwing darkwing commented Aug 4, 2016

Still hitting this but here's what I've noticed with the A-Frame Logo demo where this happens twice:

  • self is <a-entity light="" data-aframe-default-light="" position="" rotation="" scale="" visible=""></a-entity>; parentEl and parentNode are both null
  • When inspecting the DOM to find elements, they're gone; instead there's: <a-light type="directional" color="#FFF" intensity="0.45" position="4 2 1" light="" rotation="" scale="" visible=""></a-light>

If elements are being created/swapped then a reference isn't being updated which is why parentNode is null.

@ngokevin
Copy link
Member Author

@ngokevin ngokevin commented Aug 4, 2016

Not being swapped, the defining lights replaces the default ones. They shouldn't be related to one another. But seems this might be a bug.

@FrederickDesimpel
Copy link
Contributor

@FrederickDesimpel FrederickDesimpel commented Aug 6, 2016

I'm running into this too, while working on lighttarget. parentEl not defined. Only on firefox. Seems to happen mainly with lights.

Edit: if i comment out 'this.system.registerLight(el);' in light component, the problem goes away.

investigating further...

@FrederickDesimpel
Copy link
Contributor

@FrederickDesimpel FrederickDesimpel commented Aug 6, 2016

Could it have something to do with this:
In systems/light.js:
32 sceneEl.removeChild(defaultLights[i]);
a regular DOM removechild deletes the element, but what happens to the light component on it then? maybe first some cleanup is needed with the threejs scenegraph ?

Or maybe parentEl / parentNode are never set on those default lights...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants