Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
This was recently fixed in Panels and CTools suffers from the same basic problem: #1708230: panels/ajax does not respond with the 'application/json' Content-Type header
Comment | File | Size | Author |
---|---|---|---|
#34 | ctools-error-with-patch.png | 236.62 KB | SocialNicheGuru |
#20 | views-list-page-do-not-test.patch | 477 bytes | effulgentsia |
#20 | interdiff.txt | 452 bytes | effulgentsia |
#20 | ctools-ajax-deliver-1710710-20.patch | 7.94 KB | effulgentsia |
#19 | ctools-ajax-deliver-1710710-18.patch | 7.78 KB | japerry |
Comments
Comment #1
asb CreditAttribution: asb commentedProbably I'm in the totally wrong issue queue here, but #1708230 was the closest match for my search keywords (so my apologies upfront).
Correlating with the release of ctools 6.x-1.9 (D6 branch!), I have become unable to edit certain Panels content; attempts to edit these parts of a Panel page result in a browser dialog to download a file of the type
application/json
. This file is filled with something which looks like JSON:[{"command":"settings","argument":{"basePath":"\/","fivestar":{"titleUser":"Your rating: ","titleAverage":"Average: ","feedbackSavingVote":"Saving your vote...","feedbackVoteSaved":"Your vote has been saved.","feedbackDeletingVote":"Deleting your vote...","feedbackVoteDeleted":"Your vote has been deleted."},"admin_menu":{"margin_top":1},"colorbox"…
However, my problem happens under D6, and the correlation with the ctools update might be circumstantial (apologies again). Please advise if this is a unrelated issue, or has nothing to do with ctools or panels.
Thank you!
Comment #2
ericduran CreditAttribution: ericduran commentedHa, so I kept arguing with a co-worker to stop using ajax_render and use delivery callbacks and kept referring to ctools as a good example but Ha, he actually used ctools but it was still wrong.
Anyways I'm making patch for this now, I'll try to fix the example and show how to account for both non-ajax and ajax form.
Comment #3
ericduran CreditAttribution: ericduran commentedOK, Here's a first pass at this. A couple of things.
Personally I do not like that Core doesn't provide you with a default mechanism to deliver both html and ajax page at the same path. What most people do is just call ajax_deliver() manually when they're returning ajax which personally I think it's bad practice. You can find examples like this in the Examples module.
What I did here was add a ctools_page_delivery_callback_alter() to alter the callback when a request comes in from ctools-use-modal. This way anyone using the ctools-use-modal class can delivery both html and ajax by doing what they'll expect to do which is If($ajax) {return array('type' => 'ajax', '#commands' => ....); } else { return render form etc ...}.
Actually using the delivery callback allows you to even simpler just return drupal_get_form with out much work since the delivery callback will account for none command base return.
I updated the ctools_ajax_sample module and remove all the print & exit to simply
This only works for now with the ctools-use-modal class. I know there's other places in ctools that do
I'll need to play a bit with other modules using that functionality in ctools to make sure I do it right and don't brake things.
FYI, I tested all the modal examples to be sure they still work and did a quick run through with Views (Just to be 100% sure I didn't brake something by mistake).
Also I know this isn't a very straight forward change :-/
Comment #4
ericduran CreditAttribution: ericduran commentedAlso, I know it sucks to use $_SERVER & $_POST but this is an extremely rare time where this information is actually needed. There is literally no-other data available to you in that hook. I also made sure to check for everything before trying to access the array. I was super careful ;-/
Kinda sucks, but pretty much everything related to delivery_callbacks has to do a horrible hack like that. Also I do not like that we have to use $_POST but sadly we can't add a header which would be nicer because that functionality didn't get into jquery till 1.5 :-/
Comment #5
sambonner CreditAttribution: sambonner commentedAdding an updated version of this patch that uses the new delivery callback on the access control and context modal's. I'm setting the header manually for the delete callbacks as these don't seem to play nice with the delivery callback approach, not quite sure why.
Comment #6
Elijah LynnInterdiff between #3 and #5.
Comment #7
Elijah LynnBoth patches (3 and 5) against latest dev are returning HTML, not JSON in ctools_ajax_sample module when attempting to delete some of the sample content (md5 hashes). I will attempt to resolve.
Comment #8
Elijah LynnOkay, the delivery_callback_alter() was only hitting if it was a modal but I am not sure why this was checking for a modal.
It works if you remove the check for the modal and I tested the sample and it works fine. @ericduran can you tell me the logic for checking if it is a modal, I am sure there was a reason for it but just not sure what it is, thanks!
I attached a patch for not checking the modal. Otherwise this is fairly RTBC'd I would say but I will wait to mark it as so.
Comment #9
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedthe patch in 8 no longer applies cleanly to the newest ctools
Comment #10
ericduran CreditAttribution: ericduran commented✓
--
Edit. Sorry wrong issue.
Comment #11
sambonner CreditAttribution: sambonner commentedRerolled patch from #8 to apply on top of latest ctools 1.4 release.
Comment #12
sambonner CreditAttribution: sambonner commentedOK so I think the change to ctools_page_delivery_callback_alter() to make the check much wider is negatively impacting on autocomplete form elements returning JSON + HTML using drupal_json_output(). Entityreference for example has entityreference_autocomplete_callback_get_matches() returning a JSON object with some HTML properties using drupal_json_output() for it's autocomplete entity reference fields. Currently its breaking and returning an AJAX error because drupal_json_output() passes it's output to drupal_deliver_html_page() which is obviously being intercepted and overridden into processing using ajax_deliver().
While it may technically be more correct to return an array for use with ajax_deliver I suspect there's too many modules using drupal_json_output at the moment for this to be practical/acceptable. I suggest we tighten the check to restrict the ajax_deliver callback swap to only fire within ctools, particularly since drupal_json_output does at least set the correct json response headers at the moment.
I can try rolling a patch but I'd like some input from others who are more expert than I in this area as to whether this is the correct path to take.
Comment #13
dvandusen CreditAttribution: dvandusen commentedThis issue is causing some confusion to those of us with weaker minds. It pretty strongly implies that "ajax_render should not be used; ajax_deliver should be used instead."
Still, there are posts that suggest that are significantly less firm.
I must presume that ajax_render will disappear in D8 from the seeming direction shown on this issue. Is that a fair assumption?
Please give some very firm direction. Also, please be clear regarding the actual state of / problems with each. This would help nubies.
Thx
D
Comment #14
Dave ReidRegardless of how it's fixed, it's causing issue when we need the proper content type header in the response.
Comment #15
maximpodorov CreditAttribution: maximpodorov commentedSo this should be fixed soon, right?
Comment #16
japerryHaving a lack of content type header in the response has been causing a slew of issues from many module maintainers, and that ambiguity needs to be fixed.
Lets do some testing with entityreference_prepopulate and a few others modules to see where this breaks.
Comment #17
japerryHere is a re-roll against latest HEAD.
Comment #19
japerryAhh gah, when I merged the re-roll in I lost a {
Here lets try this patch instead.
Comment #20
effulgentsia CreditAttribution: effulgentsia at Acquia commented#19 breaks the Enable/Disable row operations on /admin/structure/views. Here's a fix for that, but it only works if the attached Views patch is also applied.
Comment #21
pwolanin CreditAttribution: pwolanin as a volunteer and at Acquia commentedWhy does views break? It's now getting a render array?
Comment #22
japerryUnless there are some other regressions (I haven't found any others), considering this RTBC. Now views just needs to change some of their code ;)
Comment #23
joelpittet@japerry is there an issue for the code needing fixes in views?
Comment #24
effulgentsia CreditAttribution: effulgentsia at Acquia commentedI don't know if there's an issue, but #20 has a patch.
Comment #25
joelpittet@effulgentsia thanks, filed it over here: #2563431: ajax_render should not be used; ajax_deliver should be used instead.
Comment #26
joelpittetThis seems to break ckeditor, it may need a similar fix.
Comment #27
sam-elayyoub CreditAttribution: sam-elayyoub as a volunteer commentedThe main issue is with Ckeditor, I did some tests and i found out after debugging
An AJAX HTTP error occurred.
HTTP Result Code: 200
Debugging information follows.
Path: /system/ajax
StatusText: OK
ResponseText:
Warning: Unknown: Input variables exceeded 3000. To increase the limit change max_input_vars in php.ini. in Unknown on line 0
// the reason for this issue is overload of ajax within the same page, so after testing all the ways to solve this issue, i found that ckeditor by Wysiwyg Settings is the only module that loading over 30 seconds, so if you turn ckeditor or any text editor in the page off, the page works fine.
Comment #28
sam-elayyoub CreditAttribution: sam-elayyoub as a volunteer commentedAnother solution to turn off "Disable" the ckeditor auto enable from the ckeditor basic settings in content authoring configuration.
Comment #29
sam-elayyoub CreditAttribution: sam-elayyoub as a volunteer commentedComment #30
japerrySo it appears that ckeditor compatibility is an issue then, coming from comment #27? Changing to needs review until someone can chime in confirming whether or not this specific patch causes regressions in ckeditor.
Comment #31
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedthis patch caused my site to not be able to rebuild permissions.
autocomplete also did not work :(
Comment #32
rivimeyThis issue seems to have got stuck in a mire.
I wonder, would it be useful to split it into two parts: one side being the changing of ajax_render calls into returning a render array, and the other part being the intercept of deliver page? I would think the ajax_render change is a lot less risky than the deliver page change, so perhaps we can make some progress there at least.
@sam-elayyoub regarding the ckeditor tests in #27 does 'overload of ajax' refer to more than one ckeditor widget, or was only one instance of ckeditor on the page needed to cause problems?
Comment #33
sam-elayyoub CreditAttribution: sam-elayyoub as a volunteer commented@rivimey I'm sorry it took's me so long to reply, however it was for all widgets not just one, the meaning it was a general overload of ajax from the page in general not just a field, so to be more clear, the fields which using ckeditor are loading the max of what ajax can be afford for that page, so if you sometime disable one ckeditor on that page you can add another but without disabling one always it shows the ajax overloaded issue.
the simplest way to avoid this issue is to increase the ajax timeout or disable the ckeditor and make it enable by choice.
setTimeout(function() { /* Your Code */ }, 2000);
Comment #34
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedPer #31 I am setting to needs work. When I uninstalled the patch, I was able to build permissions and autocomplete worked again.
Comment #35
sam-elayyoub CreditAttribution: sam-elayyoub as a volunteer commentedSocial.. please try to do the permissions rebuild again using a different theme. One of the issues Ajax page and if you would like try first to turn off the overlay module and uninstall it then do the same process