As more JavaScript enabled modules are available, it's quite a problem to not get collisions between their configuration variables. This patch provides a centralized place where modules can put their settings that are required for the current page by putting them in the global (JavaScript) variable Drupal.

This patch also cleans up the act of adding a JavaScript file to the page by exposing the HTML code in a theme function for easy manipulation.

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

kkaefer’s picture

FileSize
3.77 KB

Tweaked it a bit to use Drupal.settings instead of Drupal. Removed the theme function as script tags don't need to be themed.

kkaefer’s picture

Title: A centralized place for JavaScript settings » Streamline JavaScript addition and add settings storage

(Changed title to better reflect what this patch does)

beejeebus’s picture

big +1 to the concept from me.

this will make moving the chatroom module to jQuery and HEAD easier and cleaner.

i'll review the code this weekend.

ax’s picture

another +1 to the concept from me.

this will also make the 4.7 update for AJAX Spellcheck and the Javascript Tools JS insertion easier and cleaner.

hope to find time to review the patch later.

kkaefer’s picture

FileSize
3.77 KB

Rerolled against HEAD (due to http://drupal.org/node/73961).

kkaefer’s picture

FileSize
10.89 KB

Changed the patch in huge parts. It now uses the same system as drupal_add_css() (different 'layers').

Here is the (very comprehensive) documentation for the new

drupal_add_js<code>: (Note that the function itself is not that complex)

<code>The behavior of this function depends on the parameters it is called with. Generally, it handles
the addition of JavaScript to the page, either as reference to an existing file or as inline code.
The following actions can be performed using this function:

- Add a file ('core', 'module' and 'theme'):
  Adds a reference to a JavaScript file to the page header. JavaScript files are placed in a certain order
  from 'core' first, to 'module' and finally 'theme' so that files that are later added, can override
  previously added files with ease.

- Add inline JavaScript code ('page'):
  Executes a piece of JavaScript code on the current page by placing the code directly in the head of 
  the page. This can, for example, be useful to tell the user that a new message arrived using a pop up, 
  alert box or similar method.

- Call a JavaScript function ('call'):
  Generates a Javascript call, while importing the arguments as is. PHP arrays are turned into JS objects 
  to preserve keys. This means the array keys must conform to JS's member naming rules.

- Add settings ('setting'):
  Adds a setting to Drupal's global storage of JavaScript settings. Per-page settings are required by
  some modules to function properly. The settings will be accessible at Drupal.settings[$flag].

@param $data
  (optional) If given, the value depends on the $type parameter:
  - 'core', 'module' or 'theme': The path to the JavaScript file relative to the base_path().
  - 'page': The JavaScript code that should be placed directly in the head of the page.
  - 'setting': An array with configuration options as associative array.
  - 'call': The name of the function that should be called on page load.     
@param $type
  (optional) The type of JavaScript that should be added to the page. Allowed values are 'core',
  'module', 'theme', 'page', 'setting' and 'call'.
@param $flag
  (optional) The value of this parameter depends on the $type parameter:
  - 'core', 'module', 'theme' and any undefined value: Sets whether the JavaScript file should be
    cached (TRUE, default value) or loaded anew on every page (FALSE).
  - 'setting': The name of the module that adds settings to the page.
  - 'page' and 'call': The value should be omitted.
@param ...
  (optional) If the parameter $type is set to 'call', every further parameter after $type is passed
  as an argument to the function called in the page header.
@return
  The JavaScript array that has been built so far.
kkaefer’s picture

Fixed a misnamed variable. In one place, I had 'arguments' and in the other, I had 'parameters'.

I forgot to mention that I replaced drupal_call_js() with drupal_add_js('function', 'call' [, parameters]).

kkaefer’s picture

FileSize
10.89 KB

Argh, patch didn't get attached. Here it is.

rkerr’s picture

I haven't actually tried this patch yet, but having dynamic adding of css and javascript files behave in the same manner is definitely going to make it easier for developers.

Dries’s picture

Normally we wrap PHPDoc comments around column 80. This is not a strict requirement, but your lines are kinda long. They hardly fit on my 12" screen.

I'm not a JavaScript expert so I can't comment on the functionality itself, or how convenient the various optional paramaters combinations are.

kkaefer’s picture

FileSize
10.89 KB

Made the PHPDoc comment wrap at column 80.

m3avrck’s picture

I *really* like this patch. Don't have time to thoroughly test it, but code looks great, awesome job!

drumm’s picture

Whats all this setting and call stuff?

kkaefer’s picture

The call function has been there before, as drupal_call_js(). It is required by some contributed modules. The settings function is pretty important for future JavaScript modules. See for example http://drupal.kkaefer.com/ for an example of how settings can be used.

kkaefer’s picture

FileSize
10.91 KB

Rerolled against HEAD. Changed array_merge to array_merge_recursive to prevent overwriting settings.

kkaefer’s picture

Patch still applies.

beejeebus’s picture

code looks good, and this patch worked as advertised for adding files, adding config vars, adding js and calling functions with and without args.

good work tim!

don't have enough karma around here to set it to RTBC, but it looks good to go.

this patch still leaves blocks unable to add js for all core themes, but that's a different issue.

m3avrck’s picture

Status: Needs review » Needs work

First off, I really like this patch, makes things consistent with drupal_add_css().

Next, timcn, you forgot your settings.php in there ;-)

Next up, in this case:

        case 'setting':
          $output .= '<script type="text/javascript">if (typeof Drupal == "undefined") Drupal = {}; Drupal.settings = '. drupal_to_js($data) .";</script>\n";
          break;

