Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#43 | drupal_add_js_9.patch | 938 bytes | kkaefer |
#38 | drupal.extend_0.patch | 1.07 KB | kkaefer |
#35 | drupal.extend.patch | 1.06 KB | kkaefer |
#30 | drupal_add_js_8.patch | 12.02 KB | kkaefer |
#29 | drupal_add_js_7.patch | 12.09 KB | kkaefer |
Comments
Comment #1
kkaefer CreditAttribution: kkaefer commentedTweaked it a bit to use Drupal.settings instead of Drupal. Removed the theme function as script tags don't need to be themed.
Comment #2
kkaefer CreditAttribution: kkaefer commented(Changed title to better reflect what this patch does)
Comment #3
Anonymous (not verified) CreditAttribution: Anonymous commentedbig +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.
Comment #4
ax CreditAttribution: ax commentedanother +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.
Comment #5
kkaefer CreditAttribution: kkaefer commentedRerolled against HEAD (due to http://drupal.org/node/73961).
Comment #6
kkaefer CreditAttribution: kkaefer commentedChanged 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
Comment #7
kkaefer CreditAttribution: kkaefer commentedFixed 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()
withdrupal_add_js('function', 'call' [, parameters])
.Comment #8
kkaefer CreditAttribution: kkaefer commentedArgh, patch didn't get attached. Here it is.
Comment #9
rkerr CreditAttribution: rkerr commentedI 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.
Comment #10
Dries CreditAttribution: Dries commentedNormally 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.
Comment #11
kkaefer CreditAttribution: kkaefer commentedMade the PHPDoc comment wrap at column 80.
Comment #12
m3avrck CreditAttribution: m3avrck commentedI *really* like this patch. Don't have time to thoroughly test it, but code looks great, awesome job!
Comment #13
drummWhats all this setting and call stuff?
Comment #14
kkaefer CreditAttribution: kkaefer commentedThe 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.Comment #15
kkaefer CreditAttribution: kkaefer commentedRerolled against HEAD. Changed
array_merge
toarray_merge_recursive
to prevent overwriting settings.Comment #16
kkaefer CreditAttribution: kkaefer commentedPatch still applies.
Comment #17
Anonymous (not verified) CreditAttribution: Anonymous commentedcode 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.
Comment #18
m3avrck CreditAttribution: m3avrck commentedFirst 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:
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!
Comment #19
kkaefer CreditAttribution: kkaefer commentedI actually can't find any settings.php rules in the patch.
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.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.
Comment #20
m3avrck CreditAttribution: m3avrck commentedTimnc, 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:
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?Comment #21
kkaefer CreditAttribution: kkaefer commentedUpdated the patch. Note that settings was not broken before. The arrays were merged together so that only one
Drupal.settings
was required.Changes
'header'
, but there is also a scope called'footer'
available. Themes can define and use arbitrary scopes.$type = 'call'
as it is probably easier to writedrupal_add_js('alert("message")', 'inline');
than using the parameter construction.page
toinline
as it is easier to understand.setting
type:Drupal.settings
namespace.array_merge_recursive
is now only called once. Before, it was called whenever you added a setting.defer
the execution of JavaScript code. Take a look at the MSDN page if you don’t know what that means.$flags
is not overloaded anymore.Comment #22
kkaefer CreditAttribution: kkaefer commentedI forgot to mention that I will work on JavaScript localization once this patch is in core.
Comment #23
kkaefer CreditAttribution: kkaefer commentedDamn, forgot a
print_r()
in the code.Comment #24
m3avrck CreditAttribution: m3avrck commentedTimcn, 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.
Comment #25
Anonymous (not verified) CreditAttribution: Anonymous commentedi'll review and test this tonight. (GMT +10 Sydney)
Comment #26
kkaefer CreditAttribution: kkaefer commentedRerolled the patch.
Comment #27
Anonymous (not verified) CreditAttribution: Anonymous commentedtested again, and it still works as advertised.
i think the docs need an update here though:
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 :-)
Comment #28
kkaefer CreditAttribution: kkaefer commentedYou're right. I changed the documentation for 'inline'. Thanks for testing.
Comment #29
kkaefer CreditAttribution: kkaefer commentedFixed a wrong indentation in drupal.js and took a file out that has nothing to do here ;)
Comment #30
kkaefer CreditAttribution: kkaefer commentedRerolled the patch to remove fuzziness. Setting this RTBC.
Comment #31
m3avrck CreditAttribution: m3avrck commentedAgreed, we need this to go in before JQuery hits... this will really help out.
Comment #32
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Marking this 'code needs work'. When this is properly documented, switch to 'fixed'. Thanks timcn! :)
Comment #33
drewish CreditAttribution: drewish commentedlooks like dries forgot to change the status...
Comment #34
kkaefer CreditAttribution: kkaefer commentedAs per Steven's request, here is a documentation patch for
Drupal.extend()
.Comment #35
kkaefer CreditAttribution: kkaefer commentedAs per Steven's request, here is a documentation patch for
Drupal.extend()
.Comment #36
m3avrck CreditAttribution: m3avrck commentedA 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."
Comment #37
kkaefer CreditAttribution: kkaefer commentedWell, actually objects can contain functions and variables. While functions are also objects, variables are not.
Comment #38
kkaefer CreditAttribution: kkaefer commentedUpdated the module upgrade page.
Updated the documentation patch with m3avrck's suggestion.
Comment #39
m3avrck CreditAttribution: m3avrck commentedLooks good now.
Comment #40
Dries CreditAttribution: Dries commentedSteven will have to commit this -- I don't know what his requests were.
Comment #41
drummThis looks straightforward enough. What/where were Steven's requests?
Comment #42
drummI was informed via IRC that Steven simply wanted documentation.
Committed to HEAD.
Comment #43
kkaefer CreditAttribution: kkaefer commentedJust discovered that a variable has been misnamed.
Comment #44
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD.
Comment #45
(not verified) CreditAttribution: commented