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
The toolbar submenus do not load anymore.
Proposed resolution
If you return a JsonResponse in your controller it should be passed directly to the output rather than being converted to an AjaxResponse.
User interface changes
None. Well, toolbar subtrees will work again :)
API changes
Returning a JsonResponse object from the controller will no longer result in it being converted to an AjaxResponse.
Comment | File | Size | Author |
---|---|---|---|
#13 | 2187483-13.patch | 6.84 KB | star-szr |
#13 | 2187483-13-testonly.patch | 4.1 KB | star-szr |
#13 | interdiff.txt | 1.12 KB | star-szr |
#12 | jsonresponse-2187483_12.patch | 2.75 KB | agentrickard |
#11 | jsonresponse-2187483_11.patch | 2.75 KB | agentrickard |
Comments
Comment #1
jessebeach CreditAttribution: jessebeach commentedAdds some jank eval support for the JSON callback sent from the Toolbar subtrees route controller. Let's get this into ajax.js so Toolbar just needs to call the URL and not worry about the JSON data eval'ing.
Oh, and the namespace of the ToolbarController was wrong in the routing file for some reason, so I fixed it:
Comment #2
webchickRegressions in previously working things == critical.
Comment #3
dawehnerOh i haven't opened this issue in the meantime but also ran into the problem as you talked about the toolbar in the other issue: #2187991: toolbar access checking points to wrong namespace
Comment #4
jessebeach CreditAttribution: jessebeach commentedThis is what JsonResponse is now returning
Comment #5
dawehnerSo yeah, if you return a JsonResponse in your controller it should be passed directly to the output. This is cleary a bug.
Comment #6
jessebeach CreditAttribution: jessebeach commentedI had a chat with nod_ about this and he agrees. A JsonResponse should return JSON. if you want commands and the whole Drupal AJAX experience, then one should use AjaxResponse.
Comment #7
dawehnerRight, in those cases you have drupal_ajax as content type, so both the JS and the PHP developer has control over it.
Comment #8
dawehnerRerolled.
Comment #9
dawehner.
Comment #10
agentrickardWorks in testing. Needs code review.
Comment #11
agentrickardVery minor documentation re-roll (a / an and a missing comma). Looks good!
Comment #12
agentrickardAnother minor docs change to use "Response object" consistently.
Comment #13
star-szrAdding back the test case that got lost and more minor docs cleanup after talking to @dawehner. This looks good to go!
Including test-only patch as well for visibility and updating the issue summary, hope I got the details right.
Comment #16
dawehnerThank you for fixing the grammar mistakes.
Comment #18
webchickCommitted and pushed to 8.x. Thanks!