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

Bug fix: Media player disappearing on reload in Edge 16 #4022

Merged
merged 2 commits into from
Mar 5, 2020

Conversation

Dananji
Copy link
Contributor

@Dananji Dananji commented Mar 4, 2020

if (t.domNode.textTracks) {	
			for (var i = t.domNode.textTracks.length - 1; i >= 0; i--) {	
				t.domNode.textTracks[i].mode = 'hidden';	
			}	
		}

This code block hides the tracks if browser supports native captions in order to load MEJS captions. But this code block fails when it runs in Edge 16.
For one of the items this code failed in mallorn (https://mallorn.dlib.indiana.edu/media_objects/td96k2606), on the first page load there was only one textTracks item while on the reload there were 2 textTracks items. But this does not happen in Google Chrome.
And it seems the code uses VTTCue interface to handle the captions, which is not supported by Edge 16 (https://caniuse.com/#feat=mdn-api_vttcue).
Even though this fixes the issue, I'm not sure what the exact reason for this to fail on the reload.

Copy link
Member

@cjcolvar cjcolvar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know if causes different behavior (like native instead of mejs captions) on any browser/platform? Maybe Safari or mobile would be most likely to show a difference?

I guess in general I'm concerned about removing this for all browsers/platforms. Maybe it could be skipped for Edge 16 or Edge only?

@Dananji
Copy link
Contributor Author

Dananji commented Mar 5, 2020

@cjcolvar I tested on Safari on Browserstack, and seems to be working fine. Yes I agree. I'll add a browser check for it instead removing the entire code block 👍

Copy link
Member

@cjcolvar cjcolvar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@cjcolvar cjcolvar merged commit 66c3722 into develop Mar 5, 2020
@cjcolvar cjcolvar deleted the fix-edge16-mejs-bug branch March 5, 2020 22:00
@joncameron joncameron mentioned this pull request Mar 16, 2020
25 tasks
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.

2 participants