Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Video: Cannot close player in Firefox when plugin is installed #1439

Closed
itsthejoker opened this issue Jul 20, 2021 · 11 comments
Closed

Video: Cannot close player in Firefox when plugin is installed #1439

itsthejoker opened this issue Jul 20, 2021 · 11 comments
Assignees
Labels
bug Something isn't working released Available in the stable release

Comments

@itsthejoker
Copy link

System info:

  • Ubuntu 21.04
  • 32GB RAM
  • Ryzen 7 2700X
  • Firefox 90.0

When launching a video on the demo site (https://demo.photoprism.org/browse?view=mosaic&order=newest&public=true&quality=3) I cannot exit the video player on Firefox. Clicking around the video generates an error in the console but does nothing visually. I can close the video as expected when using Chromium.

Error:

TypeError: e.pause is not a function    app.js:2:2368793
    stop https://demo-cdn.photoprism.org/static/build/app.js?1f68e8e1:2
    onClose https://demo-cdn.photoprism.org/static/build/app.js?1f68e8e1:2
    click https://demo-cdn.photoprism.org/static/build/app.js?1f68e8e1:2
    Ue https://demo-cdn.photoprism.org/static/build/app.js?1f68e8e1:2
    n https://demo-cdn.photoprism.org/static/build/app.js?1f68e8e1:2
    _wrapper https://demo-cdn.photoprism.org/static/build/app.js?1f68e8e1:2

    Ge https://demo-cdn.photoprism.org/static/build/app.js?1f68e8e1:2
    $e https://demo-cdn.photoprism.org/static/build/app.js?1f68e8e1:2
    je https://demo-cdn.photoprism.org/static/build/app.js?1f68e8e1:2
    Ue https://demo-cdn.photoprism.org/static/build/app.js?1f68e8e1:2
    n https://demo-cdn.photoprism.org/static/build/app.js?1f68e8e1:2
    _wrapper https://demo-cdn.photoprism.org/static/build/app.js?1f68e8e1:2
@graciousgrey
Copy link
Member

Just tested it here (Linuxmint, Firefox 90.0) and it works fine.

Is anyone else observing this error?

@graciousgrey graciousgrey added the bug Something isn't working label Jul 21, 2021
@rickysarraf
Copy link
Contributor

No problem here either. Running PhotoPrism develop branch + Firefox 90.0

@lastzero
Copy link
Member

I've noticed a similar error in Firefox last year, triggered by mouse over playback on live photos. We fixed it, but maybe there's still an edge case left?

@lastzero
Copy link
Member

@itsthejoker I'm not sure how to trigger this error. Works for me on our demo using the latest Firefox on macOS and Linux. If you can reproduce it, can you please describe what exactly you're doing, or share a screen recording?

@lastzero lastzero added the waiting Impediment / blocked / waiting label Jul 22, 2021
@lastzero lastzero self-assigned this Jul 22, 2021
@itsthejoker
Copy link
Author

I did some more troubleshooting. I apologize, I've found the issue -- it's with an extension that I have installed: https://github.com/codebicycle/videospeed

Disabling that extension fixes the issue for me and I can close the video as expected. With the extension enabled, I receive the error above. I don't know if that's an edge case that you want to focus on or not, but thanks for your help and apologies. I'll go ahead and close this now.

@ecker00
Copy link
Contributor

ecker00 commented Nov 28, 2022

My family use this extension on all our machines (Vivaldi and Firefox), so we're unable to close videos. The browser console when you click outside the video to close it, logs error e.pause is not a function.

Looking a bit closer at the source code of PhotoPrism, seems that the video element returned from methods.videoEl() { this.$el.childNodes[0] } function does not get the video element with this extension enabled. Inspecting the HTML, the extension slightly modifies the HTML, as it adds a child element <div class="vsc-controller vsc-hidden"></div> above the video HTML, so the videoEl() function grabs the extension element instead of the <video>.

A slight modification to the javascript should make the PhotoPrism video player more robust and not break from small markup changes.

ecker00 added a commit to ecker00/photoprism that referenced this issue Nov 28, 2022
@lastzero
Copy link
Member

Feel free to suggest tested code changes via pull request. We are currently too busy to do this and perform all the testing necessary.

@ecker00
Copy link
Contributor

ecker00 commented Nov 28, 2022

@lastzero Found the offending line in the Vue source and created a pull request. 👍 Only changed the line so it specifically grabs the video element instead of the first child:
return this.$el.getElementsByTagName('video')[0];

ecker00 added a commit to ecker00/photoprism that referenced this issue Nov 28, 2022
lastzero pushed a commit that referenced this issue Nov 28, 2022
@lastzero lastzero added please-test Ready for acceptance test and removed waiting Impediment / blocked / waiting labels Nov 28, 2022
@lastzero lastzero changed the title Cannot close video player on Firefox Video: Cannot close player in Firefox when plugin is installed Nov 28, 2022
@lastzero lastzero reopened this Nov 28, 2022
@lastzero
Copy link
Member

I've started a development preview build for testing! It should be ready within an hour. You can then also use our demo to test it.

@ecker00
Copy link
Contributor

ecker00 commented Nov 28, 2022

Tested in both Chrome and Firefox, fix work as expected in the live demo. 👍🎉 Thank you for merging and deploying so quickly, awesome! Guess it will be included in the next official release?

@lastzero
Copy link
Member

lastzero commented Nov 28, 2022

Not 100% sure... We need to complete some backend tasks first so that users can upgrade directly in the app. Probably in the next week or two. You can temporarily use the :preview image. At the moment there are no major changes, mainly fixes and updated translations:

@graciousgrey graciousgrey added tested Changes have been tested successfully and removed please-test Ready for acceptance test labels Apr 24, 2023
@graciousgrey graciousgrey added released Available in the stable release and removed tested Changes have been tested successfully labels May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released Available in the stable release
Projects
Status: Release 🌈
Development

No branches or pull requests

5 participants