Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
- Two AJAX-enabled forms on a page.
- One is a views exposed form, one isn't
- Views exposed forms get input from GET, normal forms get theirs from POST
- The views form is rendered first
- when checking $ajax_form_request - FormBuilder only checks if it's an AJAX_FORM_REQUEST at all, not if it's for the $form_id being built
- this leads to a BrokenRequestException because there is no form ID in $_GET - but there's not supposed to be, because it's in POST
- and/or you get Uncaught PHP Exception Symfony\\Component\\HttpKernel\\Exception\\HttpException: "The specified #ajax callback is empty or not callable." at /Users/catch/Sites/8.x-dev/core/lib/Drupal/Core/Form/FormAjaxResponseBuilder.php
Proposed resolution
Check the form ID matches before setting $ajax_form_request = TRUE
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | 2699489-ajax-17-PASS.patch | 2.6 KB | tim.plunkett |
| #17 | 2699489-ajax-17-FAIL.patch | 1.22 KB | tim.plunkett |
| #8 | 2699489-tests.patch | 1.67 KB | catch |
| #6 | ajax-2699489.patch | 1.38 KB | catch |
| #5 | ajax-2699489.patch | 1.43 KB | catch |
Comments
Comment #2
catchComment #3
catchComment #5
catchLast patch was a bit of a tautology.
This may be better.
Comment #6
catchMinus the debug.
Comment #7
catchComment #8
catchHere's a start on a unit test, but it doesn't fail yet, got distracted by rc stuff in the middle.
Comment #9
rosinegrean CreditAttribution: rosinegrean at PitechPlus commentedI had the same issue.
I can confirme the patch solved it.
Thanks,
Comment #10
Wim Leers#2710939: AddToCartFormatter lazy builder causes erroneous Ajax submits, when a second form is present had a very similar problem. And it was fixed by #2702609: Disable placeholdering (and BigPipe) on unsafe requests to help find forms as fast as possible (to allow EnforcedResponses to work). I wonder if this issue is also fixed by #2702609: Disable placeholdering (and BigPipe) on unsafe requests to help find forms as fast as possible (to allow EnforcedResponses to work)?
Comment #11
Wim LeersActually, I think those issues are only tangentially related. It's specifically about (AJAXy) forms rendered in a lazy builder. That's not the case here (at least not in the description in the IS). So, probably independent issues but in the same space.
Comment #12
catch@Wim I doubt it fixes exactly this issue, this is specifically triggerable when you have a GET and POST form on the same page, both AJAX. Definitely similar symptoms though.
Comment #13
Anonymous (not verified) CreditAttribution: Anonymous commentedSame issue here, the patch works well !
Thanks a lot !
Comment #14
gngn CreditAttribution: gngn at Computer Manufaktur GmbH commentedSame here, patch #6 helped.
Thanks a lot, too!
Comment #15
asauterChicago CreditAttribution: asauterChicago commentedCan also confirm, patch in #6 works, and this issue is still prevalent in Drupal 8.2x.
In my case it was on sort of a "add to cart" button on a search results view. Could we get this patch rolled into the next release(s)? Core patches like this always make me nervous, if I need to run an update on the site, I have to remember to apply this patch.
Comment #16
EclipseGc CreditAttribution: EclipseGc at Acquia commentedOk, this patch +1000000.
We have a bug in the Panels IPE/CTools Views integration that is only something we run into with certain in-development patches applied. After long hours of debugging that I came up with a HORRIBLE fix, and then discussed it with Tim Plunkett who pointed me here. This patch fixes the problem, so huge ++. It's only visible in certain advanced use case block placement scenarios via ajax. It's nice to see that it is indeed a core bug.
Eclipse
Comment #17
tim.plunkettThat test was really close, just cleaned it up a little. Also the fix looks spot on to me.
Comment #20
isaacrc CreditAttribution: isaacrc commented#17 Works for me, Thanks a lot!
Comment #21
Hydra CreditAttribution: Hydra at erdfisch commented#17 exactly what I needed. The patch looks good to me, but I am not experienced enough with this to tell if it's right.
Since the test is green and we have several people confirming this is working - I'll put this on RTBC to get some attention on it. Thx for working on this!
Comment #22
catchI missed #17 at the time, thanks for updating the test, had completely forgotten about this issue...
Comment #23
alexpottCommitted and pushed 8c22ddf to 8.3.x and f4a5441 to 8.2.x. Thanks!