Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Height and width values should be formally divided with an ×-mark (×
in HTML) instead of the letter x.
It is a multiplication sign.
Comment | File | Size | Author |
---|---|---|---|
#40 | use_instead_of_x-657166-40.patch | 10.93 KB | mgifford |
#36 | Screen Shot 2014-09-24 at 11.52.22 AM.png | 170.04 KB | mgifford |
#35 | use_instead_of_x-657166-35.patch | 7.52 KB | mgifford |
#34 | use_instead_of_x-657166-34.patch | 6.64 KB | mgifford |
#33 | use_instead_of_x-657166-33.patch | 6.22 KB | mgifford |
Comments
Comment #1
Tor Arne Thune CreditAttribution: Tor Arne Thune commentedStill a valid issue. See for example the description text for a user picture upload form. Moving to 8.x.
Comment #2
droplet CreditAttribution: droplet commentedsubscribe
Comment #3
Brandonian CreditAttribution: Brandonian commentedsubscribe
Comment #4
HazaHere are some changes about that. I also change the test according to the new × instead of x
Comment #5
HazaComment #6
droplet CreditAttribution: droplet commentedwhy it's using str_replace ??
Comment #7
HazaBecause the informations is still save with the x (the letter), not the × so the only purpose of this replace is to display a × instead of the x when displaying the data. (but I know, that still might be a bad idea ... )
Comment #8
Damien Tournoud CreditAttribution: Damien Tournoud commentedThis (and others) look wrong. The % in %dimensions means that the argument is plain-text. As consequence it should not contain HTML. Moreover, we try to avoid using entities, and prefer straight Unicode characters.
Comment #9
HazaGot it, what about this one so ?
Comment #10
TR CreditAttribution: TR commentedReposting patch from #9 without modification, in attempt to get the testbot to run.
Comment #12
TR CreditAttribution: TR commentedRe-rolled patch against current D8 head, and in -p1 format for the testbot.
Comment #13
irunflower CreditAttribution: irunflower commentedPatch did not work. Could not get it to apply to 8.x-dev.
I get: fatal: corrupt patch at line 139
Comment #14
heaths1 CreditAttribution: heaths1 commentedRerolled to the recent head. Noob input.
Comment #15
enhdless CreditAttribution: enhdless commentedPatch needs rerolling again.
Comment #16
TR CreditAttribution: TR commented14: 657166-14.patch queued for re-testing.
Comment #18
mgiffordOk, this needs a bunch of work. Where is the image_resize_effect() function? I could only find the test:
grep -ir image_resize_effect core/modules/
core/modules//image/src/Tests/ImageEffectsTest.php: * Test the image_resize_effect() function.
same for function image_crop_effect() & image_scale_and_crop_effect().
or for that matter core/modules/user/lib/Drupal/user/AccountFormController.php
This still needs work obviously, but wanting the bot to test it.
Comment #19
tstoecklerI think that should not be necessary.
Instead everywhere where 'x' is still used we should use the proper character instead. I could find the following places that still need to updated:
EditorImageDialog
ImageItem
(there's some instances of x there)Comment #20
mgiffordOk, I dropped the str_replace() as requested and added it to EditorImageDialog & ImageItem. Let me know if I missed anything.
Comment #22
mgiffordHope this addresses the failed test in core/modules/image/src/Tests/ImageFieldDisplayTest.php
Comment #24
cs_shadow CreditAttribution: cs_shadow commentedThis should take care of the tests.
Comment #25
cs_shadow CreditAttribution: cs_shadow commentedComment #26
TR CreditAttribution: TR commentedYou forgot to change one of the "WIDTHxHEIGHT" that appears in the patch.
Comment #27
mgiffordOk, this should address that. Thanks for pointing it out TR.
Comment #29
cs_shadow CreditAttribution: cs_shadow commentedSeems like a very unrelated failure to me.
Comment #32
mgiffordI posted a version of the code on SimplyTest.me - admin/structure/types/manage/article/fields/node.article.field_image
I noticed it isn't in admin/config/media/image-styles when I was looking at this just now... That's probably the biggest place where it is visible in the UI.
Comment #33
mgiffordNow with changes to the image module.
Comment #34
mgiffordSorry for the noise. Missed the YML file.
Comment #35
mgiffordand small/medium too apparently..
Comment #36
mgiffordScreenshot with ×
Comment #40
mgiffordWith tests..
Comment #41
tstoecklerAwesome, thanks! The patch is now a simple search and replace, which is how it should be. The various test failures show that this is sufficiently tested.
Comment #42
Wim LeersWonderful :)
Comment #43
catchCommitted/pushed to 8.0.x, thanks!
Comment #45
mgiffordComment #46
yannickooAlmost 5 years now, wow :)
Comment #48
alexpottThis broke stuff. We should have only change UI text here. See #2363077: Max and min resolution not working and #2387011: EditorImageDialog passes incorrect arguments to file_validate_image_resolution