Move drupal_add_js $reset parameter as an option for $options['type']

Posted by aaron on December 11, 2008 at 10:48pm

6 followers

Issue Summary

This patch does what the title says. This allows for the possibility of ultimately allowing the option to override function parameters for #315100: Allow to add JS/CSS libraries (sets of files, settings, and dependent libraries).

That means that to reset js, you'll call drupal_add_js(NULL, 'reset'); rather than drupal_add_js(NULL, NULL, TRUE);

For completeness, we do the same with drupal_add_css. (Hoping we can merge those two functions later for cleaner code.) This patch also takes care of all simpletests for the two. I grepped core for drupal_add_js(NULL, NULL, TRUE); and drupal_add_css(NULL, NULL, TRUE); but found nothing.

There's also the possibility that something like drupal_add_js('myfile.js', 'header', TRUE); is called, which would be bad practice now. I did a quick scan through the API and didn't see anything, so it's probably safe.

Attachment	Size	Status	Test result	Operations
drupal_add_js_reset.patch	6.38 KB	Idle	Passed: 8972 passes, 0 fails, 0 exceptions	View details

Comments

#1

Posted by Rob Loach on December 12, 2008 at 5:13pm

Status:

active

» needs work

This is wicked awesome cool, but noticed this:

<?php
-function drupal_add_css($path = NULL, $options = NULL, $reset = FALSE) {
-  static $css = array();
+function drupal_add_css($path = NULL, $options = NULL) {
+static $css = array();
?>

Would be good to have the two spaces in there. I'll be able to hit this up sometime in the next 12 hours.

#2

Posted by aaron on December 13, 2008 at 2:26am

Status:

needs work

» needs review

thanks for catching that. here's an updated patch.

Attachment	Size	Status	Test result	Operations
drupal_add_js_reset.patch	6.36 KB	Idle	Passed: 8991 passes, 0 fails, 0 exceptions	View details

#3

Posted by Rob Loach on January 9, 2009 at 2:18pm

Status:

needs review

» reviewed & tested by the community

Yay!

#4

Posted by Rob Loach on January 9, 2009 at 2:22pm

Sorry, thought I should remove the fuzzies. Didn't change anything code-wise.

Attachment	Size	Status	Test result	Operations
345973.patch	5.91 KB	Idle	Passed: 8972 passes, 0 fails, 0 exceptions	View details

#5

Posted by quicksketch on January 12, 2009 at 9:07pm

Status:

reviewed & tested by the community

» needs work

Hrm, it seems to me that the documentation for the 'reset' parameter is inaccurate.

+ *   Note that if $options or $options['type'] is 'reset', then $path will be
+ *   ignored.

This doesn't seem to be the case, the processing continues even after the reset. So you could in theory add CSS/JS file at the same time as resetting all the current files. Should we add a "return" immediately following the if (options['reset']) or should we adjust the docs? The $options['reset'] option also needs a much better description, since it doesn't say what it actually does.