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.

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

Registration Honeypot #3339

Merged
merged 3 commits into from
Mar 21, 2018
Merged

Registration Honeypot #3339

merged 3 commits into from
Mar 21, 2018

Conversation

di
Copy link
Member

@di di commented Mar 21, 2018

Fixes #3174.

Copy link
Member

@ewdurbin ewdurbin left a comment

Choose a reason for hiding this comment

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

neat!

@@ -58,6 +56,14 @@ <h1 class="page-title">Create an account on PyPI</h1>
{% endif %}
</div>

{# The following is a honeypot field and is hidden from the user #}
<div class="form-group confirm-email" aria-hidden="true">
Copy link
Member

Choose a reason for hiding this comment

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

not sure if it'd conflict but is aria-placeholder valid in addition? I'm guessing there's some ability in screen readers to unhide elements, and in case someone does that perhaps we should let them know what's up?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

My understanding is that screen readers just ignore anything aria-hidden. I didn't want to put too much in the source here indicating what this is, adding a placeholder might do that.

Copy link
Member

Choose a reason for hiding this comment

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

@tvance @Kataract any tips?

Choose a reason for hiding this comment

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

aria-hidden isn't going to do anything for content that isn't generally interactable (like <div>s). You can do this two ways:

  • display: none; or visibility: hidden the <div>, and have the <label> text indicate that this field is a honeypot and not to fill it out
  • apply aria-hidden="true" to the input and visually hide the <label>

see: https://www.w3.org/WAI/GL/wiki/Captcha_Alternatives_and_thoughts

there's a specific post I have in mind that I used for an internal discussion on reCAPTCHA that I can't find quickly in google - if you want I can link it here later, if that isn't enough info.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok great! This is already hiding the div with display: none so it seems like I should move the aria-hidden directly to the input. Thanks very much!

@dstufft
Copy link
Member

dstufft commented Mar 21, 2018

Rather than one-off add some HTML, can we make this a proper, reusable form field?

@di
Copy link
Member Author

di commented Mar 21, 2018

@dstufft We could, kinda wanted it to blend into the source for this form a little bit so it wouldn't have an obvious name like "honeypot", do you think that's not a big deal?

@dstufft
Copy link
Member

dstufft commented Mar 21, 2018

Could always give it a non-obvious name, but still be reusable yea?

@di di merged commit c0da842 into pypi:master Mar 21, 2018
@di di deleted the honeypot branch March 21, 2018 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants