fix case where removing texture (null was not equal to empty string)

[machenmusik]

Fixes #2388

[machenmusik]

easiest way to test - run local instance with this change via npm start, and load @fernandojsg's codepen on same machine, changing aframe script src to http://localhost:9000/build/aframe.js

[ngokevin]

Can we add unit tests? Both for setAttribute('material', 'src', '') and removeAttribute('material', 'src').

[machenmusik]

ok, LGTM now...

[machenmusik]

@dmarcos take a look at latest? should be much more comprehensible

[machenmusik]

specifically, latest added comments try to better explain why having a single remembrance of last texture load attempt is insufficient, per discussion on #2402

[dmarcos]

Would not be better to store all the material src within an object and not hanging directly from the shader with a strange __texture__ key? Something like shader.materialsSrc = []

[machenmusik]

I thought that the developers can arbitrarily define and use anything in the shader property namespace, so was trying to avoid collisions by using unlikely-to-collide naming, though I could be wrong. Maybe it is better to make a map with name that is unlikely to collide as you suggest, although as it is internal, I thought usual convention was to use underscore prefix.

[dmarcos]

you can then append it to the shader.material object. Safer than a strange key

[machenmusik]

While I was in there, really we don't care about the data property name, we care about the material property name, which is what we use to actually set the texture.

[machenmusik]

LGTM... @dmarcos ?

[dmarcos]

Thanks!