Plyr: Runtime error when using multiple plyr instances where one was destroyed

[jasonsanjose]

Run https://jsfiddle.net/jason_sanjose/s0c1g4vz/ and follow instructions in comments.

Summary:

• Multiple plyr instances in use
• Call destroy() on any one instance
• Toggle fullscreen on a remaining plyr instance

• Observe runtime error due to orphaned event handler from destroyed instance (see below)

• [x] Issue does not already exist

• [ ] Issue observed on https://plyr.io

Expected behaviour

No error

Actual behaviour

plyr.js:1 Uncaught TypeError: Cannot read property 'querySelectorAll' of null
    at B (plyr.js:1)
    at J (plyr.js:1)
    at HTMLDocument.he (plyr.js:1)

Environment

• Browser: Chrome
• Version: 63
• Operating System: Windows
• Version: 10

Players affected:

• [x] HTML5 Video
• [ ] HTML5 Audio
• [x] YouTube
• [x] Vimeo

Steps to reproduce

Run https://jsfiddle.net/jason_sanjose/s0c1g4vz/ and follow instructions in comments

Relevant links

Need a matching _off for https://github.com/sampotts/plyr/blob/master/src/js/plyr.js#L3198 when destroying a plyr instance

[sampotts]

Thanks for the refreshingly comprehensive bug report 👍

I'll see if I can replicate this in v3 as the release is imminent.

[Maqsyo]

I have a similar Problem:

plyr.js:1 Uncaught TypeError: Cannot read property 'settings' of null
    at e.toggleMenu (plyr.js:1)
    at HTMLHtmlElement.<anonymous> (plyr.js:1)

the old and destroyed plyr-instance is listening on ALL Clicks
-> so every click throws one or more errors ^^

-> _off could solve this.

3.0.0-beta.11

[sampotts]

@Maqsyo I assume you're using v3 based on that snippet? .off exists in v3 but it should be handled on destroy. Will look into that.

[pionl]

Hi, happens to me too with this use case:

• Render player (youtube in this case)
• Destroy the player (using destroy function)
• Toggle fullscreen via browser API (using fullscreen for my custom div)

Uncaught TypeError: Cannot read property 'querySelectorAll' of null
    at _getElements (plyr.js?99a4:1285)
    at _focusTrap (plyr.js?99a4:1304)
    at HTMLDocument._toggleFullscreen (plyr.js?99a4:2278)

[clark-stevenson]

In my case, I am using Angular. When routing to a screen then back (player.destroy()) and returning to the screen I recieve:

TypeError: Cannot read property 'settings' of null at Plyr.toggleMenu.

Using v3.0.0-beta.17

[clark-stevenson]

I believe my problem is coming down to this line because the event is applied to the global document but since the document isn't being removed and I don't see off anywhere, it persists after destroy.

https://github.com/sampotts/plyr/blob/beta/src/js/listeners.js#L450

A solution I have used previously is to not use anonymous functions for this but named properties bound to the instance and can be explicitly removed during destroy.

utils.on(document.documentElement, 'click', this.onDocumentClick);

[mhluska]

Any progress on fixing this issue? I see it as well when destroying and then full screening a player.

[sampotts]

Will get this fixed

[sampotts]

This should be fixed in 3.0.0-beta.18 - I can now call destroy() and get no errors. Sorry about that...

[clark-stevenson]

@sampotts thanks for your quick work on this.

On my side destroy() appears to work as expected and the errors have gone. 👍

[sampotts]

Awesome 👍

[sampotts]

Apologies - there were still some bugs left over. Try 3.0.0-beta.20 instead.

[sampotts]

Closing this one as I think it's fixed but feel free to re-open if not solved in v3 👍