beaverbuilder / feature-requests

Feature request board for Beaver Builder products.
26 stars 4 forks source link

Allow SVG files in the Photo module #58

Open alexmansfield opened 4 years ago

alexmansfield commented 4 years ago

Is your feature request related to a problem? Please describe. Even after I install a plugin to allow SVG files to be uploaded to the WordPress directory, the Image module still has a filter that blocks them from being uploaded within the Photo module:

function bb_add_file_ext($regex) {
    $regex = array(
            'photo' => '#(jpe?g|png|gif|bmp|tiff?)#i',
        );
    return $regex;
}

add_filter( 'fl_module_upload_regex', 'bb_add_file_ext');

Proposed Solution I want to make a case for changing it to allow SVG (and probably webP) files. Here's my reasoning:

  1. The purpose of your filter is to block non-image files. SVG is an image/photo file, so your error message "The uploaded file is not a valid photo extension." doesn't make sense. There's no way to move forward from that error message to figure out why it isn't working.
  2. If you were to add SVG to your list of allowed image types, by default the upload would still fail, but it would fail with this message from WordPress: "Sorry, this file type is not permitted for security reasons." This is a more helpful message, because it explains why the upload isn't working. The obvious next step is to see why WordPress blocks SVG for security reasons, and if that isn't a concern for the site in question, there are plugins that allow SVG file uploads and even perform security checks on them after the upload.
  3. Your filter doesn't actually prevent SVG files from being used in the image module. Once SVG support is added to WordPress, SVGs can be uploaded to the media library, and once they're in the media library, then can be added to the photo module, bypassing your filter entirely. Not only that, but they are displayed perfectly.
  4. Now that I've spent hours researching this, studying the code, testing SVG support plugins, and talking to your support team, I understand how this works and how to overcome it. But to someone who doesn't code, how are they supposed to use SVG files with Beaver Builder? Or even understand why it isn't working?

So my proposal is to add SVG to the list of allowed image types within the Beaver Builder photo module. Allowing SVGs to pass your filter would have the following advantages:

  1. It would allow the WordPress error message to be displayed on upload if SVGs are still being blocked by WordPress.
  2. It would allow SVG files to be uploaded within Beaver Builder as soon as an SVG support plugin is activated, without an additional undocumented, Beaver Builder-specific step.
  3. It would get rid of the confusing situation where users can upload SVGs directly to the media library and then insert them into the photo module, but not upload them directly to the photo module.
quasel commented 3 years ago

Yes, currently I just upload it to media and then select it within BB… Why not acknowledge the file types allowed in custom WP settings?

Workaround until it's (maybe) part of bb just add a filter…

add_filter( 'fl_module_upload_regex', function( $regex, $type, $ext, $file ) {
    $regex['photo'] = '#(jpe?g|png|gif|bmp|tiff?|svg)#i';
    return $regex;
}, 10, 4 );
alexmansfield commented 3 years ago

@quasel Yes, that's what I've been doing. I just don't think that should be necessary, so I added it as a feature request.

yitwail commented 2 years ago

Here's an updated filter function that permits webp as well as svg

add_filter( 'fl_module_upload_regex', function( $regex, $type, $ext, $file ) {
    $regex['photo'] = '#(jpe?g|png|gif|bmp|tiff?|webp|svg)#i';
    return $regex;
}, 10, 4 );
Hnnx commented 1 year ago

3 years down the line and this simple fix is still not included :/ Can confirm that the filter is however working as intended - both the svg and webp solutions

Pross commented 1 year ago

fl_module_upload_regex

you dont need to add webp, it is already in the regex, we added it when WP added native support for webp the defaults are currently:

        $regex = array(
            'photo' => '#(jpe?g|png|gif|bmp|tiff?|webp)#i',
            'video' => '#(mp4|m4v|webm)#i',
        );
codente commented 1 year ago

