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

CommentFileSizeAuthor
#17 2699489-ajax-17-PASS.patch2.6 KBtim.plunkett
#17 2699489-ajax-17-FAIL.patch1.22 KBtim.plunkett
#8 2699489-tests.patch1.67 KBcatch
#6 ajax-2699489.patch1.38 KBcatch
#5 ajax-2699489.patch1.43 KBcatch
#2 ajax-2699489.patch744 bytescatch

Comments

catch created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new ajax-2699489.patch744 bytes
catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 2: ajax-2699489.patch, failed testing.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new ajax-2699489.patch1.43 KB

Last patch was a bit of a tautology.

This may be better.

catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new ajax-2699489.patch1.38 KB

Minus the debug.

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new 2699489-tests.patch1.67 KB

Here's a start on a unit test, but it doesn't fail yet, got distracted by rc stuff in the middle.

rosinegrean’s picture

rosinegrean commented

I had the same issue.
I can confirme the patch solved it.
Thanks,

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#2710939: AddToCartFormatter lazy builder causes erroneous Ajax submits, when a second form is present, +#2702609: Disable placeholdering (and BigPipe) on unsafe requests to help find forms as fast as possible (to allow EnforcedResponses to work)

#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)?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Actually, 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.

catch’s picture

catch
he/him
Primary language English
commented

@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.

Anonymous’s picture

Anonymous (not verified) commented

Same issue here, the patch works well !

Thanks a lot !

gngn’s picture

gngn commented

Same here, patch #6 helped.
Thanks a lot, too!

asauterChicago’s picture

asauterChicago commented

Can 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.

eclipsegc’s picture

eclipsegc commented

Ok, 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

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
StatusFileSize
new 2699489-ajax-17-FAIL.patch1.22 KB
new 2699489-ajax-17-PASS.patch2.6 KB

That test was really close, just cleaned it up a little. Also the fix looks spot on to me.

The last submitted patch, 17: 2699489-ajax-17-FAIL.patch, failed testing.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

isaacrc’s picture

isaacrc commented

#17 Works for me, Thanks a lot!

hydra’s picture

hydra commented
Status: Needs review » Reviewed & tested by the community

#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!

catch’s picture

catch
he/him
Primary language English
commented

I missed #17 at the time, thanks for updating the test, had completely forgotten about this issue...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 8c22ddf to 8.3.x and f4a5441 to 8.2.x. Thanks!

  • alexpott committed 8c22ddf on 8.3.x 
    Issue #2699489 by catch, tim.plunkett: FormBuilder $ajax_form_request...

  • alexpott committed f4a5441 on 8.2.x 
    Issue #2699489 by catch, tim.plunkett: FormBuilder $ajax_form_request...

Status: Fixed » Closed (fixed)

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