Closed (outdated)
Project:
CAPTCHA
Version:
7.x-1.x-dev
Component:
User interface
Priority:
Normal
Category:
Feature request
Assigned:
Issue tags:
Reporter:
Created:
5 Jun 2011 at 08:16 UTC
Updated:
17 Jan 2018 at 01:27 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
soxofaan commentedI guess your site is in maintenance mode.
From image_captcha,module:
Comment #2
kylethaynes commentedIt is in maintenance mode. Sorry about that. Thank you for helping me out.
Comment #3
kylethaynes commentedClosing.
Comment #4
soxofaan commentedNo problem.
Maybe it's not a bad idea to put a message on the image captcha admin page about this, when in maintenance mode.
Something like
If someone knows a better wording, be welcome to improve it.
Also marking it as low-hanging fruit for candidate-co-maintainers.
Comment #5
soxofaan commentedfor reference, some duplicate/related issues:
#133548: Image not available when site is offline for maintenance
#407074: Image CAPTCHA in offline mode: math challenge displayed instead (feature, not a bug)
#983254: Captcha image not visible on user login page in site maintenance (offline) mode
Comment #6
kylethaynes commentedI wrote some code to implement the warning feature. I included a patch as well, but I'm not sure it'll work correctly since this is my first time using git to submit a patch. Please tell me if I didn't create the patch correctly.
Add to image_captcha.module.
Comment #7
kylethaynes commentedTests I did on this code:
1. single Image CAPTCHA on/off.
2. multiple Image CAPTCHA on/off.
3. default CAPTCHA = Image for single and multiple.
4. default CAPTCHA = Math for single and multiple.
5. combined default CAPTCHA = Image + manually set Image CAPTCHA.
5. maintenance mode with and without Image CAPTCHA activated.
The code behaved correctly under these conditions using Drupal 7.2 with Captcha 7.x-1.x-dev as of June 7, 2011.
Comment #8
soxofaan commentedhi kylethaynes,
Thanks for the patch, it applied without problems.
But I had something much simpler in mind (after all, it will be visible very rarely).
I would just always show the warning in maintenance mode, regardless whether there are image CAPTCHA's enabled. This avoids a lot of complexity and makes the feature more predictable to the end user.
Also, in your patch you add the warning to the general CAPTCHA admin page, which requires a form_alter hook. If we just add it to the Image CAPTCHA admin page, we don't need a form_alter hook, which further limits the required code.
See attached patch for what I had in mind
what do you think?
Comment #9
kylethaynes commentedWhile making my original code, I tried the solution that you are proposing. I liked the more complex solution because I thought it was more appropriate to have a warning that reacted specifically to the unexpected behavior when it occurred where the options that would turn the the unexpected behavior on and off were located. My code does a quick and optimized database search for any activated options that might require an Image->Math CAPTCHA switch. Since I'm using hook_form_FORM_ID_alter, the code is optimized to run only with the captcha_admin_settings form. I also corrected the bug where an outdated and incorrect warning_message (stuck in the drupal message array for some reason) would display on the next form after the CAPTCHA settings had been changed to a use case which did not activate the unexpected behavior. The code tracks whether or not it needs to run a check on the warning messages in the drupal message array. The code has also been tested by me in the relevant test cases as mentioned in the post above.
I've already written code that handles the more complex behavior in an optimized fashion and tested it. I've commented the code fairly well and used self-explanatory names. I'm not the maintainer, so I obviously don't have any right to decide whether the code is used or not. Personally, I like this solution better and that's why I went to all the extra trouble.
If you feel it would be better, you can use the code I wrote on the image_captcha page. If you feel it would manage the complexity better, it could be split off into a helper function.
My personal opinion is that it is more appropriate to have it on the general settings page, since the general CAPTCHA admin page will be where people will change the options that will activate the unexpected behavior, and it is where people will check first to see if they accidentally set the CAPTCHA to Math after seeing the unexpected behavior that converts Image CAPTCHAs to Math CAPTCHAs). I intentionally put in the conditional behavior to minimize the warning message display to only when it was appropriate and necessary.
Comment #10
kylethaynes commentedBy the way, this is driving me crazy... I know that you're supposed to use the issue number in the first part of the patch name, but I'm wasn't able to find it (1179062 I'm assuming by your patch name). I wanted to do everything using the appropriate conventions, but I'm still not able to find the location of that mysterious issue number.
Hmm, actually, now that I look at it... If you're interested in using my code, I think I might actually put in some more comments. The top half of the code looks rather empty to me.
Comment #11
kylethaynes commentedI went ahead and created two example helper functions of possible replacement warning behavior. One detects replacement behavior, while the other does a simple check for maintenance mode. Tell me which behavior your prefer (Option 1 or Option 2) and whether you want the message on the general CAPTCHA admin page or the Image CAPTCHA page, and I'll put together a patch for you and test it.
Option 1:
Code that detects replacement behavior and shows warning message.
Option 2:
Code to detect maintenance mode and show warning.
I also noticed on the git page that you recently submitted to branches 7.x-1.x and master. Which branch should development patches be built with?
Comment #12
soxofaan commentedHi kylethaynes,
Some remarks, in random order:
First, it is not really required to use the issue number, at most recommended. Second, you'll find the issue number nicely disguised in the url of this page. Probably one of those things that come naturally when you know it, but are black magic otherwise ;)
IMHO this is a good example of the evilness of switch statements :) I had to stare pretty long at this chunk before I got it. I think it's better to something like this:
count($form_state['input']) == 0).Working against the 7.x-1.x branch is ok.
I hope this answers most of your questions.
Comment #13
kylethaynes commentedThank you for helping me out.
What do you think about having a warning message pop up once per maintenance mode activation, and the message put on the image captcha or the general captcha admin page (which one do you like better?). That way it wouldn't be too annoying for the user. The code is pretty simple and takes up around 10 lines (if you don't count comments, spacing and word wrapping).
(This code is just for demonstration. I haven't formatted or tested it or anything.)
Comment #14
wundo commentedComment #15
priya sundharam commentedHi Team,
We tried implementing two Captchas (i.e Math & Image captcha) for a single webform in Drupal 8. User is allowed to choose either one. If user choose Math captcha then Image captcha will be hided. Vice versa for image captcha selection. But, this scenario is not allowing form to submit. While submitting form by selecting either one, I'm getting the following error
"An invalid form control with name='captcha_response' is not focusable."
Is that possible to use two captcha elements for a single form?
Need your guidance guys!
Thanks in Advance!