search

videojs / video.js

Video.js - open source HTML5 video player
https://videojs.com
Other
37.51k stars 7.4k forks source link

vhs.xhr.beforeRequest Is Removed On Source Reload #7605

Closed jgcaruso closed 10 months ago

jgcaruso commented 2 years ago

Description

When providing a custom function for beforeRequest it appears to work fine until the source is reloaded when using the reloadSourceOnError plugin. When reload occurs the function is removed.

It isn't clear if this is working as intended or not since this behaviour isn't described in the documentation.

When using the global videojs.Vhs.xhr.beforeRequest it works as expected, but I need to use the per-player version because I need to check player state for some options.

Steps to reproduce

I've created the following codepen snippet to demonstrate the issue https://codepen.io/jgcaruso/pen/OJxaJwX

  1. Start playback, console will show print statements every time beforeRequest is executed
  2. Click 'trigger error' to simulate an error
  3. You will notice the console statements no longer print
  4. If you change the runOnce const at the top of the page to false it will allow the beforeRequest function to be checked continuously if it is missing, and it will then be readded
  5. If you follow steps 1-2 again you will notice that the console statements continue to be printed after the error occurs.

Results

Expected

I expected beforeRequest to be maintained for the lifetime of the video player, unless modified by my code.

Actual

The beforeRequest is removed when reload is called inside of the reloadSourceOnError plugin.

Error output

Nope

Additional Information

Please include any additional information necessary here. Including the following:

versions

videojs

v7.17.0

browsers

latest firefox, chrome. (Safari doesn't appear to support beforeRequest)

OSes

macos (but I assume all OSes)

plugins

are any videojs plugins being used on the page? If so, please list them below.

The reloadSourceOnError plugin provided by videojs (no special imports, just core videojs)

misteroneill commented 2 years ago

Thanks for a nicely detailed bug report!

This does seem weird, but someone will have to look and see whether this is intentional behavior or not.

gkatsev commented 2 years ago

I believe VHS re-sets its internal xhr on load and doesn't try to keep around the beforeRequest that it was given.

stale[bot] commented 2 years ago

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.

jgcaruso commented 10 months ago

It looks like the inclusion of the new xhr-hooks-ready event and player.tech().vhs.xhr.onRequest is a more stable way of overriding beforeRequest and after refactoring my code to use it, it appears to be applied consistently before any xhr requests AND it also appears to be correctly re-applied on source change.

I'm closing this since it appears to be resolved. If I run into any new issues, I'll open a new issue.