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

[Mime] update types map #41016

Merged
merged 1 commit into from May 7, 2021
Merged

[Mime] update types map #41016

merged 1 commit into from May 7, 2021

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 5.x
Bug fix? no
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

@@ -1192,15 +1224,15 @@ public function guessMimeType(string $path): ?string
'audio/mp4' => ['m4a', 'mp4a', 'f4a'],
'audio/mpeg' => ['mp3', 'mpga', 'mp2', 'mp2a', 'm2a', 'm3a'],
'audio/mpegurl' => ['m3u', 'm3u8', 'vlc'],
'audio/ogg' => ['oga', 'ogg', 'spx', 'opus'],
'audio/ogg' => ['ogg', 'oga', 'spx', 'opus'],
Copy link
Member

Choose a reason for hiding this comment

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

we should probably keep the existing order

Copy link
Member Author

Choose a reason for hiding this comment

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

ogg is really the preferred file extension, see eg https://fr.wikipedia.org/wiki/Ogg
we already changed the order eg in symfony/mime@aa1d922
This one is now autogenerated, so it will stay stable.

@@ -1297,10 +1329,13 @@ public function guessMimeType(string $path): ?string
'font/collection' => ['ttc'],
'font/otf' => ['otf'],
'font/ttf' => ['ttf'],
'font/woff' => ['woff', 'woff2'],
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure we should remove existing extensions

Copy link
Member Author

Choose a reason for hiding this comment

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

it's not been removed: it's been split in two distinct mime types, one for each version.

Copy link
Member Author

Choose a reason for hiding this comment

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

I had a second look and I think we should keep the work from the upstream source. They made the split and there should be a reason for it.

@driesvints
Copy link
Contributor

FYI: this introduced a small regression for our test suite but I've sent in a PR to work around it: laravel/framework#37332

@nicolas-grekas This is a similar change to #39225 which broke a few things when Symfony 5.2 was released. Just want to make you aware of that.

@nicolas-grekas
Copy link
Member Author

Thanks for the ping. I think the change makes sense here, .mov is more common than .qt, so updating on laravel's side looks appropriate to me.

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

Successfully merging this pull request may close these issues.

None yet

4 participants