Background, see #1279226-140: jQuery and Drupal JavaScript libraries and settings are output even when no JS is added to the page

This patch adds support for the new way to handle javascript and will only output jQuery if needed (or if javascript_always_use_jquery is set to TRUE, the default)

If you want to test this:

  1. Apply this patch and the patch in #1279226-140: jQuery and Drupal JavaScript libraries and settings are output even when no JS is added to the page
  2. Run drush vset javascript_always_use_jquery 0 -y
  3. Clear caches
CommentFileSizeAuthor
advagg-support-drupal_add_js_page_defaults.patch594 bytesattiks

Comments

mikeytown2’s picture

mikeytown2 commented

I like where this is going. Do you think we could integrate #1279226-140: jQuery and Drupal JavaScript libraries and settings are output even when no JS is added to the page into advagg so that the core patch is not needed, or is it tied too closely with ajax_render() (one of the other functions that uses drupal_get_js()) and thus requires a core patch?

mikeytown2’s picture

mikeytown2 commented

We could go one step further and scan all JS files to see if it contains a reference to drupal.* & jQuery (I can make this a minimal performance issue). If it does then add in the default JS. If it doesn't then don't add in the defaults even if JavaScript has been added. Along that same thought, have a attribute called core_defaults; if its set to FALSE and that JS is the only other JS added then we could safely remove core JS files.

attiks’s picture

attiks commented

#1 I first tried integrating this into magic, but

  1. we need to support this for ajax calls as well
  2. we have no idea if somebody is including a jquery plugin inside node.pl.php (I know, bad idea, but I've seen it)
  3. people wanting to use this, do not need to install advagg to have to speed improvements

#2 See my seconds remark, we cannot assume people are following best practices, that's we there's a switch 'javascript_always_use_jquery' to mimic the old behaviour so we don't break anything.

If you can review/rtbc the core patch, that would be great ;-)

mikeytown2’s picture

mikeytown2 commented
Status: Needs review » Fixed

#1 has been committed.

Another way to get this in without the need for a core patch would be to use hook_ajax_render_alter().

Using Gotta Download Them All I've come up with a list of modules that implement this hook. These are the modules we should examine/test with the core patch regardless of how it get's implemented.
grep -l -r -i "_ajax_render_alter(" ./

./commerce_coupon/includes/commerce_coupon.checkout_pane.inc
./commerce_userpoints/commerce_userpoints_payment_method/commerce_userpoints_payment_method.module
./devel/devel.module
./dialog/dialog.module
./drupalforkomodo/Abbreviations/PHP/Drupal 7/hooks/d7_ajax_render_alter.komodotool
./drupal_ruble/commands/hooks.rb
./easy_module/data/easy_module_hooks.data.php
./flagging_form/flagging_dialog.module
./fontyourface/fontyourface.module
./fontyourface/modules/fonts_com/fonts_com.module
./searchlight/searchlight.module
./textmate/Support/commands/generated/hooks/7/hook_ajax_render_alter.7.php
mikeytown2’s picture

mikeytown2 commented

http://drupal.org/project/commerce_coupon/ - commerce_coupon.checkout_pane.inc
http://drupal.org/project/commerce_userpoints/ - commerce_userpoints_payment_method.module
http://drupal.org/project/devel/ - devel.module
http://drupal.org/project/dialog/ - dialog.module - This needs to be tested for sure
http://drupal.org/project/drupalforkomodo/ - d7_ajax_render_alter.komodotool
http://drupal.org/project/drupal_ruble/ - hooks.rb
http://drupal.org/project/easy_module/ - easy_module_hooks.data.php
http://drupal.org/project/flagging_form/ - flagging_dialog.module - Mentions dialog module.
http://drupal.org/project/fontyourface/ - fontyourface.module, fonts_com.module
http://drupal.org/project/searchlight/ - searchlight.module
http://drupal.org/project/textmate/ - hook_ajax_render_alter.7.php

attiks’s picture

attiks commented
Status: Fixed » Active

#5 All modules using hook_library (as dialog does) will be fine, since there's an option 'need_jquery' (default TRUE) that forces the loading of jQuery no matter what

attiks’s picture

attiks commented
Status: Active » Fixed

Status: Fixed » Closed (fixed)

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