As far as I can tell, the webform integration doesn't work. Specifically, it looks for the string 'webform_client_form' in form_ids, which isn't the case for the D8 version of webform. I feel like there should probably be a nicer way of matching webform form ID's, but for now updating the string matching to 'webform_submission' should do the trick.

Patch to follow

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

stevetweeddale created an issue. See original summary.

stevetweeddale’s picture

geerlingguy’s picture

Status: Active » Needs review

Yeah; I never have changed things to support the new Webform 5.x (descended from YAMLform). I would like to make sure to also add some sort of test for this.

geerlingguy’s picture

Status: Needs review » Needs work

It seems that the actual form ID is in the pattern webform_submission_[form-machine-name]. For example, I created a webform titled 'Test form', and when it's rendered, the form_id is webform_submission_test_form

Have you tested this patch manually? At a glance, it seems like more work needs to be done to make sure Honeypot applies to webforms correctly.

rodrigoaguilera’s picture

Then maybe the strpos should be against "webform_submission_" instead of "webform_submission"

stevetweeddale’s picture

As far as I remember it works! Looking at it, I'd say the str_pos is just checking for the presence of that substring. So I guess #5 is right in that the trailing _ would still match valid webform ID's, but reduce the chance of false positives (say someone made a form with the id "something_webform_submission"). Though IIRC the old webform ID's worked the same way (eg webform_client_form_15).

Truth be told, ideally there'd be a nicer way entirely of determining if a form was a webform, besides string matching the form ID.

mr.baileys’s picture

Status: Needs work » Needs review
FileSize
93.48 KB
2.05 KB

Truth be told, ideally there'd be a nicer way entirely of determining if a form was a webform, besides string matching the form ID.

I think using the webform_submission_ prefix is the correct approach to determine whether or not a form is a webform. Webform itself seems to use this check in at least one place: http://cgit.drupalcode.org/webform/tree/modules/webform_node/webform_nod... . An alternative approach might be to check for the '#webform_id' property

However, Webform itself also comes with Honeypot-integration. If both Webforms and Honeypot are enabled, you can enable Honeypot support on all webforms via Webform Settings > Third Party Settings > Honeypot Integration. It does not make sense to implement integration between the two modules in both places, so I suggest removing the support for Webforms from the Honeypot module, and let Webforms handle it.

Patch attached that removes support for Webforms from Honeypot. I am not sure we need an upgrade path, since the settings has never really worked in the first place. We might want to warn a user on update that the setting is removed, directing them to the Webform Honeypot settings instead?

geerlingguy’s picture

Status: Needs review » Reviewed & tested by the community

Sounds good to me. I think a warning on update is overkill, but I might add a note on Honeypot's project page at least.

geerlingguy’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.