Problem/Motivation

A logical bug allows the form submit handler to be bound multiple times when there are multiple forms on the page.

Proposed resolution

Limit to a single submit handler bound, per form.

CommentFileSizeAuthor
#8 hide_submit-2559585-8-submit-handler-bound-multiple-times.patch783 bytesgeek-merlin
#4 submit_handler_is_bound-2559585-4[7.x-2.2].patch416 bytesalexandrdnlv
#4 submit_handler_is_bound-2559585-4[7.x.-2.x].patch426 bytesalexandrdnlv
#2 submit_handler_is_bound-2559585-2.patch608 bytesjtwalters
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jtwalters created an issue. See original summary.

jtwalters’s picture

jtwalters CreditAttribution: jtwalters commented
FileSize
submit_handler_is_bound-2559585-2.patch608 bytes

This patch may also fix the following issues:

- #2393923: Buttons get mixed up
- #2086887: Limit the action to the clicked form
- #2304087: The timer fails when one page contain multiple forms

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles at CARD.com commented
Status: Active » Needs review

This seems likely to require a re-roll after #2317141: Uncaught TypeError when resubmitting a form that had a missing required field was committed. Can you do that?

alexandrdnlv’s picture

alexandrdnlv CreditAttribution: alexandrdnlv as a volunteer commented
FileSize
submit_handler_is_bound-2559585-4[7.x.-2.x].patch426 bytes
submit_handler_is_bound-2559585-4[7.x-2.2].patch416 bytes

Hi I've updated a little bit a patch to work with 7.x-2.2. and 7.x-2.x versions (attached both of them).

geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin commented
Status: Needs review » Needs work

I can't see how the code should fix the problem by only dropping context. Instead this should be wrapped in a JS once().

Look into this:
https://www.lullabot.com/articles/understanding-javascript-behaviors-in-...

jtwalters’s picture

jtwalters CreditAttribution: jtwalters commented

It's already using the once() method, the problem is that the selector can grab un-related forms on the document context.

jtwalters’s picture

jtwalters CreditAttribution: jtwalters commented
Status: Needs work » Needs review
geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin commented
FileSize
hide_submit-2559585-8-submit-handler-bound-multiple-times.patch783 bytes

> It's already using the once() method

Well, partly right, that code is part of a large once() block, see:
http://cgit.drupalcode.org/hide_submit/tree/js/hide_submit.js?id=07b7078...

So then applying the selector again is wrong, but removing context also.
Instead we should simply use $form.

Patch flying in, please test.