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 issue with mediaelement loading image in firefox + update #834

Merged
merged 6 commits into from Apr 29, 2015

Conversation

Projects
None yet
2 participants
@mfairchild365
Copy link
Contributor

mfairchild365 commented Apr 23, 2015

It appears that firefox does not always send the canplay event, which causes the loading image to always display. This is similar to another android bug which was already 'fixed'. I don't like this method of fixing this issue, but I don't know a better way.

This PR also updates mediaelement

mfairchild365 added some commits Apr 23, 2015

Fix loading bug for firefox
Firefox will sometimes fail to send the `canplay` event, which causes the loading image to always display.  The is similar to another android bug which was already 'fixed'.  I don't like this method of fixing this issue, but I don't know a better way.
@mfairchild365

This comment has been minimized.

Copy link
Contributor

mfairchild365 commented Apr 23, 2015

@kabel I'm not sure about the correct way to update this plugin as the file structure is not 1:1 with the medialement repo. I cherry picked the files that I thought needed to be updated, but I didn't check the css/images. Should I update those too? If so, which ones?

@@ -2957,10 +3031,23 @@ if (typeof jQuery != 'undefined') {

loading.show();
controls.find('.mejs-time-buffering').show();
// Firing the 'canplay' event after a timeout which isn't getting fired on some Android 4.1 devices (https://github.com/johndyer/mediaelement/issues/1305)
if (mejs.MediaFeatures.isAndroid | mejs.MediaFeatures.isFirefox) {

This comment has been minimized.

@kabel

kabel Apr 23, 2015

Contributor

This should be a logical OR || operator, not a bitwise OR | operator.

This comment has been minimized.

@mfairchild365

mfairchild365 Apr 24, 2015

Contributor

fixed in 2e387e0

mfairchild365 added some commits Apr 24, 2015

@mfairchild365

This comment has been minimized.

Copy link
Contributor

mfairchild365 commented Apr 24, 2015

@kabel I updated the rest of the assets. Ready for another review.

@kabel

This comment has been minimized.

Copy link
Contributor

kabel commented Apr 28, 2015

I think we should fix the permissions that were changed. Images and CSS should not have the Execute unix permission on.

Do not change the permission of files
They should be 644
@mfairchild365

This comment has been minimized.

Copy link
Contributor

mfairchild365 commented Apr 28, 2015

@kabel done.

@kabel kabel merged commit 73bb8a7 into unl:develop Apr 29, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kabel

This comment has been minimized.

Copy link
Contributor

kabel commented Apr 29, 2015

I found a hopefully better way to handle this by creating a new patch for the MediaElementJS upstream. See f9da1c4.

The problem with the upstream is that (after reading the spec at http://dev.w3.org/html5/spec-preview/media-elements.html#event-media-loadeddata) the events they add listeners for are not exactly right. The loadeddata event should signal the end of loading, but that is used by mejs to show the loading interface.

@mfairchild365

This comment has been minimized.

Copy link
Contributor

mfairchild365 commented Apr 29, 2015

@kabel Nice find! It looks good to me. Where did you find that upstream patch? I can't find it anywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment