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

Mrtango improve protection #6

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

MrTango
Copy link

@MrTango MrTango commented May 24, 2022

No description provided.

@MrTango MrTango changed the title Mrtango imrpove protection Mrtango improve protection May 24, 2022
Copy link
Member

@mauritsvanrees mauritsvanrees left a comment

Choose a reason for hiding this comment

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

So this is meant to counter spammers that have grown smart enough to not fill in inputs with style="display: none", right? Seems good.

Problem though: when I try it, and manually set the two css properties in the web developer tools, the input is invisible, but the surrounding div still takes up space. Can you come up with some css or another way to hide the div as well? height: 0 does the trick when I put it on the element, but then you have to somehow specify on which element.

What would be good, is if this CSS would be in the package itself. Perhaps even simply as a viewlet, so you don't have to activate it from within the Modules control panel. But then: that is what the resource registry is for.
The css file could be a small browser view that displays the CSS dynamically with the HONEYPOT_FIELD already filled in. Calculating this once a startup should be enough.

Could you review my PR #7 where I replace Travis with GitHub Actions? Then rebase your branch on it. I think you will see a test failure because we test for the display: none literally being in the output. Other than that, it should be fine.

@mauritsvanrees
Copy link
Member

I have merged my own PR for adding GitHub Actions, so you can rebase on master now.

Copy link
Member

@mauritsvanrees mauritsvanrees left a comment

Choose a reason for hiding this comment

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

Some remarks:

  1. You added a section 'Hiding the field with CSS' to the readme, but this is no longer needed because it is already in honeypot.pt, right?
  2. This leads to multiple inputs having the same id.
  3. This should be done in our transform in auto.py as well.

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.

None yet

2 participants