The solution for this issue #2561619: Drupal Ajax objects and settings grows endlessly lead previously to errors in contrib modules like the one we fixed here #2673824: Views JS passing wrong type of object to Drupal.ajax.
But due to other possible errors in contrib that issue was rolled back in 8.0.x, but I found a related error that breaks in 8.1.x due to that change, you can see it here: https://github.com/dpi/courier/issues/21 maybe in other modules.
| Comment | File | Size | Author |
|---|---|---|---|
| #34 | 2705327-nr-bot.txt | 149 bytes | needs-review-queue-bot |
| #28 | umami-console-error.jpg | 215.34 KB | bmunslow |
| #7 | friendly-ajaxExpired-typeError.patch | 698 bytes | droplet |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !384
Comments
Comment #2
edurenye commentedThis seems to fix the issue, and I tested a bit and does not seem to break anything else.
Comment #3
edurenye commentedComment #4
droplet commentedWe may consider checking it explicitly to add support of jQuery object also. It's better backward supports for wrong usages.
Comment #5
droplet commentedI raised it to Major as it's close to D8.1 release day. We may hit many similar wrong usages.
Comment #6
nod_I don't want to add support for a jQuery object here, the Drupal API should not expect a jQuery object to be passed around. We could add code detecting a jQuery object, to be able to throw an error saying that it's not allowed. Like what we did for Drupal.ajax(). It's already like this for attach functions.
Cross post from #2706577: Non-HTMLElement values for ajax.element cause AJAX errors:
Our docs are pretty clear: ajax.element should be a HTMLElement, which inherits from Element which inherits from Node.
Comment #7
droplet commented1. throw an error for DX.
2. Skipped in return also.
Needs manual testing before backport to D8.1.x. (A bit confusing at the time, some scripts may missing in D8.1.x)
Comment #8
dpiThis no longer affects Courier because I am removing the JQ usage. I believe I used jQuery there because I got the impression I had to from the Drupal.ajax documentation:
I dont know if this is actually misleading... I know enough JS to be dangerous.
Re the patch from #7:
if (instance && !(instance.element instanceof HTMLElement)) {Sometimes second condition is failing because
instance.elementisfalse.. needs to check for isset?Comment #9
worldlinemine commentedI tested the change in Drupal 8.1.0 manually (not running the patch) and it resolved issue with ajax.js where trying to change the selection of a dropdown for choosing a widget in Content Type spun endlessly.
How does one go about insuring that an updated patch is in place for 8.1.1?
Comment #11
sumanthkumarc commentedI'm having a similar error when using modal api of core. I'm using nodejs integration and error comes on ajax call.
Similar issue raised in node js issue queue. Link: https://www.drupal.org/node/2828066
Also, the above patch throws the following error with node js contrib module enabled.
"Uncaught Error: TypeError: instance.element is not a HTMLElement"
Update: The issue got resolved if i disable the Node Js Ajax Framework integration module.
Comment #12
cilefen commentedComment #17
socialnicheguru commentedI tried this on Drupal 8.5.8
I got a number of js errors in the console after I enabled agregation.
No errors without aggregation
Comment #19
handkerchiefAny news on this? It would be great if this fix could be integrated into the core.
Comment #23
Hamulus commentedthis patch was helpful for me in D9 in 2021
so why don't include it in release?
Comment #24
bnjmnmI notice #19 and #23 both ask why this hasn't been added to Drupal. This is because it's waiting for someone in the community to review it then change the issue status to "Reviewed and Tested by the Community". Anyone that has had this patch work for their site has effectively already performed a manual review. If that is accompanied by a code review - and both the manual and code reviews are documented here - you can switch to "Reviewed and Tested by the Community" and it will move to the next stage of getting into core.
In this case, a bit of work is needed due to the age of the patch. Setting to "needs work" as the patch only alters a .js file, presumably because it was created before Drupal began using .es6.js files that transpile to .js. The change would need to be made in
ajax.es6.js, which would then be transpiled toajax.jsComment #27
abhisekmazumdarHey, I thought this would be the right time to create an MR for this PR. As this required a re-roll.
Kindly review my MR.
Comment #28
bmunslow commentedHi,
I reviewed and tested MR by @abhisekmazumdar in #27.
It applies cleanly on D.9.x and it fixes the issue indeed.
My only concern is that this patch produces 25 errors in JS console in pages where there were no errors before the patch.
These errors don't seem to affect any other functionality so far, but they certainly impact negatively the developer's experience.
The errors appear regardless of whether JS aggregation is enabled or not.
Comment #29
bmunslow commentedSo I did further testings and noticed the JS errors I reported about in #28, appeared only if the Big Pipe module was enabled.
I added an additional check:
instance.element !== falsebefore throwing Drupal error, this fixes all issues for me:Could someone please review this latest change and report back so we can set this to RTBC?
Comment #34
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.