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.When drupal is returning a JSON object to an ajax request, XHProf still appends it's link, which results in an invalid JSON object error.
I've created a patch to fix that. It uses hook_ajax_render_alter to detect when we are about to return a JSON object and then, if it is, skips appending the link.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 1485190-xhprof-fix-json-responses-4.patch | 594 bytes | erikwebb |
| #1 | 1485190-xhprof-fix-json-responses.patch | 828 bytes | andrewbelcher |
Comments
Comment #1
andrewbelcher CreditAttribution: andrewbelcher commentedHere is the patch...
Comment #2
JohnAlbinI tried testing that XHProf does break AJAX by testing a node edit page's "authored by" autocomplete. The "XHProf output" link is at the bottom of the node edit form, but the autocomplete works just fine. So, I'm obviously not testing this properly.
What's a sure-fire way to test that XHProf is breaking an AJAX request? (If possible, with just D7 core modules).
Comment #3
msonnabaum CreditAttribution: msonnabaum commentedI have noticed this using devel query log and clicking on the S A E links. I think this patch works well although I'm open to a possibly cleaner approach. If no one else has any ideas I'll probably commit this in a few days.
Comment #4
erikwebb CreditAttribution: erikwebb commentedI think the real issue is that the response is JSON, not generally AJAX. Shouldn't we just be checking against the response's Content-Type? An HTML AJAX request would still work with the XHProf link in it.
Comment #5
andrewbelcher CreditAttribution: andrewbelcher commented@erikwebb - Yes, sorry. I was using confusing language. You're right that the issue is when we're returning JSON rather than AJAX. I've updated the title to reflect.
As for the solution, not sure whether checking the response Content-Type or using hook_ajax_render_alter() is more reliable/consistent...
Comment #6
erikwebb CreditAttribution: erikwebb commentedI would like to use Content-Type so that other JSON responses, such as Services callbacks can have the link removed as well. If we avoid checking Content-Type, we are stuck in the situation of trying to allow and predict non-standard behavior. If the response format is something other than text/html, the Drupal module should be setting that appropriately.
Comment #7
msonnabaum CreditAttribution: msonnabaum commentedWait, I actually fixed this a while back in 2407398fda6c5c7ba82ef377cbe1c9608a9d560f, which is similar to Erik's approach.
I noticed on the devel query example it's not returning json at all so it doesn't work there, but I'm curious what other situations people are finding where it does break.
Comment #8
RobLoachJSON is also extremely broken in Drupal 8: #1619446: All autocomplete and drupal_json_output() responses are broken (returning Ajax instead of JSON)
Comment #9
erikwebb CreditAttribution: erikwebb commentedCan this possibly be better fixed by #1775368: X-XHProf HTTP Response Header?
Comment #10
Dave ReidRE #9: No, because I would expect the 'display results on page' to not break AJAX requests which is likely what it would still do without this patch.
Comment #11
erikwebb CreditAttribution: erikwebb commented@Dave Reid - I think if we can get #1775368: X-XHProf HTTP Response Header in first, then this issue becomes simply preventing the link from being printed for non-HTML responses.
Comment #12
Dave ReidNote that #1710710: ajax_render should not be used; ajax_deliver should be used instead. is a primary issue with AJAX requests - since CTools doesn't use the correct content type response header for AJAX.