h5p / h5p-audio

10 stars 44 forks source link

HFP-3776 Catch play promise error #86

Closed otacke closed 1 year ago

otacke commented 1 year ago

When merged in, will catch potential play-Promise errors that can arise when autoplaying.

edgecity13 commented 11 months ago

Hello, I am the person who is experiencing the bug. I see a change was implemented on September 20. Looking at the releases here: https://h5p.org/post-hub-releases it looks like that fix hasn't been released yet. Do you know when it will be released so I can test it? thanks.

Gwenillia commented 11 months ago

Hello @edgecity13 Correct, it's not been released yet, we're still working on it among other issues part of its release. We'll roll it out in an upcoming update.

edgecity13 commented 11 months ago

Thanks for the quick response Gwenillia. Would it be possible to learn the exact code change and how to apply it so we can try and fix it now? I ask because we have about 1500 instances of the audio player in 3 moodle installs with many users affected by this. Thanks.

otacke commented 11 months ago

@edgecity13 If you want to patch this yourself (know what you're doing!), you can find all the changes by clicking on Files changed at the top.

edgecity13 commented 11 months ago

Thanks Oliver @otacke . I am comfortable patching a script. I just can't find the location of scripts/audio.js for h5p in the moodle directories. Do you know where scripts/audio.js lives?

edgecity13 commented 11 months ago

Oliver @otacke I found it. It is stored in the moodle data directory through a hash value. Thanks.

Here's what I changed, is this correct?

250 // Audio element is visible. Autoplay if autoplay is enabled and it was 251 // not explicitly paused by a user 252 self.autoPaused = false; 253 self.play(); 254 } 255 }, { 256 root: document.documentElement,

300 this.flowplayer.play(); 301 } 302 if (this.audio !== undefined) { 303 // play() returns a Promise that can fail, e.g. while autoplaying 304 this.audio.play().catch((error) => { 305 console.warn(error); 30 });

otacke commented 11 months ago

@edgecity13 You can check https://github.com/h5p/h5p-audio/blob/master/scripts/audio.js for how the whole script currently looks like.

There's no (direct) way of knowing where the file that you need to change is located, because H5P needs to use moodle's file API in order to handle files, and that API doesn't simply copy files in some directory.

Please mind caching when you make your change.

Also be aware that you may be messing with other people's systems if you patch H5P content type files: If they download H5P files from your server and upload it on their H5P integration - if H5P.Audio is not available there in the version that you serve - the missing modified file may be installed. Not likely, but I told you now. (@Gwenillia That's why https://h5p.org/node/1242568 feels important. Hope that H5P Group is going to implement something like that at some point.)

A clean way of doing those kind of changes would be to use H5P's alter_scripts hook (cmp. https://h5p.org/documentation/for-developers/authoring-tool-customization), and to override functions of H5P content types programmatically, but that requires some coding and may come with some roadblocks of its own.

edgecity13 commented 11 months ago

@otacke I did find the file, the moodle database has a record that stores the hash and location of the file audio.js. I did clear the moodle cache, and also first made sure all the instances of the audio player were updated to the latest version via moodle admin. I'm going to recheck my work as the patch did not appear to resolve the issue. I will compare to https://github.com/h5p/h5p-audio/blob/master/scripts/audio.js to be sure. Thanks for your help.

edgecity13 commented 11 months ago

Hi @Gwenillia and @otacke,

We still see an issue on iPhones only. Here's what I have done.

  1. Upgraded all the audio player instances in Moodle to 1.5.3
  2. Replaced the audio.js file with the latest version at https://github.com/h5p/h5p-audio/blob/master/scripts/audio.js to ensure I have incorporated all the changes available.
  3. Tested on iPhones after having cleared all browser data and the Moodle caches. 4: Here is a screenshare video of the issue being reproduced on an iPhone 13: https://nvcacademy.zoom.us/rec/play/uEj3DOoPCtBjWkjoLv6IVtTTeLNTynRVnrsbZi_ctIa9ZmrJ2nmLjteu2_HazOVBWDGBB9al9Q9REBIO.wFBm68lHCcMISlUG

Summary of reproducing the issue:

  1. Using an iPhone with any common browser, browse to page with an h5p audio player.
  2. Play the audio for at least 15 seconds.
  3. Tap Pause
  4. Hit the back button to exit the player page.
  5. Re enter the player's page.
  6. See that the player has reset back to zero, and not retained the position.
  7. Click Play, the audio will play.
  8. Try scrubbing the timeline, it will not scrub forward or backward anymore.
edgecity13 commented 10 months ago

Hi @Gwenillia and @otacke, bumping this reply...

We still see an issue on iPhones only. Here's what I have done.

Upgraded all the audio player instances in Moodle to 1.5.3 Replaced the audio.js file with the latest version at https://github.com/h5p/h5p-audio/blob/master/scripts/audio.js to ensure I have incorporated all the changes available. Tested on iPhones after having cleared all browser data and the Moodle caches. 4: Here is a screenshare video of the issue being reproduced on an iPhone 13: https://nvcacademy.zoom.us/rec/play/uEj3DOoPCtBjWkjoLv6IVtTTeLNTynRVnrsbZi_ctIa9ZmrJ2nmLjteu2_HazOVBWDGBB9al9Q9REBIO.wFBm68lHCcMISlUG Summary of reproducing the issue:

Using an iPhone with any common browser, browse to page with an h5p audio player. Play the audio for at least 15 seconds. Tap Pause Hit the back button to exit the player page. Re enter the player's page. See that the player has reset back to zero, and not retained the position. Click Play, the audio will play. Try scrubbing the timeline, it will not scrub forward or backward anymore.

Gwenillia commented 10 months ago

Hi, The current PR is about fixing an error that could break the content. And this seem to be solved.

The problem you're experiencing with the player not retaining the last position but resetting is known, we have to investigate more on this issue.

We found so far that it is only reproducible on physical iPhones machine and not through simulation or development tools that supposedly reproduces iOS and its behaviours, also it isn't reproducible in all iOS versions.

Thanks for all the informations you provided —especially the advancing forward / backward issue and the recording.— I will add them to what we have.

Thanks for your time.

edgecity13 commented 10 months ago

Thanks @Gwenillia, do you have a URL where I can track progress on the iPhone issue?

Gwenillia commented 10 months ago

Sure, this ticket was private, but we made it public now. You should be able to access it there.

https://h5ptechnology.atlassian.net/browse/HFP-3853