3 years down the line and this simple fix is still not included :/ Can confirm that the filter is however working as intended - both the svg and webp solutions

When WP adds native support for SVG, then we will as well.

alexmansfield commented 1 year ago

This still doesn't seem to make sense. WordPress already blocks SVG files by default. Why does Beaver Builder add a separate process for blocking SVGs? It just creates confusion for people who have enabled SVG uploads in WordPress.

Pross commented 1 year ago

That isn't how it works. The wp media upload in wp admin does not block svg. It allows certain types of media. Same when in the builder we have to tell it what is allowed not what is blocked.

alexmansfield commented 1 year ago

I must be missing something.

Uploading SVG to vanilla WordPress

When I attempt to upload an SVG file on a vanilla WordPress install directly to the media library with no plugins installed, I get this error message: "Sorry, you are not allowed to upload this file type."

I've tried to follow the path of validation through the WordPress code base, and here's what I've come up with:

wp-admin/includes/file.php: The _wp_handle_upload() function tests uploaded files against the list of allowed mime types.

wp-includes/functions.php: The wp_get_mime_types() function stores a list of mime types that WordPress recognizes (SVG is not on this list)

wp-includes/functions.php: The get_allowed_mime_types() function specifies what mime types are allowed to be uploaded. This is pretty much just the list of mime types from wp_get_mime_types() but with SWF and EXE removed. This function also includes the upload_mimes filter which allows the list of allowed mime types to be modified.

Uploading SVG through Beaver Builder on vanilla WordPress

As far as I can tell, the upload validation built into WordPress kicks in before the Beaver Builder validation. When I try to upload an SVG in Beaver Builder with an otherwise vanilla WordPress install, I get the "Sorry, you are not allowed to upload this file type." message that is coming from WordPress itself. WordPress doesn't even attempt to load the file into the media library, and the error message shows up while still in the "Upload files" tab of the media selection modal.

Uploading SVG through Beaver Builder with SVG support added to WordPress

If I enable the Safe SVG plugin by 10up (800k active installations) or the SVG Support plugin (1+ million active installations) so that WordPress will allow SVG uploads, then I get the "The uploaded file is not a valid photo extension." error message that comes from Beaver Builder instead of the default WordPress error. It also changes tabs from the "Upload files" tab to the "Media Library" tab, shows the image for a split second and then removes it. The Beaver Builder error message is displayed in the right hand column of the "Media Library" tab instead of in the "Upload files" tab.

Uploading SVG through Beaver Builder on vanilla WordPress with SVG support added only to Beaver Builder

