sul-dlss / sul-embed

An oEmbed Service for Stanford University Libraries
Other
19 stars 6 forks source link

Captions are off-center if they appear on the screen prior to playback #2258

Open andrewjbtw opened 5 days ago

andrewjbtw commented 5 days ago

Expected behavior

Captions should use the same text alignment whenever they are displayed on screen.

Actual behavior

If a video starts with the captions already displayed, the text is pushed to the right. It then aligns properly after the next set of text gets loaded.

https://github.com/user-attachments/assets/c937a1ba-2c93-4ada-9987-52bfc94199ce

Steps to reproduce the behavior

Go to this item and then play it back.

Browser / Environment

Chrome, Firefox, Edge on Mac. Safari is, as usual, weird in its own way.

jmartin-sul commented 5 days ago

If you wait for the video to fully load, then this happens. If you start playing the video immediately, it might not happen.

jcoyne commented 5 days ago
Screenshot 2024-11-19 at 2 24 42 PM

It seems to be due to the "inset-inline" css rule on this element.

This is set by videojs here: https://github.com/videojs/video.js/blob/62f38446a5f01e85739bdb7207c7e992b57c1c86/src/js/tracks/text-track-display.js#L379-L385

dnoneill commented 5 days ago

It is also because the text track is on the screen when the sidebar opens.

dnoneill commented 5 days ago

Also it seems the sidebar is doing weird things to the video tag on smaller screens

dnoneill commented 4 days ago

Not the sidebar. Issue is caused by https://github.com/dnoneill/video.js/blob/main/src/js/tracks/text-track-display.js#L128-L132 being triggered after https://github.com/dnoneill/video.js/blob/main/src/js/tracks/text-track-display.js#L127

dnoneill commented 3 days ago

When I went to create a PR to fix this in videojs's main branch, the texttrackchange was being triggered after loadedmetadata thereby fixing the issue. I have added a patch to sul-embed. My guess, unless something changes with videojs that the newest release should 🤞 fix the issue.

jcoyne commented 3 days ago

@dnoneill it looks like we are not using an up to date version of videojs: https://github.com/sul-dlss/sul-embed/blob/f4f90b07953e0d8981dd5d9221dea81ceb6b00b2/config/importmap.rb#L16C54-L16C61. Have you tried updating it?

dnoneill commented 3 days ago

Yes and deployed to stage, didn't fix the problem.

dnoneill commented 3 days ago

https://github.com/sul-dlss/sul-embed/tree/update-videojs

jcoyne commented 2 days ago

@dnoneill Can you make a PR for that branch? I think we'd want that change merged in.