Because extension checks both the media setting and widget setting if they are not the same, the file can't be uploaded as you always get the error message for the extension doesn't match the allowed extensions set either in image or widget settings.
Steps to reproduce, in the Media field settings for image set the allowed extension to be png and then on the entity browser pixabay widget set the allowed extension to jpg, jpeg. Now go to the content and try adding either a jpg, jpeg or png image and the error will be shown. Since the allowed extension field is also required on the widget, I think it's enough to check only that, or combine/merge both and do the check there and display a more general error message for the extensions error.
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | 3116266_14.patch | 9.72 KB | slogar32 |
| #13 | 3116266_13.patch | 7.94 KB | strozx |
| #11 | 3116266_11.patch | 7.91 KB | strozx |
| #8 | 3116266_8.patch | 4.92 KB | strozx |
| #6 | 3116266_4.patch | 5.18 KB | strozx |
Comments
Comment #2
strozx commentedHello, I have written a patch for this issue. It now loads the allowed extensions from the selected media type and I also implemented a validator for allowed extensions.
Comment #3
ngkoutsaik commentedHi, thanks for the patch. I tested it on my machine and it works properly.
Comment #4
strozx commentedComment #5
strozx commentedComment #6
strozx commentedHi I updated the module some more so that it's less confusing selecting the extensions
Comment #7
deaom commentedHi @strozx, I'm not sure how I like the approach of populating the allowed extensions from the selected media as my opinion is that they can be different. So media image can have just jpeg defined, and then the pixabay one can also allow the png. With the solution provided in the patch that is not possible. But if you did choose to do it this way, then you should also disable the user input for extensions as it can never be different from the media ones.
Maybe a better approach would be to populate the extensions with the ones from media (since you went with that approach), but also allow the user to change (add or remove) extensions in the widget. Also if I decide to change extensions on the media image, I need to go back to pixabay and re-save it, to get the correct ones.
Also check out your code. Line 245 variable $media_type is never used and not needed there. Same goes to line 724 variable $extensions (you do set it but then don't use the set variable in the explode function.
So you can either remove the extensions variable or use it.
And also you have mixed cases. It seems that the camelCase should be used so don't use snake_case. So $mediaType not media_type.
Comment #8
strozx commentedHi @DeaOm, I made it so that the allowed extensions are populated form the selected media type, but the user can also select fever extensions than the ones that are allowed. Allowing the user to add extensions that are not allowed in the media type I think is wrong because this widget uses the media type. For example, if the media type has these extensions allowed (jpg, png, svg) and you want only png and jpg on the widget you can do that but you can't add to the allowed extensions of the media type. The user input isn't disabled because the user can still change the extensions he just can't add them. Also here is the new patch with removed unused lines and corrected case.
Comment #9
strozx commentedComment #10
deaom commentedHi @strozx, we will just need to agree to disagree with the adding of extensions in the widget :) In this case it probably makes sense to leave it as you implemented it. But I still have an issue, if I change the allowed extensions in the media, they do not get updated on the widget, which means I need to remove the widget and add it again, to get the updated extensions.
Everything else from the new patch seems good.
Comment #11
strozx commentedHi @DeaOm, I have looked into the issue of the non-updating extensions and came to the conclusion that an error message is best suited for this.
Regards
Comment #12
strozx commentedComment #13
strozx commentedFixed a coding standard and added more clarity to the error message.
Comment #14
deaom commentedHi @strozx and thanks for the patch. I'm just checking your implementation of the solution and it works. The patch also applies with no problems. Not quite my place to say if that is the best approach, but it gets the job done. So marking it as RTBC and if anybody will have or does have a problem with it, new issue can be opened to address it.
Comment #15
slogar32 commentedHello! I'm reopening this one because I am getting errors on the configuration form if I have more than one media type with an image source configured. I also can't add new Pixabay widgets and extensions aren't added correctly if multiple Pixabay widgets are used. There is also a PHP error in the report log.
Comment #16
slogar32 commentedNew patch created. Patch rerolled, validation fixes, ajax callback fixes, throw tags added and some coding standard fixes. Php error also fixed and removed the hardcoded label widget identifier, since the label can be changed by users. As for multiple Pixabay widgets on an entity browser, I am leaving this out, because it makes no sense.
Comment #17
slogar32 commentedComment #18
slogar32 commentedComment #19
strozx commentedHi, I have reviewed the patch and everything works so I'm settings this to RTBC.
Regards.
Comment #21
slogar32 commentedComment #22
slogar32 commentedThank you for the contributions, marking this issue as fixed and closing it.
Comment #23
slogar32 commentedComment #24
slogar32 commented