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.

Sign up for GitHub

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

Jump to bottom

Ensure mp3 type is audio/mpeg in media player #7937

Merged
merged 1 commit into from Apr 1, 2021
Merged

Ensure mp3 type is audio/mpeg in media player #7937

merged 1 commit into from Apr 1, 2021

Conversation

bjester
Copy link
Member

@bjester bjester commented Mar 30, 2021

Summary

Ensures that we're passing the correct audio type for .mp3 files as it's type should be audio/mpeg.

Reviewer guidance

  1. Import audio content from channel nakav-mafak
  2. Verify that audio files play correctly in supported browsers

References

Found during investigating: #7395

Contributor Checklist

PR process:

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')

Testing:

  • Contributor has fully tested the PR manually

Reviewer Checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@bjester bjester added the TODO: needs review Waiting for review label Mar 30, 2021
@bjester bjester added this to the 0.15.0 milestone Mar 30, 2021
@bjester bjester requested a review from jonboiser March 30, 2021 22:36
@bjester
Copy link
Member Author

bjester commented Mar 30, 2021

Once @rtibbles change here is available, we can replace this with looking up the type in the JSON map.

@jonboiser jonboiser self-assigned this Apr 1, 2021
@jonboiser jonboiser added the DEV: renderers HTML5 apps, videos, exercises, etc. label Apr 1, 2021
jonboiser
jonboiser approved these changes Apr 1, 2021
View reviewed changes
Copy link
Contributor

@jonboiser jonboiser left a comment

Choose a reason for hiding this comment

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

Looks good!

@jonboiser
Copy link
Contributor

Looks good. Just looked up the list here https://www.iana.org/assignments/media-types/media-types.xhtml#audio to make sure that the mp3 -> audio/mpeg mapping was a special case. Other popular audio files seem to be more consistent.

@jonboiser jonboiser merged commit a2fb851 into learningequality:develop Apr 1, 2021
@jonboiser jonboiser removed the TODO: needs review Waiting for review label Apr 1, 2021
@bjester bjester deleted the ensure-correct-audio-type branch July 15, 2021 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonboiser jonboiser jonboiser approved these changes

Assignees

@jonboiser jonboiser

Labels
DEV: renderers HTML5 apps, videos, exercises, etc.
Projects
None yet
Milestone
0.15.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@bjester @jonboiser