Editor: Make mime type config less confusing

[swissspidy]

Task Description

We currently pass this list of mime/file types to the editor:

web-stories-wp/includes/Admin/Editor.php

Lines 363 to 369 in f130e0c

	'allowedFileTypes' => $this->types->get_allowed_file_types(),
	'allowedTranscodableMimeTypes' => $this->types->get_allowed_transcodable_mime_types(),
	'allowedImageFileTypes' => $this->types->get_file_type_exts( $image_mime_types ),
	'allowedImageMimeTypes' => $image_mime_types,
	'allowedAudioFileTypes' => $this->types->get_file_type_exts( $audio_mime_types ),
	'allowedAudioMimeTypes' => $audio_mime_types,
	'allowedMimeTypes' => $mime_types,

As we are working on some documentation on using the decoupled story editor, it became more obvious how confusing this config is.

• allowedMimeTypes: map of allowed mime types for actual story elements. For example, allowedMimeTypes.image might contain image/svg+xml or something to say that you can drop SVGs on the canvas
• allowedImageMimeTypes : this is used for places in the design/document tab where you can upload images (video poster, story poster, publisher logo). For example, here we would never want to allow uploading SVGs. Hence this separate config.
• allowedAudioMimeTypes: similarly, this is used for when you want to add background audio to a page.

In addition to that, allowedFileTypes is just a simple array of file types so that in the editor we can say “You are only allowed to upload jpeg, png, gif” etc. We can just get rid of this and use getExtensionFromMimeType from the media package instead.

allowedTranscodableMimeTypes is really pointless too, which is why I opened #10591 for that already.

To-do

• Rename allowedMimeTypes to somehting more obvious, like for example allowedElementMimeTypes
• Maybe rename allowedImageMimeTypes and allowedAudioMimeTypes to something more obvious too
• Make allowedFileTypes obsolete by assembling that on-the-fly in the editor instead, using getExtensionFromMimeType

[spacedmonkey]

I wonder if we should have one object with all this data. Something like this

const fileTypes = {
    images: {
       jpg: 'image/jpeg',
       jpeg: 'image/jpeg',
       png: 'image/png',
    }, 
    videos: {
        mp4: 'video/mp4',
    },
    audio: { ...}
}

So to get allowedImageFileTypes or allowedImageMimeTypes. You would do this.

const allowedImageFileTypes = Object.keys( fileTypes.images );
const allowedImageMimeTypes = Object.values( fileTypes.images );

There is also a difference between allow images types and list images that can be uploaded. One is used for allow images for posters. How would be best denote that?

[swissspidy]

Including file types in there is redundant and makes it harder to maintain for us and everyone trying to build an editor. We can operate with just mime types and then use getExtensionFromMimeType on the client-side.

There is also a difference between allow images types and list images that can be uploaded. One is used for allow images for posters. How would be best denote that?

The suggestion above is to rename the existing fields to make that more clear.

[spacedmonkey]

We can operate with just mime types and then use getExtensionFromMimeType on the client-side.

Do we even need allowedFileTypes, allowedAudioFileTypes and allowedImageFileTypes then? Don't we just need the mime types? Meaning this could become.

const fileTypes = {
    images: ['image/jpeg', 'image/png',], 
    videos: ['video/mp4', 'video/webm'],
    audio: [ ... ]
}

So to get allowedImageFileTypes or allowedImageMimeTypes. You would do this.

const allowedImageMimeTypes  = fileTypes.images;
const allowedImageFileTypes = allowedImageMimeTypes.map( ( mimeType ) => getExtensionFromMimeType( mimeType )) ;

[swissspidy]

Precisely. That's what I meant in the ticket description by saying we can make it obsolete.

[ayushnirwal]

There might be a problem with passing down mime types and getting an extension array in the case of image/jpeg since both .jpeg and .jpg have the mime-type image/jpeg.
if allowedImageMimeTypes is ['image/jpeg'] allowedImageFileTypes would be ['jpeg'] but MediaUpload would accept both jpeg and jpg files.

[swissspidy]

Thanks for flagging that. Agreed that we need to keep that in mind.

As far as I know we only use the file extensions to display user-facing messages like You can upload jpeg, jpg, png, mp4. For MediaUpload and others, the checks are based on the mime type.

If the user-facing message now says You can upload jpeg, png, mp4 (omitting jpg and FWIW also jpe), that is OK.

[spacedmonkey]

There is a similar argument for mp4/m4v files as well. Also what if developers add a custom file type, like videopress or a new file format. It may not be part of the mine type package.

[swissspidy]

I'm not concerned about invalid mime types, we don't really support that.

[spacedmonkey]

My orignal suggestion #10592 (comment) would handle this issue.

As you can see in my sudo code shows, that structure can support multiple file extensions with the same mime types.

[swissspidy]

Not catering for mime types with multiple corresponding file extensions is fine, because file extension info is only used for error messages.

Again, I want us to avoid having to pass file extensions via config. It's redundant. So let's try it first without. If it doesn't work for some reason we can always revisit.

And something like video/videopress is not supported anyway because it's an invalid mime type.

[spacedmonkey]

I have a POC PR up here #10709.

I noticed a problem. The getExtensionFromMimeType function doesn't work as expected. Some functions return odd file extensions or nothing at all.

Take this example. audio/mpeg, returns mpga, not mp3 as expected. audio/acc returns null and not acc file.

This is an issue with the mimetype library being used. This will have to be fixed or replaces. Can you advice @swissspidy

[swissspidy]

It seems to work as expected, although it's definitely confusing at first.

This is related to @ayushnirwal's comment above.

audio/mpeg maps to multiple file extensions: mpga,mp2,mp2a,mp3,m2a,m3a. You're getting the first one.

The mime package doesn't offer a way to get all of them, see broofa/mime#254

audio/aac returns nothing because it has no extension defined in mime-db but audio/x-aac does, see https://github.com/jshttp/mime-db/blob/ebb6bf92ea39d3168a50942295d5edfdcdce641a/db.json#L6367-L6369 and jshttp/mime-db#81

It's not a huge deal because again, this is only used for error messages.

To make it less confusing it should be fine to have a hardcoded list of mime types for these cases

See #10718 for my suggestion.