What if mutlipe JS files define settings? That seems to overwrite each other.

Also, what about Google Analytics? Why? Well, that places code that goes in the *footer*, just before , same with other modules that place JS in the $closure. How would that work?

And what if a module wants to output Javascript in the page, like TinyMCE? Should it use drupal_add_js()?

Inline seems like it places JS within the page, not within the head. Maybe make the distinction between JS files in head and actual JS code in head, verse JS code in the body?

Otherwise, looks great!

kkaefer’s picture

Next, timcn, you forgot your settings.php in there ;-)

I actually can't find any settings.php rules in the patch.

What if mutlipe JS files define settings? That seems to overwrite each other.

In drupal_add_js(), no HTML code is generated. The values are just merged into the global (static actually) settings array. The code for settings is generated in drupal_get_js() when all modules have merged their settings into the array. That means, all settings are there. I am successfully running this patch with quite some JavaScript code (from different modules) for my SoC project.

Also, what about Google Analytics? Why? Well, that places code that goes in the *footer*, just before , same with other modules that place JS in the $closure. How would that work?

You're right. There is definitely a need to add JavaScript code to the $closure. We should figure out, in what places it makes sense to position JavaScript code.

m3avrck’s picture

Timnc, yeah sorry about the settings.php comment, no idea where that came from. Clearly not in there :-)

As for including multiple JS files, what I was talking about was:

 case 'setting':
$output .= '<script type="text/javascript">if (typeof Drupal == "undefined") Drupal = {}; Drupal.settings = '. drupal_to_js($data) .";</script>\n";

I see that it'll apend multiple script tags no problem. However, this part: Drupal.settings = '. drupal_to_js($data) , each subsequent script will overwrite that, no?

kkaefer’s picture

FileSize
12.01 KB

Updated the patch. Note that settings was not broken before. The arrays were merged together so that only one Drupal.settings was required.

Changes

  • Scopes allow you to insert your code in other locations than the head of the page. The default scope is 'header', but there is also a scope called 'footer' available. Themes can define and use arbitrary scopes.
  • Removed $type = 'call' as it is probably easier to write drupal_add_js('alert("message")', 'inline'); than using the parameter construction.
  • Renamed page to inline as it is easier to understand.
  • Reworked the setting type:
    • It now uses a cool JavaScript function to extend the JavaScript Drupal object
    • You might now want to wrap your settings into another variable in order to prevent the pollution of the Drupal.settings namespace.
    • array_merge_recursive is now only called once. Before, it was called whenever you added a setting.
  • Added the ability to defer the execution of JavaScript code. Take a look at the MSDN page if you don’t know what that means.
  • The parameter $flags is not overloaded anymore.
kkaefer’s picture

Status: Needs work » Needs review

I forgot to mention that I will work on JavaScript localization once this patch is in core.

kkaefer’s picture

FileSize
11.99 KB

Damn, forgot a print_r() in the code.

m3avrck’s picture

Timcn, excellent job! You addressed all of my concerns with the patch.

Looking through it now, looks very solid. This will go a long way to unifying a consisten approach to adding JS to a Drupal page, much like drupal_add_css() did this for CSS.

I'd say it's RTBC, but maybe someone else wants to have a review first? Thumbs up here.

beejeebus’s picture

i'll review and test this tonight. (GMT +10 Sydney)

kkaefer’s picture

FileSize
12.01 KB

Rerolled the patch.

beejeebus’s picture

tested again, and it still works as advertised.

i think the docs need an update here though:

*   - 'inline': The JS code that should be placed directly in the page head.

this needs to take into account the $scope param, and the fact that the code might not be placed in the page head.

nice work :-)

kkaefer’s picture

FileSize
13.06 KB

You're right. I changed the documentation for 'inline'. Thanks for testing.

kkaefer’s picture

FileSize
12.09 KB

Fixed a wrong indentation in drupal.js and took a file out that has nothing to do here ;)

kkaefer’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
12.02 KB

Rerolled the patch to remove fuzziness. Setting this RTBC.

m3avrck’s picture

Agreed, we need this to go in before JQuery hits... this will really help out.

Dries’s picture

Committed to CVS HEAD. Marking this 'code needs work'. When this is properly documented, switch to 'fixed'. Thanks timcn! :)

drewish’s picture

Status: Reviewed & tested by the community » Needs work

looks like dries forgot to change the status...

kkaefer’s picture

As per Steven's request, here is a documentation patch for Drupal.extend().

kkaefer’s picture

FileSize
1.06 KB

As per Steven's request, here is a documentation patch for Drupal.extend().

m3avrck’s picture

A bit wordy "Arbitrary objects containing for example functions or variables can be used. " ... how about "Arbitrary objects, such as functions or variables, can be used."

kkaefer’s picture

Well, actually objects can contain functions and variables. While functions are also objects, variables are not.

kkaefer’s picture

Status: Needs work » Needs review
FileSize
1.07 KB

Updated the module upgrade page.

Updated the documentation patch with m3avrck's suggestion.

m3avrck’s picture

Status: Needs review » Reviewed & tested by the community

Looks good now.

Dries’s picture

Steven will have to commit this -- I don't know what his requests were.

drumm’s picture

This looks straightforward enough. What/where were Steven's requests?

drumm’s picture

Status: Reviewed & tested by the community » Fixed

I was informed via IRC that Steven simply wanted documentation.

Committed to HEAD.

kkaefer’s picture

Status: Fixed » Reviewed & tested by the community
FileSize
938 bytes

Just discovered that a variable has been misnamed.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD.

Anonymous’s picture

Status: Fixed » Closed (fixed)