Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RFC] Hide captcha field for users and add honeypot field #832

Merged
merged 4 commits into from May 19, 2017
Merged

[RFC] Hide captcha field for users and add honeypot field #832

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1383140
Hide captcha field for users
ausi May 18, 2017
9469389
Shorten the inline CSS definition.
leofeyer May 19, 2017
4be0458
Add a change log entry.
leofeyer May 19, 2017
dee4aff
Fix a typo.
leofeyer May 19, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 17 additions & 1 deletion src/Resources/contao/forms/FormCaptcha.php
View file
Open in desktop
Expand Up @@ -136,7 +136,7 @@ public function validate()

$arrCaptcha = $objSession->get('captcha_' . $this->strId);

if (!is_array($arrCaptcha) || !strlen($arrCaptcha['key']) || !strlen($arrCaptcha['sum']) || \Input::post($arrCaptcha['key']) != $arrCaptcha['sum'] || $arrCaptcha['time'] > (time() - 3))
if (!is_array($arrCaptcha) || !strlen($arrCaptcha['key']) || !strlen($arrCaptcha['sum']) || \Input::post($arrCaptcha['key']) != $arrCaptcha['sum'] || $arrCaptcha['time'] > (time() - 3) || \Input::post($arrCaptcha['key'].'_name'))
{
$this->class = 'error';
$this->addError($GLOBALS['TL_LANG']['ERR']['captcha']);
Expand Down Expand Up @@ -181,6 +181,22 @@ protected function getQuestion()
}


/**
* Get the correct sum for the current session
*
* @return int The sum
*/
protected function getSum()
{
/** @var SessionInterface $objSession */
$objSession = \System::getContainer()->get('session');

$arrCaptcha = $objSession->get('captcha_' . $this->strId);

return $arrCaptcha['sum'];
}


/**
* Generate the label and return it as string
*
Expand Down
12 changes: 12 additions & 0 deletions src/Resources/contao/templates/forms/form_captcha.html5
View file
Open in desktop
Expand Up @@ -15,4 +15,16 @@

<input type="text" name="<?= $this->name ?>" id="ctrl_<?= $this->id ?>" class="captcha mandatory<?php if ($this->class) echo ' ' . $this->class; ?>" value=""<?= $this->getAttributes() ?>>
<span class="captcha_text<?php if ($this->class) echo ' ' . $this->class; ?>"><?= $this->getQuestion() ?></span>

<?php if (!$this->hasErrors()): ?>
<div style="display:none">
<label for="ctrl_<?= $this->id ?>_hp">Do not fill out this field</label>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In English should be fill in this field and I wonder if we should translate it anyway for screenreaders? Also I wonder if there's some WAI-ARIA directive to improve usability for handicapped people.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the PR wasn’t completely finished, thats why I added [RFC], not [RTM] :)
Because there is a display:none div directly around it, no user and no screenreader user will see that in 99.9999% of cases.
But yes, we should change it to Do not fill in this field and translate it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With display:none, the message will never be read out by screen readers. We do not need to translate it therefore.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 820ea42.

<input type="text" name="<?= $this->name ?>_name" id="ctrl_<?= $this->id ?>_hp" value="">
</div>
<script>
document.getElementById('ctrl_<?= $this->id ?>').parentNode.style.display = 'none';
document.getElementById('ctrl_<?= $this->id ?>').value = '<?= $this->getSum() ?>';
</script>
<?php endif ?>

<?php $this->endblock(); ?>