- Merge request !553Issue #3008292: ImageItem::getUploadValidators() should be the source of truth for validating uploaded images โ (Open) created by phenaproxima
- ๐บ๐ธUnited States smustgrave
Think a change record will be needed since a new function is added to ImageItem.php
- Status changed to Needs work
almost 2 years ago 7:54pm 7 March 2023 - ๐บ๐ธUnited States smustgrave
For the change record and left a VERY small change in the MR.
- last update
over 1 year ago 29,389 pass, 2 fail - ๐ง๐ชBelgium thierry.beeckmans
We noticed an issue when this patch has been applied by an update of an install profile where the site does not use media library.
When GD2 image manipulation toolkit is used, the only supported types are png, jpeg, gif and webp.
An image field configured to allow only svg uploads ends up with an empty list of supported types, the validator prevents the content editors to be able to upload the svg image. - ๐ฆ๐บAustralia darvanen Sydney, Australia
@thierry.beeckmans I think out of the box Drupal doesn't support svgs in image fields, are you mixing contrib with this?
- ๐ณ๐ฑNetherlands arantxio Dordrecht
I've rerolled the MR to work on D10.2.
- last update
about 1 year ago Custom Commands Failed - ๐ฏ๐ตJapan orakili
Updated patch for 10.2.x using the new validation constraints from https://www.drupal.org/node/3363700 โ
- last update
about 1 year ago 25,786 pass, 1,827 fail - ๐ฏ๐ตJapan orakili
Sorry the patch in #76 was not replacing the
file_validate_image_resolution
validator properly. Fixed in this new patch. - ๐ฆ๐บAustralia darvanen Sydney, Australia
Why have we reverted from MR to patches? If you're providing patches to assist with patching via composer only please note that, and if you're making updates to the code please do that on the MR so the changes can be reviewed effectively :)
- First commit to issue fork.
- ๐ฌ๐ทGreece dimitriskr
I've rebased the branch to latest 11.x and incorporated some changes from the latest patches, so that someone else can work on this easily and to have automatic tests (which currently pass)
Leaving at NW due to some open comments by @wimleers, @phenaproxima and @smustgrave in the MR + no CR yet
- ๐ฎ๐นItaly nicoschi
Hi there,
how can we apply the patch from merge request in Drupal 10.2.x? At this moment the plain diff doesn't apply while the patch in #77 yes
- ๐ฎ๐นItaly nicoschi
I'll clarify the question, the only solution to continue to use the merge request commits is the application of the procedure in Creating a patch file of a Merge Request for an older stable release โ ?
Thanks
- ๐ฌ๐งUnited Kingdom scott_euser
In case an MR targeting 11x is not compatible with 10x branch, you can follow the steps here: https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... โ
- ๐ฎ๐นItaly nicoschi
@scott_euser thanks, maybe we were writing at the same time the comment, it is the same link I posted in #84.
So after that procedure should I use that as my local patch and is it useless to publish it somewhere, for example in the list of patches in this post or produce a new issue fork? - ๐ฌ๐งUnited Kingdom scott_euser
If you create a new MR branch via the issue but set to the 10.x branch, you can then subsequently hide the branch to avoid confusion, but note it in a comment. Other users can then get the patch from the MR 10.x branch (issue UI let's you show hidden branches) and download to their local project. Better not to post the actual patch here or it can become confusing about what is the latest/under active development version of the work.
- ๐ฎ๐นItaly nicoschi
nicoschi โ changed the visibility of the branch 3008292-imageitemgetuploadvalidators-should-be to hidden.
- ๐ฎ๐นItaly nicoschi
nicoschi โ changed the visibility of the branch 3008292-imageitemgetuploadvalidators-should-be to active.
- Merge request !7021Issue #3008292: ImageItem::getUploadValidators() should be the source of truth for validating uploaded images โ (Open) created by nicoschi
- ๐ฎ๐นItaly nicoschi
I did it a new branch and MR which applies to 10.2.x cherry picking commits in the 11.x, but maybe I did something wrong also with branch visibility
- ๐ฌ๐งUnited Kingdom james.williams
james.williams โ made their first commit to this issueโs fork.
- Merge request !8484Issue #3008292 by phenaproxima, dimitriskr: ImageItem::getUploadValidators()... โ (Closed) created by james.williams
- ๐ฌ๐งUnited Kingdom james.williams
I've opened MR !8484 in an attempt to get something usable with Drupal 10.3. Work from ๐ Provide a trait to create file upload validators from settings Needs review is in 10.3, but conflicts with this issue's work; I'm not entirely sure whether my attempt has resolved that satisfactorily. It looks to me a bit like these two issues have gone in divergent directions; at the least because
FileValidatorSettingsTrait::getFileUploadValidators()
only takes$field_definition->getSettings()
as input, whereas this issue requires the whole field definition to be passed around.I had a look at !7021, but that was missing changes to ImageWidget.php which seemed wrong to me. Sorry if opening yet another MR is unhelpful! It's only really to help those of us that need to be able to work with 10.3. The ideal is to get a correct MR against 11.x still.
- ๐ฌ๐งUnited Kingdom james.williams
james.williams โ changed the visibility of the branch 3008292-imageitem-getuploadvalidators-10-3 to hidden.
- ๐ฌ๐งUnited Kingdom james.williams
james.williams โ changed the visibility of the branch drupal-3008292/10.2.x to hidden.
- ๐ฌ๐งUnited Kingdom james.williams
james.williams โ changed the visibility of the branch 10.3.x to hidden.
- ๐ฌ๐งUnited Kingdom james.williams
james.williams โ changed the visibility of the branch 11.x to hidden.
- ๐ฌ๐งUnited Kingdom james.williams
james.williams โ changed the visibility of the branch 3008292-imageitemgetuploadvalidators-should-be to hidden.
- Status changed to Needs review
8 months ago 1:40pm 24 June 2024 - ๐ฌ๐งUnited Kingdom james.williams
!MR553 (which is the one for 11.x) now shows tests passing as they should, and the test-only feature failing as expected. Since the file upload in the test was switched to use the public stream, it needed a slight adjustment to take into account user access.
- Status changed to Needs work
8 months ago 1:43pm 24 June 2024 - ๐ฌ๐งUnited Kingdom james.williams
Ah sorry, this should have been left at needs work, as per comment 82:
Leaving at NW due to some open comments by @wimleers, @phenaproxima and @smustgrave in the MR + no CR yet
- ๐ฏ๐ตJapan orakili
Patch from #104 didn't apply cleanly on 10.3.1. Here's an updated patch for this version back-ported from MR 553.
- First commit to issue fork.
diederik.beirnaert โ made their first commit to this issueโs fork.