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.
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.
Comment | File | Size | Author |
---|---|---|---|
#33 | captcha.patch | 2.5 KB | elachlan |
#24 | captcha-6.x-2.3-branch-[881156].patch | 2.65 KB | elachlan |
#23 | captcha-6.x-2.3-branch-[881156].patch | 2.71 KB | elachlan |
#19 | captcha-[881156].patch | 2.59 KB | elachlan |
#17 | captcha-[881156].patch | 691 bytes | elachlan |
Comments
Comment #1
soxofaan CreditAttribution: soxofaan commentedGood idea.
Should also be applied to simple math CAPTCHA.
Flagging it as "low-hanging fruit", so aspirant co-maintainers can tackle this one
Comment #2
vivekkhurana CreditAttribution: vivekkhurana commentedPatch attached which adds autocomplete off to image_captcha and math captcha.
Comment #3
soxofaan CreditAttribution: soxofaan commentedHi 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.
Comment #5
soxofaan CreditAttribution: soxofaan commentedOk, 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
Comment #6
pavel.karoukin CreditAttribution: pavel.karoukin commentedPorted to D7
Comment #7
soxofaan CreditAttribution: soxofaan commentedCommitted: http://drupal.org/cvs?commit=448672
thanks!
Comment #8
elachlan CreditAttribution: elachlan commentedThis 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.
Comment #9
soxofaan CreditAttribution: soxofaan commented(changing title accordingly)
Comment #10
pavel.karoukin CreditAttribution: pavel.karoukin commentedSo #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:
Comment #11
elachlan CreditAttribution: elachlan commentedAdd this to the captcha.js file
How can we run a test for it?
Comment #12
elachlan CreditAttribution: elachlan commentedPatch with changes. I did it manually as I don't have CVS.
Comment #13
elachlan CreditAttribution: elachlan commentedPatch needs tests.
Also the original changes will need to be reverted.
Comment #15
elachlan CreditAttribution: elachlan commentedResubmitting patch. Hopefully fixed.
Comment #17
elachlan CreditAttribution: elachlan commentedLearning the hard way not to do it manually. If this fails I'm going to leave it to someone else.
Comment #19
elachlan CreditAttribution: elachlan commentedOK, now it should work.
Comment #20
pavel.karoukin CreditAttribution: pavel.karoukin commentedelachlan,
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?
Comment #21
elachlan CreditAttribution: elachlan commentedit should be similar to image_captcha.js
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.
Comment #22
gregarios CreditAttribution: gregarios commentedXHTML 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).Comment #23
elachlan CreditAttribution: elachlan commented6.x-2.x branch patch.
Comment #24
elachlan CreditAttribution: elachlan commentedSorry patch format.
Comment #26
pavel.karoukin CreditAttribution: pavel.karoukin commented@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...
Comment #27
soxofaan CreditAttribution: soxofaan commentedHi all,
some remarks:
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.: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 thecaptcha_process
function (I'm thinking out loud here, it should be tested for real).Comment #28
elachlan CreditAttribution: elachlan commentedIf 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?
Comment #29
elachlan CreditAttribution: elachlan commentedhttp://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?
Comment #30
elachlan CreditAttribution: elachlan commentedIn image_captcha.user.inc on line 44 I think this should be inserted.
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.
Comment #31
powery CreditAttribution: powery commentedsubscribe
Comment #32
apprayo CreditAttribution: apprayo commentedsubscribe
Comment #33
elachlan CreditAttribution: elachlan commentedFinally a Patch.
I think I worked it out. Not 100% Sure though :S
Fingers Crossed!
Comment #34
elachlan CreditAttribution: elachlan commentedok 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??
Comment #35
soxofaan CreditAttribution: soxofaan commentedmoved the
drupal_add_js()
from_captcha_insert_captcha_element()
tocaptcha_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