Browser autocomplete feature does not seem desirable for the capcha response form field.

Index: image_captcha.module
===================================================================
--- image_captcha.module (revision 690)
+++ image_captcha.module (working copy)
@@ -230,7 +230,6 @@
           '#weight' => 0,
           '#required' => TRUE,
           '#size' => 15,
-          '#attributes' => array('autocomplete' => 'off')
         );
         // Handle the case insensitive validation option combined with ignoring spaces.
Files: 
CommentFileSizeAuthor
#33 captcha.patch2.5 KBelachlan
PASSED: [[SimpleTest]]: [MySQL] 700 pass(es).
[ View ]
#24 captcha-6.x-2.3-branch-[881156].patch2.65 KBelachlan
FAILED: [[SimpleTest]]: [MySQL] 579 pass(es), 3 fail(s), and 0 exception(es).
[ View ]
#23 captcha-6.x-2.3-branch-[881156].patch2.71 KBelachlan
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in captcha-6.x-2.3-branch-[881156].patch.
[ View ]
#19 captcha-[881156].patch2.59 KBelachlan
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#17 captcha-[881156].patch691 byteselachlan
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in captcha-[881156]_2.patch.
[ View ]
#15 captcha-[881156].patch690 byteselachlan
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in captcha-[881156]_1.patch.
[ View ]
#12 captcha-[881156].patch690 byteselachlan
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in captcha-[881156]_0.patch.
[ View ]
#6 captcha-[881156].patch1.26 KBpavel.karoukin
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#2 captcha-[893170].patch1.29 KBvivekkhurana
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch captcha-[893170].patch.
[ View ]

Comments

Component:Image Captcha (image_captcha)» User interface
Status:Needs review» Needs work
Issue tags:+captcha, +low-hanging fruit

Good idea.

Should also be applied to simple math CAPTCHA.

Flagging it as "low-hanging fruit", so aspirant co-maintainers can tackle this one

StatusFileSize
new1.29 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch captcha-[893170].patch.
[ View ]

Patch attached which adds autocomplete off to image_captcha and math captcha.

Version:6.x-2.3-rc2» 6.x-2.x-dev
Status:Needs work» Needs review

Hi Vivek,

Thanks for your patch!
The CAPTCHA module has simpletest coverage, so when you submit a patch, you should set the status to "needs review" so the automated testing system can do its job.

Status:Needs review» Needs work

The last submitted patch, captcha-[893170].patch, failed testing.

Version:6.x-2.x-dev» 7.x-1.x-dev
Status:Needs work» Patch (to be ported)
Issue tags:-captcha

Ok, the patch looked all right to me so I committed it:
http://drupal.org/cvs?commit=439220
I committed it before the test system kicked in and that's probably why it reported failure of applying the path.

Anyway: thanks a lot and to be ported to D7

Status:Patch (to be ported)» Needs review
StatusFileSize
new1.26 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Ported to D7

Status:Needs review» Fixed

Status:Fixed» Needs work

This change breaks XHTML compliance.

It is suggested in this article to use !ATTLIST just after the Doctype - https://wiki.mozilla.org/The_autocomplete_attribute_and_web_documents_us...

eg.

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd" [
<!ATTLIST form autocomplete CDATA #IMPLIED>
]>
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en">

Title:Add autocomplete off for captcha_response form fieldXHTML compliance of "autocomplete=off" on captcha_response form field
Issue tags:+xhtml strict

(changing title accordingly)

With that, my xhtml properly validates, no need to create a custom DTD. Only gotcha is that you must send the application/xhtml+xml header with the document, but if you are using xhtml, you should be sending that header anyway.

So #8 solution will work only if page served as XHTML page. If it served as HTML page - this will still fail?

If so, may be we should stick to JS solution? I.e. injecting something like:

  document.getElementById( "MyInput" ).setAttribute( "autocomplete","off" )

Add this to the captcha.js file

//Turn off Auto-Complete for input feild
$("#edit-captcha-response").attr("autocomplete", "off");

How can we run a test for it?

StatusFileSize
new690 bytes
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in captcha-[881156]_0.patch.
[ View ]

Patch with changes. I did it manually as I don't have CVS.

Status:Needs work» Needs review

Patch needs tests.

Also the original changes will need to be reverted.

Status:Needs review» Needs work

The last submitted patch, captcha-[881156].patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new690 bytes
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in captcha-[881156]_1.patch.
[ View ]

Resubmitting patch. Hopefully fixed.

Status:Needs review» Needs work

The last submitted patch, captcha-[881156].patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new691 bytes
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in captcha-[881156]_2.patch.
[ View ]

Learning the hard way not to do it manually. If this fails I'm going to leave it to someone else.

Status:Needs review» Needs work

The last submitted patch, captcha-[881156].patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.59 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

OK, now it should work.

elachlan,

