As it currently is, honeypot css file is generated:
- During install, defaulting to honeypot_element_name = 'url'
- When saving the honeypot setting in the admin form.
Now imagine a site where honeypot_element_name is deployed using features/strongarm, or there as an issue generating the original CSS file.
In both cases you are quite stuck with honeypot not working because the css file is missing/inappropriate, and this can be quite frustrating to diagnose specially if aggregation is turned on.
The module should probably verify (and regenerate if missing) the css file in honeypot_add_form_protection().
Comment | File | Size | Author |
---|---|---|---|
#9 | honeypot_does_not-2503309-9.patch | 3.43 KB | geerlingguy |
#7 | honeypot-ensure-css-exists-on-cron-2503309-7.patch | 1.37 KB | chrisfree |
#1 | 2503309-ensure-css-is-ok.patch | 1.35 KB | david_garcia |
Comments
Comment #1
david_garcia CreditAttribution: david_garcia commentedNot sure if this is the best approach, but does the job.
Comment #3
david_garcia CreditAttribution: david_garcia commentedComment #5
geerlingguy CreditAttribution: geerlingguy commentedI would rather regenerate the CSS file if it doesn't exist as part of the cron job, and not at runtime when the form is being built, as this will not impact per-request performance any further than it already is.
If we can move the logic from
honeypot_add_form_protection()
in this patch intohoneypot_cron()
, that would make this simpler and more performant, and worst case, a site would have a missing file for a few minutes (or a few hours at max) until cron runs.Comment #6
chrisfree CreditAttribution: chrisfree at Chromatic commentedI just ran into this issue and will see if I can get a patch created tonight/tomorrow.
Comment #7
chrisfree CreditAttribution: chrisfree at Chromatic commented@geerlingguy - I think this patch accomplishes what you are looking for.
Comment #9
geerlingguy CreditAttribution: geerlingguy commentedI committed the above patch, and am also going to commit the attached patch as a follow-up, which adds some tests around CSS file regeneration (that way we can ensure that cron-based regeneration will continue working, hopefully forever!
After I commit these extra changes, we'll need to forward-port the changes to Drupal 8 ASAP (unless the mechanism for building the file is radically different... it's been a few months since last time I worked on the D8 code).
Comment #10
geerlingguy CreditAttribution: geerlingguy commentedI've committed the above patch for D7. On to D8 next!
Comment #12
geerlingguy CreditAttribution: geerlingguy commentedNevermind... in Drupal 8, we add the CSS to the page's
<head>
dynamically, in a way that should work properly with things like modals. So no need to forward-port! And no need to backport to Drupal 6 since the CSS file generation was never backported.