If I add SVG support to Beaver Builder using the filter at the top of this thread (but I DON'T add SVG support to WordPress with a plugin), I get the "Sorry, you are not allowed to upload this file type." error message from WordPress again. This seems to indicate that WordPress is blocking SVG files independent of the Beaver Builder validation.

Uploading SVG through Beaver Builder with SVG support added to WordPress AND with SVG support added to Beaver Builder

...it works. No error messages.

It seems to me that if Beaver Builder would add SVG to its list of allowed image types, it could fall back on the image validation built into WordPress itself. This would have 2 advantages:

Is there something I'm missing here?

Pross commented 1 year ago

The reason the photo module only allows images and the video module only allows videos is in the before time when there was no validation people would upload text files pdfs isos videos zip files you name it with the photo module and expect it to display them. So we added some basic feedback.

alexmansfield commented 1 year ago

That makes sense why it initially worked that way. Now that WordPress handles that, I'm not sure it still makes sense. Is there any way we can add SVG to the photo list so that Beaver Builder can default to the WordPress validation?

The Beaver Builder validation doesn't actually prevent SVGs from being added to the Photo module, it just prevents them from being uploaded through the Photo module. It seems like a strange user experience to have to go through this process:

  1. Install a plugin that allows SVGs in WordPress
  2. Edit a page with Beaver Builder
  3. Add a photo module
  4. Leave Beaver Builder
  5. Go to the media library
  6. Upload the photo directly to the media library
  7. Go back to the page they were working on
  8. Go to the settings for the Photo module they added earlier
  9. Select the SVG from the media library

If SVG was just added to the list of allowed file types for the photo module, SVG uploads would work directly in Beaver Builder (provided they were allowed in WordPress with code or a plugin).

Is there any reason not to add support for this?

Pross commented 1 year ago

Is there any reason not to add support for this?

If we add native support for it we then have to provide support for people to add svgs and use them in image tags which opens up a whole new bunch of issues like Why wont it resize to what ive set in the module? Why is it ignoring my colour settings Etc Etc

When WP add native support for svgs so will we. Until then you just need to add one line of code to your functions file.

Hnnx commented 1 year ago

After reading @Pross comments, I understand the reasoning behind not including native SVG support. I'm not entirely sure how other site builders are handling this. Best course of action is educating and letting people know about this fix, maybe including it as a footnote in official documentation.

Pross commented 1 year ago

After reading @Pross comments, I understand the reasoning behind not including native SVG support. I'm not entirely sure how other site builders are handling this. Best course of action is educating and letting people know about this fix, maybe including it as a footnote in official documentation.

Its always been in the official docs, if you search for "svg" https://docs.wpbeaverbuilder.com/beaver-builder/2.8/developer/tutorials-guides/common-beaver-builder-plugin-filter-examples/#adding-svg--other-file-type-support

murfemersed commented 1 year ago

Please allow uploading of svg in the beaver builder photo module. Should be same rules for the media upload tab as in the photo module. it's extra steps to bounce back and forth that could be easily reduced.

codente commented 1 year ago

@murfemersed you can allow for this with a filter https://docs.wpbeaverbuilder.com/beaver-builder/2.8/developer/tutorials-guides/common-beaver-builder-plugin-filter-examples/#adding-svg--other-file-type-support

Otherwise, we will support it when WP does for the reasons discussed above.

murfemersed commented 1 year ago

aaaaaah! That is excellent. As long as there is something I'm super happy with that. It's been a frustration for a while and I guess I just didn't do the homework good enough. I added it to my functions (actually a plugin I use instead of a functions file edit for things that go in my starter build). Much appreciated!!! :)

alexmansfield commented 1 year ago

If we add native support for it we then have to provide support for people to add svgs and use them in image tags which opens up a whole new bunch of issues.

I realize that there can be sizing issues with SVGs and all that, so I wasn't asking for native support for SVGs.

I was only asking to add SVG to the list of recognized image types. SVG files would still be blocked by WordPress.

Obviously people are still figuring out that they can install a plugin to add SVG support to WordPress, upload SVGs to the media library directly, and then add them to the Photo module. Or they can install the extra code to get it working via a filter.

At this point it feels like Beaver Builder is saying "Look, you've installed a plugin to try to enable SVG support, so even though you can upload SVGs to the media library, we're still going block you from uploading them directly in Beaver Builder. If you actually want to use SVGs in Beaver Builder, you have to dig into the help docs and install some extra code, or use a convoluted workflow to bypass our additional upload validation."

At the very least could I at least make a feature suggestion to change the misleading error message?

The uploaded file is not a valid photo extension.

This message was intended to let people know that the file they uploaded wasn't an image. Since SVGs are images, it makes it difficult to tell why the upload is being blocked, or at what level it is being blocked. Could it be changed to something like this?

Beaver Builder blocks SVG uploads by default.

alexmansfield commented 1 year ago

...also, since tone/feeling doesn't come across very well in text format, I just want to say that I love Beaver Builder, I'm testing the development version, filing bug reports, and I'm invested in the success of the plugin. This feels like it's one of the few areas where the plugin is working against me instead of for me, so I was just trying to explain that. Anyway, I realize that I may have sounded disgruntled or argumentative, so I wanted to clarify that wasn't my intention.