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(html): remove ogv file from example #34514

Merged
merged 5 commits into from
Jul 4, 2024
Merged

Conversation

OnkarRuikar
Copy link
Contributor

The demo fails on Chromium-based browsers as they only play the audio, not the video. Firefox doesn't play ogv at all.

The demo still remains relevant after removing the ogv source, because the next inline source avi isn't supported in any browser and they skip to mp4.

@OnkarRuikar OnkarRuikar requested a review from a team as a code owner June 30, 2024 10:06
@OnkarRuikar OnkarRuikar requested review from chrisdavidmills and removed request for a team June 30, 2024 10:06
@github-actions github-actions bot added Content:HTML Hypertext Markup Language docs size/xs [PR only] 0-5 LoC changed labels Jun 30, 2024
Copy link
Contributor

github-actions bot commented Jun 30, 2024

Preview URLs

(comment last updated: 2024-07-04 06:52:26)

Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

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

@OnkarRuikar The change you have made is perfectly fine, but you also need to update the corresponding description below the embedded example.

There are also some other mentions of OGG that you might want to review and update (for example in the "Server support for video" section).

@github-actions github-actions bot added size/s [PR only] 6-50 LoC changed and removed size/xs [PR only] 0-5 LoC changed labels Jul 2, 2024
Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

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

@OnkarRuikar looking good! A couple more minor suggestions for you to look at, but I've decided to approve this

files/en-us/web/html/element/video/index.md Outdated Show resolved Hide resolved
files/en-us/web/html/element/video/index.md Outdated Show resolved Hide resolved
Co-authored-by: Chris Mills <chrisdavidmills@gmail.com>
@chrisdavidmills
Copy link
Contributor

Awesome, thanks again @OnkarRuikar. Merging!

@chrisdavidmills chrisdavidmills merged commit 282fc64 into mdn:main Jul 4, 2024
8 checks passed
@OnkarRuikar OnkarRuikar deleted the patch-4 branch July 4, 2024 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:HTML Hypertext Markup Language docs size/s [PR only] 6-50 LoC changed
2 participants