Few suggestions:
1) Make sure to use jQuery() instead $(). It will be more safe to use this way with other JS libraries which can overtake $()
2) Why "captchaAdmin"? I believe something like plain Drupal.behaviors.Captcha would be better, or may be this behavior already taken by something?

it should be similar to image_captcha.js

//TODO: change this to Drupal.behaviors.captchaadmin = function (context) {}
$(document).ready(function(){
+ //Turn off Auto-Complete for input feild
+ $("#edit-captcha-response").attr("autocomplete", "off");

But as you can see we are moving it to "captchaadmin". I believe it has the same function as document.ready?

Conversion of the image_captcha.js would be a good idea soon as well.

Edit: I believe jQuery is used, it references it at the end of the file. Also $() is used throughout the file.

Priority:Normal» Major

XHTML compliance should be extended to the 6.x-2.3 branch as well, as it is required by many professional website developers and customers. Raising importance, since it is also important to have autocomplete turned off. The JQuery method in #21 should work well to fix the compliance issues.

Note: The $(document).ready(function(){ is the generic way of naming any function that is to happen after the document loads (in JQuery).

Version:7.x-1.x-dev» 6.x-2.x-dev
StatusFileSize
new2.71 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in captcha-6.x-2.3-branch-[881156].patch.
[ View ]

6.x-2.x branch patch.

StatusFileSize
new2.65 KB
FAILED: [[SimpleTest]]: [MySQL] 579 pass(es), 3 fail(s), and 0 exception(es).
[ View ]

Sorry patch format.

Status:Needs review» Needs work

The last submitted patch, captcha-6.x-2.3-branch-[881156].patch, failed testing.

@gregarios, in Drupal it's prefered to use Drupal.behaviors instead $(document).ready(function(){

Here you can read more with some nice examples - http://www.nicklewis.org/drupal-hackers-cookbook/jquery-js/unlocking-dru...

Hi all,

some remarks:

  • I just committed http://drupal.org/cvs?commit=452052 so that the JavaScript attaching in the CAPTCHA and Image CAPTCHA module works as encouraged by Drupal coding standards (http://drupal.org/update/modules/5/6#javascript-behaviors). This should clear up some confusion in this thread.
  • As already noted, Drupal.behaviors.captchaAdmin, should be limited to JavaScript magic for the CAPTCHA admin pages. The JavaScript for adding the autocomplete attribute should be in a (new) "captcha" behavior, e.g.:
    Drupal.behaviors.captcha = function (context) {
      // Turn off autocompletion  for the CAPTCHA response field
      $("#edit-captcha-response").attr("autocomplete", "off");
    }
  • Also note that "captcha.js" is only added to the CAPTCHA administration page, see "captcha.admin.inc", around line 44:
    <?php
     
    // Use javascript for some added usability on admin form.
     
    drupal_add_js(drupal_get_path('module', 'captcha') .'/captcha.js');
    ?>

    If the JavaScript for adding the autocompletion should also work on non-administration pages (duh), it should also be added to pages with a CAPTCHA. For example, add a drupal_add_js call in the captcha_process function (I'm thinking out loud here, it should be tested for real).

If you move it to the captcha.inc you won't need to have the same code in administration.

This is because the captcha.admin.inc includes the captch.inc file.

Would this be done in captcha_set_form_id_setting? Where is the captcha_process you are talking about?

http://dev.w3.org/html5/spec/common-input-element-attributes.html#the-au...

Does this mean that HTML 5 makes autocomplete valid html??

If so should we still fix it or just leave it?

In image_captcha.user.inc on line 44 I think this should be inserted.

<?php
   
// Add javascript
   
drupal_add_js(drupal_get_path('module', 'image_captcha') . '/image_captcha.js');
?>

This is because its just before the image gets sent to the user?

I feel this is wrong because it should be standardised across all captcha implementations.

Should we place it in captcha.module instead in the captcha_captcha function which is an implementation of hook_captcha()

You could place the code just before the break, I am just guessing here, does this make send?

I am very lost in this code. Help would be appreciated.

subscribe

subscribe

Version:6.x-2.x-dev» 7.x-1.x-dev
Status:Needs work» Needs review
StatusFileSize
new2.5 KB
PASSED: [[SimpleTest]]: [MySQL] 700 pass(es).
[ View ]

Finally a Patch.

I think I worked it out. Not 100% Sure though :S

Fingers Crossed!

ok cool passed the test, can I get someone to independently verify that it works??

It might be a good idea to separate the jQuery for admin and user.

What does everyone else think??

Status:Needs review» Fixed

moved the drupal_add_js() from _captcha_insert_captcha_element() to captcha_element_process() from elechlan's patch in #33 and committed:
http://drupal.org/cvs?commit=495694 (6.x-2.x)
http://drupal.org/cvs?commit=495712 (7.x-1.x)

thanks everyone

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.