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

Fix: Poster hidden when item index > 0 #260

Merged
merged 2 commits into from
Apr 23, 2024
Merged

Conversation

Essk
Copy link

@Essk Essk commented Apr 22, 2024

Description

Changes in #243 prevent poster "flashing" in auto-advance scenarios but introduces a new issue whereby posters are hidden if the playlist is set to an item index of > 0, whether or not it's auto-advancing. The v6 major rewrite does address this problem however it may be some time before existing integrations can be fully tested with and migrated to v6. As an interim measure, these relatively minor changes take the idea of a parameter from v6 and implement it against the v5 codebase.

To reproduce the issue, checkout the v5.1.0 tag. Before running npm start, in index.html change the URL for the poster of the second item in the playlist from 'http://www.videojs.com/img/poster.jpg' (this is a 404) to 'http://vjs.zencdn.net/v/oceans.png'.
run npm start
Visit http://localhost:9999/
in the browser dev-tools console, enter player.playlist.next(). Observe that no poster is visible. Verify that player.poster_ shows a value of ''.

To manually test these changes, checkout this branch.
No need to update index.html :)
Visit http://localhost:9999/
in the browser dev-tools console, enter player.playlist.next(). Observe that the poster is visible and player.poster_ shows a value of http://vjs.zencdn.net/v/oceans.png.

We can verify that posters are still suppressed in an auto-advance setup by setting the auto-advance to a value in the UI on the test page and letting the playlist play in Chrome or Firefox. No poster flashes should be observed.

Specific Changes proposed

  • Add suppressPoster param to playItem, playlist.currentItem and playlist.next. The parameter defaults to false.
  • Set the parameter as true when calling playlist.next in autoadvance.setup

fixes #253

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Unit Tests updated or fixed
    • Docs/guides updated
  • Reviewed by Two Core Contributors

@Essk Essk merged commit d117f2c into 5.x Apr 23, 2024
@Essk Essk deleted the fix-deep-linked-item-poster branch April 23, 2024 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants