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

Caption zoom bug in min js #8425

Closed
j77h opened this issue Aug 31, 2023 · 9 comments
Closed

Caption zoom bug in min js #8425

j77h opened this issue Aug 31, 2023 · 9 comments

Comments

@j77h
Copy link

j77h commented Aug 31, 2023

Video JS versions tested

8.5.2
8.5.3
7.14.3 from the wordpress plugin

Faulty behaviour happens only with minified js of version 8.

Browser versions tested

Recent Chromium and Firefox, on Manjaro Linux.
(updated within the last two weeks)

Steps to reproduce

Put video, html, vtt files in one folder, with video.min.js and (video-js.css or video-js.min.css).

This is my video element, but I think most of the settings are not relevant,
as I first observed the behaviour before I added most of them.

  <video id="videojs1" class="video-js" muted playsinline controls preload="metadata" 
  		width="768" height="432" data-setup='{"persistTextTrackSettings": true, "preferFullWindow": true}'>
    <source src="./test.mp4" type="video/mp4">
    <track kind="subtitles" src="./test.vtt" srclang="en" label="English" default>
  </video>

The behaviour is the same with remote web server or local,
so for testing just use python -m http.server locally,
and in a browser open http://localhost:8000/test.html

Play the video, pause while a caption is visible,
and go into the caption settings dialog.

Select a font size that's not 100%, observe the change in caption size.
No need to press Reset or Done button.
Select another font size and observe the change in caption size.

Observed behaviour

Selected 150%, and caption size increased as expected;
then selected 125%, and caption size increased a further 125% (to nearly 200%).

Pressed 'Done' and played the video; the caption size remained near 200%.
At the end of the video, pressed 'Replay'; the caption size got another large increase.

Reloaded the page, played the video: caption size was at 125%, as intended.

What seems to be happening is that
any change of caption zoom is relative to the current size;
if you alternate selecting 50% and 75%, the captions become progressively smaller.

If you edit the minified js file and make the default size 150%,
then in the caption settings click 'Reset' repeatedly,
the caption size increases by 150% on each click.

Expected behaviour

The minified js file should produce the same behaviour as the readable js file, which is that
changes to the caption zoom level are always relative to the pre-determined 100% size.

@welcome
Copy link

welcome bot commented Aug 31, 2023

👋 Thanks for opening your first issue here! 👋

If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.
To help make it easier for us to investigate your issue, please follow the contributing guidelines.

@mister-ben
Copy link
Contributor

That's strange. I can replicate what you're seeing.

@ben-voytko-talogy
Copy link

ben-voytko-talogy commented Sep 13, 2023

I also see this behavior on minified version of videojs 7.21.5 with videojs-vtt 0.15.5 on Chrome on Win 10.

@mister-ben
Copy link
Contributor

mister-ben commented Sep 15, 2023

The difference is here. In the minified version the constructor name is not VTTCue, and this block is entered. In the unminified version the name matches and this is skipped.

if (cue.constructor && cue.constructor.name !== 'VTTCue') {
cue = new window.vttjs.VTTCue(originalCue.startTime, originalCue.endTime, originalCue.text);
for (const prop in originalCue) {
if (!(prop in cue)) {
cue[prop] = originalCue[prop];
}
}
// make sure that `id` is copied over
cue.id = originalCue.id;
cue.originalCue_ = originalCue;
}

This line changed in #8370 for v8, #8372 for v7

@amtins
Copy link
Contributor

amtins commented Oct 15, 2023

@mister-ben can we close this issue now that the bug has been fixed?

@j77h
Copy link
Author

j77h commented Oct 15, 2023

Is releasing a fixed version part of this process? :)
(It hasn't been done yet.)

@amtins
Copy link
Contributor

amtins commented Oct 15, 2023

@j77h this issue seems to has been fixed in video.js v8.6.1 see the PR #8442.

@j77h
Copy link
Author

j77h commented Oct 15, 2023

Ah, I saw only "Releases Latest v8.5.2" on the front page.
Now I see that 8.6.1 is a "pre-release".
I was helping a non-techie user, and I guess I'll leave her on v7 until the next "release" of v8.

@amtins
Copy link
Contributor

amtins commented Oct 15, 2023

@j77h the release process of video.js is described here videojs-releases

Version 8.6.1 should be promoted to the latest release by next Thursday.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants