Hello,
I have developed a module, called Invisible Captcha (http://blog.ag-prime.com/2009/06/drupal-captcha-invisible-captcha-challe...) and would like to include it CAPTCHA Pack module.

Please review and see the latest version of my code.

CommentFileSizeAuthor
invisible_captcha.zip4.07 KBgabrielu
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

soxofaan’s picture

Title: I would like to contribuite Invisible Captcha module » I would like to contribute Invisible Captcha module

Hi, thanks for your work

I would recommend to contribute the Invisible CAPTCHA module on its own, separate from the CAPTCHA Pack module. The CAPTCHA Pack module already contains a fair amount of sub-modules an this pollutes the admin user interface. I am even considering to slim down the modules in CAPTCHA Pack. I thinks it would be cleaner and easier to offer it as a separate module.
If you don't have the right permissions right now, I could create the project for you if you want.

Anyway, here is some review of the code (I didn't try it out yet, I only read through the code).

invisible_captcha.info

description = You can secure your forms, without anoying users with indescifrable captcha's.

Some typos here: annoying, indecipherable, CAPTCHAs (in capitals)
I would also make the description more clear:

description = "A CAPTCHA that is invisible to normal users and uses javascript and cookies to protect your forms against spam bots."

invisible_captcha.module

You seem to be using tabs for indentation, please use 2 spaces as specified in http://drupal.org/coding-standards

What happens when user has no javascript or cookies, I only see a commented line

 				//$conf['captcha_description'] = '<noscript>'.t('Please set your browser to use javascript before using this form.').'</noscript>';

What is the fall back?

global $conf;
$conf['captcha_description'] = '';

Document why this is needed.

$captcha['form']['captcha_response'] = array(
  '#type' => 'hidden',
  '#id' => 'captcha_response',
  '#process' => array('invisible_captcha_process'),
);

there is '#id' => 'captcha_response',, but what happens when there are multiple forms with a CAPTCHA on the same page?

 /**
 * Implementation of hook_form_alter().
 *
 * This function adds a CAPTCHA to forms for untrusted users if needed and adds
 * CAPTCHA administration links for site administrators if this option is enabled.
 */

I think this documentation does not correspond to the code

	if (arg(0) != 'admin' || variable_get('captcha_allow_on_admin_pages', FALSE)) {

	    module_load_include('inc', 'captcha');

	    if (!user_access('skip CAPTCHA')) {

	    	$captcha_point = captcha_get_form_id_setting($form_id);
	    	if ($captcha_point && $captcha_point->type && ( strpos('Invisible CAPTCHA',$captcha_point->type) !== false ) ) {

This is a lot of duplication from the CAPTCHA module. Can't this be done easier?

in invisible_captcha_form_alter(&$form, $form_state, $form_id) you call

				captcha_form_alter($form, $form_state, $form_id);

So this function is called twice for each form? why is that?

Why is there:

function invisible_captcha_help($path, $arg) {

It will never be used as far as I can tell.

script.js

I think this script deserves a better name, why didn't you use 'invisible_captcha.js'?

I didn't have time to dig through the javascript, but it looks there is more code than I would expect

soxofaan’s picture

Status: Active » Needs work

For extra inspiration: I found this tutorial about a comparable solution: http://docs.jquery.com/Tutorials:Safer_Contact_Forms_Without_CAPTCHAs

gabrielu’s picture

Hello,
I didn't expect such a detailed review.
You have some pretty good suggestions.
I agree that this would better be a stand-alone project. I will make time to fix the issues.

Thanks for the feedback. I will come back this days with more info / updates.

AlexisWilke’s picture

Hi Gabrielu,

Did you create another project for your CAPTCHA?

I have a version too, that only uses CSS which I think is safer than using JavaScript.

So if you have your own project, we could probably include my version in your project since the CAPTCHA pack author does not want more in his pack (which I don't understand why since these are sub-folders we can delete if we don't want them?!)

Anyway... this was Dec 2009... so that's a while back already!

Thank you.
Alexis Wilke

gabrielu’s picture

Well true, I ddin't had the time to take care of this. There isn't any module created yet.
I think we can both make a Invisible captcha, where user could select the method, CSS or Javascript (btw, there are numerous Invisible captcha methods, we could provide some of them, and finally the user could select the most effective one).

We shall discuss.

AlexisWilke’s picture

As an FYI, I have created such an option since then:

http://drupal.org/project/hidden_captcha

The Hidden CAPTCHA uses an external CSS for right now, but we may add other features as we move forward.

Thank you.
Alexis Wilke