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"
}
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

Also...

"url":"\/node\/1"

Can someone explain me why we escape forward slashes...? To my knowledge, regular slashes don't have any special meaning within strings.

rfay’s picture

Yeah, forward slashes don't have any special meaning anywhere that I know of :-) Subscribing.

sun’s picture

Assigned: Unassigned » sun
sun’s picture

Status: Active » Needs review
FileSize
2.69 KB

Apparently, Drupal.ajax defines the following defaults already:

  var defaults = {
    url: 'system/ajax',
    event: 'mousedown',
    keypress: true,
    selector: '#' + base,
    effect: 'none',
    speed: 'slow',
    method: 'replaceWith',
    progress: {
      type: 'bar',
      message: 'Please wait...'
    },
    submit: {
      'js': true
    }
  };

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.

sun’s picture

Any feedback? Patch is RTBC for me.

sun’s picture

attiks’s picture

Status: Needs review » Reviewed & tested by the community

Tested on latest CVS with examples module and everything works still the same.

webchick’s picture

Status: Reviewed & tested by the community » Needs work

We 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.

webchick’s picture

Status: Needs work » Needs review
sun’s picture

Feedback, 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.

sun’s picture

rfay’s picture

My 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.

sun’s picture

The 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.

rfay’s picture

Thanks for the detailed explanation. It makes sense to me.

sun’s picture

If it makes sense to you, can we move forward? AJAX functionality has been manually tested already.

sun’s picture

Issue tags: -frontend performance

#4: drupal.ajax-defaults.4.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +frontend performance

The last submitted patch, drupal.ajax-defaults.4.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
2.3 KB

Status: Needs review » Needs work

The last submitted patch, drupal.ajax-defaults.18.patch, failed testing.

rfay’s picture

Status: Needs work » Needs review

Testbot failure, putting NR. I requested retest on qa.

rfay’s picture

I 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:

[{"command":"settings","settings":{

"basePath":"\/",

"ajaxPageState":{"theme":"bartik","theme_token":"_oXbIX5U4gyIJair810jPAi5d30tVgK4JxAT0AD9xOs","css":[]} //end ajaxPageState,

"colorbox":{"transition":"elastic","speed":350,"opacity":"0.85","slideshow":false,"slideshowAuto":false,"slideshowSpeed":2500,"slideshowStart":"","slideshowStop":"","current":"{current} of {total}",

"previous":"\u00ab Prev","next":

"Next \u00bb",

"close":"Close",

"overlayClose":true,

"maxWidth":"100%",

"maxHeight":"100%",

"__drupal_alter_by_ref":["default"]},

"admin_menu":{"destination":"destination=system\/ajax","hash":"9dab10d7d7ae42afeba91ccfc1833b5d","basePath":"\/admin_menu","replacements":{".admin-menu-users a":"0 \/ 1"},"margin_top":1}},"merge":true},

{"command":"insert","method":"after","selector":"#after_div","data":"New 'after'...","settings":null},

{"command":"insert","method":"replaceWith","selector":"#after_status","data":"\u003cdiv id='after_status'\u003eUpdated after_command_example Mon, 06 Dec 2010 07:56:59 -0700\u003c\/div\u003e","settings":null}
]

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.

Status: Needs review » Needs work
Issue tags: -frontend performance

The last submitted patch, drupal.ajax-defaults.18.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
Issue tags: +frontend performance

#18: drupal.ajax-defaults.18.patch queued for re-testing.

rfay’s picture

Status: Needs review » Reviewed & tested by the community

OK, 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.

sun’s picture

Issue tags: -frontend performance

#18: drupal.ajax-defaults.18.patch queued for re-testing.

sun’s picture

Issue tags: +frontend performance

#18: drupal.ajax-defaults.18.patch queued for re-testing.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)
Issue tags: -frontend performance

Automatically closed -- issue fixed for 2 weeks with no activity.