Prevent error when name is null or undefined (fixes #8804)

[YonatanKra]

fixes #8804

[cesium-concierge]

Thank you so much for the pull request @YonatanKra! I noticed this is your first pull request and I wanted to say welcome to the Cesium community!

The Pull Request Guidelines is a handy reference for making sure your PR gets accepted quickly, so make sure to skim that.

• ❌ Missing CONTRIBUTORS.md entry.
  • Please add yourself to the contributors file!
• ✔️ Signed CLA found.
• ❔ CHANGES.md was not updated.
  • If this change updates the public API in any way, please add a bullet point to CHANGES.md.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

[mramato]

Thanks for the PR @YonatanKra. I'll need to find some time to dig into this a little more to make sure the check happens in the right place. If it happens for name, I would expect it happens for some other properties as well so there might be a more central place for the check.

[YonatanKra]

@mramato
It doesn't happen with other properties AFAIK: sandcastle

I looked at the properties involved.
Name is the only one that's not prefilled:

cesium/Source/DataSources/Entity.js

Line 144 in c3e375d

this._billboard = undefined;

The error happens because name is added in the constructor. Because it is undefined/null,

cesium/Source/DataSources/Entity.js

Line 604 in c3e375d

if (!defined(targetProperty) && propertyNames.indexOf(name) === -1) {

is true so it tries to addProperty("name"). Problem is, "name" in this is true - hence, it throws the error.

Actually... a better fix would be to just remove name from the Entity constructor... what do you think?

[YonatanKra]

@mramato
I took another look at the code.
I pushed a more robust solution.
I think the verification was all wrong in the Entity's merge method.
Let me know what you think.

[hpinkos]

bump @mramato

[cesium-concierge]

Thanks again for your contribution @YonatanKra!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[mramato]

Thanks again for the PR @YonatanKra! The issue here was a little more subtle than I thought and also affected availability, so I ended up pushing both a change to sync this branch up with master as well as provide a more complete fix. I'll merge as soon as CI passes and this will go out with the release on the 1st.