There is something wrong about the Max and Min resolution setting for Image field.
The values get saved to database but can then not be retrieved in the form again when editing the field settings because of a problem with explode. In the explode function there is used something strange "x" like this "ᵡ"? Also this "ᵡ" is used in some other places here. Not sure why this was chosen. I replaced "ᵡ" with normal "x" and now save and retrieve is working for me.
In addition there was some error in display, see picture.
I removed this error by using "fieldgroup" for the resolution fields and class "container-inline" to have the two fields inline. I took this sample from Field Storage Form.
Comment | File | Size | Author |
---|---|---|---|
#8 | 3363077-8-min_max_resolution_with_tests.patch | 3.08 KB | eojthebrave |
#3 | Max and min resolution not working-2363077-3.patch | 1.57 KB | stefan.korn |
#1 | Max_and_min_resolution_not_working-2363077-1.patch | 3.3 KB | stefan.korn |
resolution.PNG | 4.55 KB | stefan.korn |
Comments
Comment #1
stefan.kornComment #2
mikispeed CreditAttribution: mikispeed commentedI think there are 2 issues here:
This is working and values are displaying on the form after being entered for the first time.
I do not see this problem anymore (without the patch, current HEAD). Also, this should be submitted as a new issue.
Please reroll the patch with only first bug fixed.
Comment #3
stefan.kornYes, you are right the issue with HTML markup got fixed somewhere else between beta-2 and current HEAD.
As for the dimensions not showing in the form after beeing saved I have reworked the patch to just reflect the issue with the explode function.
Comment #4
stefan.kornComment #5
mikispeed CreditAttribution: mikispeed commentedI confirm that saved values for Min and Max resolution are showing in form after path in #3 is applied.
Comment #6
mikispeed CreditAttribution: mikispeed commentedComment #7
alexpottThis looks like a testable bug.
Comment #8
eojthebraveI added a check to see if the min/max x & y values are displayed on the fields settings form after the image field is created in ImageFieldDisplayTest::testImageFieldSettings().
Locally this test fails before applying the patch, and passes afterwards. Lets see what happens here.
Comment #9
skyredwangThe patch works, and the test works as well.
Comment #10
alexpottLet's using preg_split here so that we plit on both x and ×. This is so that the people can copy and paste from the description text.
Comment #11
stefan.kornHi,
what do you mean about copy and paste from the description text? There are two fields for resolution-x and resolution-y, so I think copy and paste from the description text will not work with x or × anyway.
Or am I missing something here?
Comment #12
alexpott@stefan.korn nope you're not missing anything. My mistake. The usage of x or × in core is quite confusing. The important thing is that file_validate_image_resolution expects a regular x which means I've discovered another bug - #2387011: EditorImageDialog passes incorrect arguments to file_validate_image_resolution
Reverting back to rtbc.
Comment #13
alexpottThis issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 5e83b6c and pushed to 8.0.x. Thanks!
Comment #15
tstoecklerThere is no discussion here at all why the more semantic "×" was replaced with the less semantic "x". The issue where this was changed is referenced here in the sidebar but not discussed here anywhere.
I would like to see some justification for this and if it is indeed needed this certainly warrants an inline code comment because the more semantic "×" is still used elsewhere around core.
Comment #16
alexpott@tstoeckler hang on - that discussion is not needed. The UI continues to display the semantic "×".
Comment #17
alexpottfor more information if we to use "×" internally we need to refactor
file_validate_image_resolution()
. But requiring a function to use "×" in its args will lead to more madness. In reality we should not store resolutions as600x400
- yep that is a regular x. We should instead store an array.