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

HTML5Video - Fix invalid getDuration method response #50

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

joaopaulovieira
Copy link
Member

@joaopaulovieira joaopaulovieira commented Apr 2, 2021

Summary

TL;DR: Fixes clappr/clappr#1910.

Currently, the HTML5Video playback calls the _ready method even if the HTMLMediaElement is not ready yet. This PR moves the _ready call to inside the _onLoadedMetadata method, which will be called after the HTMLMediaElement loadedmetadata event occurs.

With this change, the ready event is only called after the HTMLMediaElement metadata is available.

Changes

  • html5_video.js:
    • Removes the _ready call inside render method;
    • Adds the _ready call inside _onLoadedMetadata method;
    • Simplify if statements inside render method;
    • Moves template for the to of class declaration;
  • html5_video.test.js:
    • Creates tests for new code added on _onLoadedMetadata method;

How to test

  • Try to get media duration immediately after the ready state has updated on any Clappr scope.
    • E.g.: Set the option events: { onReady: function() { alert(`player.getDuration returns: ${this.getDuration()}`) } } on Clappr initial setup.
    • The alert should be rendered with the correct media duration value.

A picture/video tells a thousand words

iMac with Firefox:

    Before this PR:
    After this PR:

iOS with Safari:

    Before this PR:
    After this PR:

@joaopaulovieira joaopaulovieira added the bug Something isn't working label Apr 2, 2021
@leaofelipe leaofelipe self-assigned this Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

player.getDuration() return NaN or 0
3 participants