Use prefix for custom event names

[chemerisuk]

Link to related issue (if applicable)

#1581
#1623

Summary of proposed changes

At present function triggerEvent dispatches CustomEvent with name in type argument. In some cases such events conflict with system ones because use the same name. As a result folks could have weird error records in their reporting tools.

For instance the error event used to collect all information about errors on page. This event has defined structure and additional fields like source, lineno etc. But CustomEvent with the same name could be dispatched at https://github.com/sampotts/plyr/blob/master/src/js/listeners.js#L515. As the result depended event listeners could fail and display weird results.

The change in PR adds prefix to all dispatched CustomEvents to avoid any naming conflicts with existing events. I'm not sure about potential consequences of this so let me know if any.

[sampotts]

I'd like to see this configurable (extra property in the config) and opt-in so it's not a breaking change ideally. Also, worth adding to the docs.

[chemerisuk]

Hello @sampotts. On the current project we decided to use patched version of plyr.js and yes, many errors we had before gone away. Existing custom event dispatching approach is the cause of such errors.

After making the change I've also discovered a bug that produces errors with message 0. The cause fixed in ddf8332.

I'd like to see this configurable (extra property in the config) and opt-in so it's not a breaking change ideally

Keep in mind that other people have errors because of the current custom event names. I'd suggest to force using a unique prefix for event names like other projects do. This way you also avoid potential conflicts with other JS projects.

I understand this is a breaking change and it's better to publish a major release. To improve backward compatibility we can update implementation of on(). This method could append event name prefix by default so existing code that uses on() will work without any change. What do you think?

[sampotts]

I'm happy for this to wait for the next major release if that's the case but I can't guarantee when that would be. I wouldn't bump it to version 4 simply for this change that wouldn't be important for the majority of users. I've used Plyr in several production projects without any issues around event names. Many of them are specific to HTML5 video and audio elements anyway.

[chemerisuk]

Hi @sampotts, couple of updates related to this PR. It turns out custom event prefix breaks logic that handles videos in <iframe>. As a result Youtube/Video players don't work properly. So I have to rollback the change.

To solve issue with error tracking tools I've just made error event not to bubble, so it won't be catched by global handler. BTW according to documentation https://developer.mozilla.org/en-US/docs/Web/API/Element/error_event this event should not bubble. In result we don't see redundant errors in tracking tools and Youtube/Video players work properly.

What do you think about this change?

[hasan-wajahat]

Hey @sampotts after the latest changes in this PR there is no longer a prefix added to the custom event. Meaning this should not be a breaking change. Would you please have a look again and see if this can be merged now in version 3.