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.
Hey there,
I uploaded to the Captcha fonts folder an Hebrew ttf file, the module show the letters correctly and actually recognize them as it should, the only problem though is that the validation is being run from Left to Right instead of Right to Left.
So for example, if It's written 'חתול' at the image it expect me to write 'לותח' (if it was English it would expect me to type 'tac' for an image which display 'cat').
What can be done to fix this issue?
Thanks!
Comment | File | Size | Author |
---|---|---|---|
#24 | image_captcha-RTL-472450-24.patch | 3.31 KB | hejazee |
#19 | 472450_rtl_support_06.patch | 3.69 KB | soxofaan |
#18 | image_captcha_rtl_support_5.patch | 2.17 KB | ahwebd |
#16 | image_captcha_rtl_support_4.patch | 2.1 KB | ahwebd |
#14 | image_captcha_rtl_support_3.patch | 4.33 KB | ahwebd |
Comments
Comment #1
soxofaan CreditAttribution: soxofaan commentedInteresting use case.
This should be possible to solve in a clean way when #473002: Provide a way to define custom CAPTCHA validation is fixed
Comment #2
soxofaan CreditAttribution: soxofaan commenteddoes this patch fix it for you? I can't really test it because I don't speak right to left ;)
The patch makes the rendering of the code language direction dependent (using the current language direction of the current user), instead of doing the validation language direction dependent.
Comment #3
soxofaan CreditAttribution: soxofaan commentedis there any interest in this issue/patch?
I can't really test this, as I am not very familiar with RTL support.
tagging and rerolling too
Comment #4
soxofaan CreditAttribution: soxofaan commentedreroll
Comment #5
soxofaan CreditAttribution: soxofaan commentedreroll
Comment #6
ahwebd CreditAttribution: ahwebd commentedHere's a patch to add rtl support, I'm using it in my sites and it is working fine, it will add an option in admin page to make characters in captcha image start from the right if the current language is RTL.
(note: this patch was made against version: 6.x-2.2)
Comment #8
soxofaan CreditAttribution: soxofaan commentedhi Ahmed,
Why making it an option?
Does it make sense to not do RTL when the current language is RTL?
anyway, reroll of patch from #5
Comment #9
ahwebd CreditAttribution: ahwebd commentedHi soxofaan,
yes it should be an option, mainly because rtl image captcha won't work out of the box for rtl sites, the site admin needs to add a font that supports his language, and for multilingual sites he needs to set 'image_captcha_image_allowed_chars' as a multilingual variable in settings.php to allow for different character sets for each language. You can't use latin characters for an rtl language because typing latin characters in a text box will result in them being ordered from left to right (will confuse the user) as opposed to arabic characters for example.
I'll check your patch using character position in the image, but I think using array_reverse is cleaner and more straight.
Comment #10
soxofaan CreditAttribution: soxofaan commented#6: image_captcha_rtl_support.patch queued for re-testing.
Comment #11
soxofaan CreditAttribution: soxofaan commentedHi Ahmed,
Thanks for your feedback. I'm not familiar with RTL languages, so this is a bit unknown territory for me.
If I read your comment, it appears that there is much more to consider than just rendering the characters from right to left. Is the patch enough or should there be more change than just the rendering, e.g. language dependent font selection?
Comment #12
ahwebd CreditAttribution: ahwebd commentedHi soxofaan,
The patch is all what is needed from the module for rtl support, we can't bypass some steps to be done by the site admin (or more accurately, not practical) by adding more changes to the module.
Well, for single-language site the site admin just needs to ensure the font used supports the site language then in the allowed characters field he changes it with characters from site language (in my case the supplied fonts don't support arabic, so I need to add another font, otherwise arabic characters will appear as rectangular boxes in the image) .
We can't add fonts to the module to cater for all languages as this will make the module size too big.
for a multilingual site the site admin needs also to set 'image_captcha_image_allowed_chars' as a multilingual variable in settings.php , see here , this will allow him to select for example latin characters for "english" and arabic characters for arabic in field :"Characters to use in the code".
No need, the site admin can choose one (or more) font that supports all languages of the site. (generally rtl fonts support also latin characters)
Here's a patch for version 6.x-2.3 , I removed the css that makes font previews appear in multi-columns because it includes LTR specific style ( float: left; ) and would make previews appear wrong in an RTL site, also I prefer them in one column, if you prefer the other way we can add another css file for rtl and include it.
For people using Arabic I'm attaching a free font that supports arabic (it might support other languages)
(uncompress and rename to FreeSerif.ttf then put in "sites/all/libraries/fonts")
Comment #13
soxofaan CreditAttribution: soxofaan commentedhi ahwebd,
What is the licencing of that FreeSerif font? We can't just package any font with the CAPTCA module. For more info see: #497822: Drupal.org CVS policy regarding fonts and #259219: provide a better default font for image CAPTCHA.
About the CSS-tweak you did: is there in case of RTL mode a special "rtl" related class on the body element, so we could switch to right floating in that case (instead of disabling floating everywhere)?
Comment #14
ahwebd CreditAttribution: ahwebd commentedGPLv3 (see here) , However I did not mean to include it in the module (its size is a little big (1.2M))
Attached below two small size fonts that support both arabic and latin characters, they are licensed under GPLv2 ( they were downloaded and extracted from here (the package includes licensing info) )
these fonts were tested and behaved nicely with image_captcha as opposed to some other fonts where sometimes parts of the letters go outside the image (can you increase the space between the letters and the image borders to resolve this issue with some fonts)
in patch below there is an added css file for RTL
Comment #15
soxofaan CreditAttribution: soxofaan commentedHi ahweb,
Thanks for the update,
Apparently, we don't have to do
because Drupal does this automatically: see http://api.drupal.org/api/drupal/includes--common.inc/function/drupal_ad...
I would also keep the CSS files where there are instead of moving them to a new directory. It makes the patch review harder. If you look to all modules of Drupal core, you'll see that the all css files are at the same level of the .module files and not in a dedicated directory.
Comment #16
ahwebd CreditAttribution: ahwebd commentedThanks soxofaan,
new patch
Comment #17
soxofaan CreditAttribution: soxofaan commentedHi Ahmed,
thanks for the update.
I was thinking about user interface optimization/streamlining: maybe we should only show the option for RTL mode when at least one of the languages used on the site is an RTL language? What do you think?
also, I noticed some inconsistency in your patch:
Apart form that, I would also call the variable "image_captcha_rtl_support" or something instead of "image_captcha_direction", because LTR language, not unlike RTL ones, have a direction too :)
Comment #18
ahwebd CreditAttribution: ahwebd commentedHello soxofaan,
feature included in the new patch
the use context is different:
assuming the 'image_captcha_direction' variable is not set;
in "image_captcha.admin.inc" I wanted a return value of "0" to put in '#default_value' of the form while in "image_captcha.user.inc" I wanted a boolean return value to use in the "if" statement, anyway I modified the code so they look the same (end result is the same, php treats 0 as false)
This variable defines the direction of the letters in the image (from right to left or from left to right) thus its name.
The name "image_captcha_direction" describes the variable purpose exactly in the same way as "$language->direction" does, (if the value is 1 then rtl, if 0 then ltr)
Comment #19
soxofaan CreditAttribution: soxofaan commentedHi Ahmed,
I tried to merge the patch from #8 with your work, see attachment.
Nice trick to detect if there are RTL languages, by the way.
I still disagree about "direction" being a good name for a variable with boolean interpretation (it's represented as a checkbox). (Note that the values for $language->direction in Drupal core come from named constants LANGUAGE_LTR and LANGUAGE_RTL).
I decided to use the longer but more clear "image_captcha_rtl_support" instead.
Anyway, if you have no further remarks on this patch (and the test bot is happy with it), I'd like to commit it.
Comment #20
ahwebd CreditAttribution: ahwebd commentedpatch tested, working nicely
Comment #21
soxofaan CreditAttribution: soxofaan commentedThanks for testing, committed to Drupal 6 version: http://drupal.org/cvs?commit=457086
To be ported to Drupal 7 version
Comment #22
ahwebd CreditAttribution: ahwebd commentedHello soxofaan,
any news about port to D7 version ?
Comment #23
soxofaan CreditAttribution: soxofaan commentedhi Ahmed,
I don't have time to do it myself, but this would be a nice task for the co-maintainers I'm looking for, so setting back the issue tag.
Comment #24
hejazee CreditAttribution: hejazee commentedThis is #19 but using git. (works well)
Comment #26
hejazee CreditAttribution: hejazee commentedComment #27
soxofaan CreditAttribution: soxofaan commented#24: image_captcha-RTL-472450-24.patch queued for re-testing.
Comment #29
hejazee CreditAttribution: hejazee commentedI have created this patch using git
git diff > output-file.patch
So I don't know why it fails testing.
Anyway the patch works and solves the RTL problem.
I'm changing the issue status to "patch to be ported" as it was before my patch.
Comment #30
soxofaan CreditAttribution: soxofaan commentedcommitted to d7