Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Any ajaxified element on a page leads to a bloat of default Drupal.settings being output.
For example, just defining:
'#ajax' => array(
'method' => 'html',
'wrapper' => 'block-system-main',
),
Results in:
{"ajax-link":{
"method":"html",
"wrapper":"block-system-main",
"event":"click",
"selector":"#ajax-link",
"effect":"none",
"speed":"none",
"progress":{
"type":"throbber"
},
"url":"\/node\/1"
}
But most of those properties are defaults that should be set via JS. The actual Drupal.settings should be:
{"ajax-link":{
"method":"html",
"wrapper":"block-system-main",
"selector":"#ajax-link",
"url":"\/node\/1"
}
Comment | File | Size | Author |
---|---|---|---|
#18 | drupal.ajax-defaults.18.patch | 2.3 KB | sun |
#4 | drupal.ajax-defaults.4.patch | 2.69 KB | sun |
Comments
Comment #1
sunAlso...
Can someone explain me why we escape forward slashes...? To my knowledge, regular slashes don't have any special meaning within strings.
Comment #2
rfayYeah, forward slashes don't have any special meaning anywhere that I know of :-) Subscribing.
Comment #3
sunComment #4
sunApparently, Drupal.ajax defines the following defaults already:
Apparently, those defaults defined in JS have never been applied, because the server always sent defaults already. Even worse, the defaults defined on the server-side and on the client-side are different.
Attached patch removes all defaults from the server-side, and updates the client-side defaults to match the current server-side defaults.
Effectively, this patch
- removes a bloat of unnecessary output on pages with AJAX functionality, therefore speeding up HTML page rendering.
- decreases the size of AJAX settings in general, therefore also speeding up AJAX callbacks.
Comment #5
sunAny feedback? Patch is RTBC for me.
Comment #6
sunThis patch is required to make any sense of #850612: setting class 'use-ajax-submit' for form submission causes js error(s)
Comment #7
attiks CreditAttribution: attiks commentedTested on latest CVS with examples module and everything works still the same.
Comment #8
webchickWe need an AJAX person to review this, like rfay, merlinofchaos, quicksketch, etc.
And I need some justification that this won't break anything. I see that attiks tried it with Examples module, but I'm not sure how extensively that tests our existing AJAX system.
Comment #9
webchickComment #10
sunFeedback, anyone?
That said, Examples module pretty much tests everything the AJAX framework is capable to do. So if manual testing of all those examples was positive, there's little room for possible breakage.
Comment #11
sunI still need this patch in order to fix #850612: setting class 'use-ajax-submit' for form submission causes js error(s)
Comment #12
rfayMy reservation about this is that it adds a WTF into forms code. Everybody would assume that the defaults would be set in PHP, so it makes it harder to understand what's going on.
Comment #13
sunThe default values that are applied in PHP are almost invisible for any code. The only place where those current default settings are currently visible is hook_js_alter(), and FWIW, hook_ajax_alter().
#ajax and all the code in ajax.inc is merely a wrapper around #attached in order to apply additional logic for Drupal.settings that are going to be output. But effectively, the heavy logic of the AJAX framework lives in ajax.js.
Normally, all JavaScript/jQuery code always applies default "options" within the JavaScript context/environment. Many examples can be found in the jQuery and jQuery UI libraries.
The fact that we already have code in ajax.js that tries to apply default settings only underlines that this is nothing new, but actually is how it was originally expected to work.
However, the defaults in ajax.js are almost never applied currently, because we already pass all settings via Drupal.settings. Even worse, if the defaults defined in JS would be applied currently, then you would get different results.
The AJAX framework is going to be used by lots of modules in D7. Every single #ajax definition in PHP leads to a bloat of Drupal.settings being output in the HTML source of a page currently. Without any technical need or requirement that's just a major waste and abuse of server/client resources and bandwidth.
Comment #14
rfayThanks for the detailed explanation. It makes sense to me.
Comment #15
sunIf it makes sense to you, can we move forward? AJAX functionality has been manually tested already.
Comment #16
sun#4: drupal.ajax-defaults.4.patch queued for re-testing.
Comment #18
sunRe-rolled for #954804: All .js in /misc should be registered as a library
Comment #20
rfayTestbot failure, putting NR. I requested retest on qa.
Comment #21
rfayI took a full test run with this on AJAX Example and see no problem with it. When the bot comes back green I'd say it's ready to fly.
A real problem with settings bloat is the very real bloat. Lots of things are doing settings that aren't needed in an AJAX callback.
So on my D7 instance *with* this patch:
Only the last two items were relevant to the actual AJAX call.
Note that this is not at all an objection to this patch - has nothing to do with it. But we should discuss how we might somehow get rid of the irrelevant settings.
Comment #23
sun#18: drupal.ajax-defaults.18.patch queued for re-testing.
Comment #24
rfayOK, I think it's ready to fly. I found no problems in my explorations, and it's a pretty simple patch doing a pretty reasonable thing.
Comment #25
sun#18: drupal.ajax-defaults.18.patch queued for re-testing.
Comment #26
sun#18: drupal.ajax-defaults.18.patch queued for re-testing.
Comment #27
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.