fix: check element for null in case of a disposed player

[travisbader]

Description

#5706

Specific Changes proposed

Fail fast

Requirements Checklist

• Feature implemented / Bug fixed
• If necessary, more likely in a feature request than a bug fix
  • Change has been verified in an actual browser (Chome, Firefox, IE)
  • Unit Tests updated or fixed
  • Docs/guides updated
  • Example created (starter template on JSBin)
• Reviewed by Two Core Contributors

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[brandonocasey]

Should we do this in most of the functions in this file?

[gkatsev]

It's possible but generally they expose worse errors which are otherwise hidden by this null check.

[brandonocasey]

I wonder if we should log a warning in this case then @gkatsev

[gkatsev]

We spoke about it and while we still have reservations about it, if we add a log for this and do the null check we can merge it in.

[DaveStein]

@brandonocasey @gkatsev does this address my errors like this?

Uncaught TypeError: Cannot read property 'parentNode' of null
Uncaught TypeError: Cannot read property 'language' of null
Uncaught TypeError: Cannot read property 'classList' of null
Uncaught TypeError: Cannot read property 'classList' of null
Uncaught TypeError: Cannot read property 'classList' of null

in minified code I had seen it as coming from this line, when on is fired for ready event

this.readyQueue_=[],e&&e.length>0&&e.forEach((function(e){e.call(this)}),this),this.trigger("ready")}),1)},t.$=function(e,t){return jt(e,t||this.contentEl())},t.$$=function(e,t){return Ft(e,t||this.contentEl())},

[gkatsev]

@DaveStein could be. Do you happen to have a live test page for this that you can share?

[DaveStein]

@gkatsev unfortunately i haven't reproduced this yet myself but just see it in error logs.

[gkatsev]

@DaveStein if you try latest main, we just merged a fix related to the readyQueue not being cleared out properly. We don't have a release out yet, but hopefully soon.

[DaveStein]

Thanks for the update @gkatsev. Since I haven't reproduced this locally, I'll need to wait for the release to be able to update my repo and deploy it. I imagine the worst case scenario for a release update is that my issue won't be fixed. If it does, I'll comment in this thread as well so future people searching Google for the error will see this PR fixes both issues.

[gkatsev]

Yes, once we get a release out, please circle back whether it's fixed or not.

[Szefczuk]

Hi, when are you planning to merge this pr?

[damjan25]

Any plans to merge this in next release ? Or does it break something ?

[welcome]

Congrats on merging your first pull request! 🎉🎉🎉