jQuery .once() method for adding behaviors once

Getting no errors in IE6 and 7, FF and Safari. A big +1 from me for a commit, I haven't been through everything but its looks like a great cleanup that doesn't break anything and just improves the way jquery is used.

I guess it was too big to notice at first, but I now get the Body textareas that disappear in all browsers when adding a node (tried blog and forum types).

Sounds like needs work then.

This patch is awesome. Adding some tags........... Somewhat related issue for behaviors that are more then once: #404902: Replace Drupal.behaviors with jQuery.live().

This is a great idea. It'll be nice to continue adding new native jQuery methods like the setSummary and getSummary methods added in the #323112: Vertical Tabs patch.

Despite being shorter and easier to implement, I think we're going to cause a large amount of confusion with this method. Upon looking at this issue, I immediately thought, "Oh I didn't know jQuery had a once() method!", and then went to docs.jquery.com to look it up. When it wasn't there I downloaded the source to jQuery itself and searched for "once". Then I was generally just confused and decided to read the patch. ;-)

The same thing happened when I looked at Vertical Tabs, but I figured that one out much faster because I was already pretty sure that set/getSummary weren't native jQuery methods. However, the shortness of this method name, and it's generic utility, make it seem to be a very likely method to be provided by jQuery itself, and for people looking for documentation on how to use once(), they'll probably experience the exact thing I went through, searching desperately for where this method comes from and how to use it.

So, I'd like to ask, is there some way we can name-space these methods as being Drupal-specific? While I agree that "once" is the perfect name for this method, it's completely unclear to me that Drupal would add this method. Considering all the jQuery methods are just thrown into a "function soup" after they've been defined, we're risking name-space collisions with other libraries that might declare the same methods. This is a bit different from PHP namespace problems, which for the most part we just assume people don't combine Drupal with other PHP programs in the same memory, but for jQuery methods, it's completely common for a single site to be using many different jQuery libraries that all need to share the same space.

Some possibilities to avoid this problem. Prefix all methods with "drupal" or "d", like drupalOnce and drupalSetSummary. Or make a universal class for Drupal methods, so we could use $().drupal.once() and $().drupal.setSummary().

It looks like adding an object within jQuery is not as common as directly declaring methods, but it's not unheard of. Some projects that namespace multiple methods within a single object:

http://plugins.jquery.com/project/AjaxFilter jQuery.ajaxFilter.register()
http://plugins.jquery.com/project/zendjsonrpc jQuery.Zend.jsonrpc({url: 'rpc.php'});

If we start converting a lot of Drupal's JavaScript to be jQuery methods, we'll run into name spacing issues frequently. Some method names that are already "claimed" by other jQuery libraries that we'd probably want to use:

thobber()
progressbar()
autocomplete()

There are also jQuery plugins for just about everything Drupal core offers (table drag and drop, resizable textareas, sticky table headers, etc.) Note that I'm not suggesting we should use the jQuery plugins instead of using our own, I'm saying that it'd be great to convert Drupal's core JS files to be jQuery methods, but not collide with other plugins from the broader jQuery community.

Regarding Nate's comments about deciding on a convention for these jQuery extensions that we create explicitly for Drupal, I think he's right in saying that we should namespace it, but at the same time, I think that it should be tackled in a separate issue.

We had a brief discussion about it and thought that namespacing it into a "Drupal" class might be a good idea:

• $(".something").Drupal.once();
• $('#field-summary').Drupal.setSummary();

Using the uppercase for the JavaScript class name seems like the way to go as Sizzle.js uses it, as does Processing.js.

In hopes to not de-rail this thread, I created #445130: Namespace the Drupal jQuery functions. Other then that, I'm very confident with this patch.

Perfect in all browsers here, I think for this issue it just needs review, this cleanup would not interfere with future namespacing. Just to make sure, any theming hooks using those .addClass('ahah-processed') that get removed?

My vote is in for $('select.timezone-detect').drupalOnce();. The other presented solutions just don't really look that pretty.

Although I'd rather not use it, another solution Konstantin missed here would be to take $d as a Drupal JavaScript function, like how jQuery $ is a JavaScript function......... $d('select.timezone-detect').once(). Again, my vote is still in for $('select.timezone-detect').drupalOnce();.

We need chaining...

If the .once() plugin is being enclosed in the wrapper

(function($) {

})(jQuery);

why is it enclosed by an anonymous function?

Um, yeah. In the future I'll try to read those code comments that clearly state what's going on :)

Went through all the JavaScript and tested the instances that used the new .once plugin. Worked nicely, kkaefer did a very thorough job here.

Instead of sticking this in Drupal.js, what about sticking it in jquery.once.js, and having it always loaded by default? Like how jquery.js and drupal.js are loaded by default? This could also mean that we could minify it and just document that it's the jQuery Once plugin with the URL.

@Rob Loach I like the idea of having it separated from drupal.js. Since this is technically an external library. We should have it separated out for easier updates, swapping it out with modules like jQuery Update when needed, and maintainability in general.

Marking CNW to do this change.

Moved to jquery.once.js.

This is a great idea. I wrote a big paragraph about it all a few seconds ago and lost it, dang! try again (I now have the Lazarus plugin in Firefox...)

So, I was going to say that this could be a great way to solve the problem of adding jQuery UI components to core #315035: Add jQuery UI elements to core. We could add a stable ui.core.js (or stick with a stable, or even custom one?) for basic UI methods. We could also have a custom (or stable version of) ui.draggable.js and ui.dialog.js for example. It would be a great way to have better jQuery support, provided we document it well of course. Personally, when coding a jQuery plugin or anything with jQuery, I don't really care about the methods I'll have to use. First, because it's jQuery anyway, and second, I just want some UI methods available so I don't have to reinvent the wheel, and mostly know that everything will be consistent in my module's JavaScript. The #315100: Allow to add JS/CSS libraries (sets of files, settings, and dependent libraries) could still exist for extra UI plugins since that needs to be addressed also. I still think this should be left for modules to handle and trying to concentrate efforts on that a little more for plugins.

There's also that my imageplacement module depends on ui.core and ui.draggable, and that with imagefield in core, it's something that I'd like to see there too ;)

Updated to HEAD.

Updated to HEAD.

Setting to 'needs review' - testbot was broken.

Updated with use of #315100: Allow to add JS/CSS libraries (sets of files, settings, and dependent libraries) as well as updated the Toolbar to use it.

Updated to HEAD.

Quicksketch found a couple of places where the context wasn't brought over. tha_sun asked for a -N cvs reroll.

I'm not sure whether this .once() approach is the way to go... it equals more or less what we're doing all over the place currently, but instead of introducing a (slow) class-based lookup, we might have a look into jQuey.once() and advance on that instead...? (I didn't look at that yet, but in general, the _direction_ feels wrong)

Also, I think that this patch doesn't account for detaching behaviors. I can understand that detaching behaviors are hard to support, given that Drupal core doesn't require/use them on its own... but still.

If I get this right, .once() should be named .attach() - to match our current naming...?

well, it shouldn't be called .attach(), because attaching behaviors are invoked multiple times. But given that confusion, there should probably also be an .attach() behavior.

Are you thinking of jQuery live() and die()? The unfortunate thing about them is that they only supports a set of events, and don't give "setup" or "cleanup" events. There was a short discussion about this over at #404902: Replace Drupal.behaviors with jQuery.live(), and I think it would be great, but it doesn't give us all the support we need yet. Konstantin called the plugin jQuery Once, and that does make sense.

You're right about detaching behaviors. What we could look at is extending once to support removeOnce(), which would check for -processed, remove the class and run the actions. Any thoughts? Know of any place in core that removes the behaviors yet?

Regarding the performance of class manipulation, litwol brought up the idea of using jQuery Data to store the information, but that might be out of scope for this issue....

heh, yes, seems like I was horribly confused. That should have read "but instead of introducing a (slow) class-based lookup, we might have a look into jQuey.live() and advance on that instead", i.e. take over the internal mechanism to attach events only once to certain elements.

As of now, I think that we do not have any .detach() method in core. I pushed that patch in for better wysiwyg support in D7. I'm not sure which other use-cases exist for detaching behaviors, but it's possible there are some.

Since we already define behaviors with .attach() and .detach() methods, maybe it would be most sane to name the functions here $.attachOnce() and $.detachOnce() ? OTOH, I'd say that behaviors are always detached only once.

Another issue with $.once() is that detach() methods need to know/access the storage to determine whether they need to detach. So if we abstract the attaching, we should also abstract the detaching.

bad trial on new testing bot

bad trial run of new testing bot again

This includes $.removeOnce() which acts on elements that have already been processed by once() and removes the processed class. It's not used anywhere in core yet since we don't detach any elements. Almost seems out of scope for this issue, but its reasons are valid. Thanks, sun.

This patch also doesn't touch ajax.js since it seems there are still a bunch of patches going into it at #544418: Integrate CTools AJAX framework with Drupal to extend (and replace) existing ahah framework. We can use jQuery Once on it as soon as the patches on it slow down.

+++ misc/jquery.once.js	1 Jan 1970 00:00:00 -0000

In general, this looks nice and makes sense.

Though I'm still not sure whether we should not name these methods .attachOnce() and .detachOnce(), following our current naming pattern.

Along with the following remarks, I'd propose to move this into drupal.js, since we will do various adjustments anyway.

That said, it seems like most of our attaching behaviors just contain $(selector).once(class, function); with that now. I wonder whether it wouldn't make sense to enhance our behavior object with attachOnce and detachOnce methods, so Drupal.attachBehaviors() would take over the handling of $.once().

+++ misc/jquery.once.js	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,69 @@
+   * Filters elements that have not yet been processed.

"Filters elements by whether they have not yet been processed."

+++ misc/jquery.once.js	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,69 @@
+   *   If this is a string, it will be used in the class name applied to

s/it will be used in the class name applied/then it will be used as a CSS class name that is applied/

+++ misc/jquery.once.js	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,69 @@
+   *   The elements will get a class in the form of id-processed.

The class name should be wrapped in single quotes.

+++ misc/jquery.once.js	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,69 @@
+   *   Otherwise, this acts as a unique identifier; the id will be a generated
+   *   number. If it's a function, it will additionally be called for each
+   *   element using .each().

I don't understand this part. There seem to a variety of ways to invoke $.once(), so ideally we explain them in the JSDoc, potentially using example code snippets.

+++ misc/jquery.once.js	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,69 @@
+   *   has not yet been processed. The function should not return a value.
...
+   *   has not yet been processed. The function should not return a value.

No, the function follows $.each()'s logic:

return false; entirely breaks the iteration.

return;/return true; continues to the next matched element in the set.

+++ misc/jquery.once.js	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,69 @@
+   * @version 1.2
+   * @see http://plugins.jquery.com/project/once
+   *
+   * Copyright (c) 2009 Konstantin Kaefer <mail@kkaefer.com>
+   * Dual licensed under the MIT and GPL licenses.
...
+   * @version 1.2
+   * @see http://plugins.jquery.com/project/once
+   *
+   * Copyright (c) 2009 Rob Loach <http://www.robloach.net>
+   * Dual licensed under the MIT and GPL licenses.

If we move this drupal.js, may we remove those? In case not, that stuff belongs to a starting @file JSDoc at the top of the file, not into every single function.

+++ misc/jquery.once.js	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,69 @@
+      // Generate a numeric ID if the id passed can't be used as a CSS class.
+      if (!(id in cache)) {
+        cache[id] = ++uuid;

What happens if id is an array or object?

Additionally, the comment should be reworded to not claim that it validates the id for usage in a CSS class, because that does not happen.

+++ misc/jquery.once.js	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,69 @@
+        cache[id] = ++uuid;

It seems this cache does not take context into account?

+++ misc/jquery.once.js	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,69 @@
+      // When there is second parameter, we have to save it so that we can
+      // call it later.
+      if (!fn) {
+        fn = id;
+      }
...
+    return $.isFunction(fn) ? elements.each(fn) : elements;

I believe a "no" is missing here. Could also be "a". But then again, that comment does not make much sense when looking at the code that follows.

+++ misc/jquery.once.js	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,69 @@
+    // Filter out elements that have been processed already.
+    var name = id + '-processed',
+        elements = this.not('.' + name).addClass(name);
...
+    var name = id + '-processed',
+        elements = this.filter('.' + name).removeClass(name);

Can we write this on two separate var lines, please?

+++ misc/jquery.once.js	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,69 @@
+   * Filters elements that have been processed once already.

s/already//

+++ misc/jquery.once.js	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,69 @@
+   *   been processed by the once function. The id should be the same id that
+   *   was originally passed to the once() function.

Quite possibly, I can't retrieve the auto-generated uuid in the cache, when $.once() was used without id? Can we make the internal cache accessible?

12 days to code freeze. Better review yourself.

Back when this issue began, we realized that others might like to take advantage of $().once(), so Konstantin stuck up http://plugins.jquery.com/project/once . I think it's best not to fork off of it, but rather keep it at the upstream. Managing it as jquery.once.js rather then sticking it as part of drupal.js also allows us to manage it through hook_library() so that we could swap out an updated version of it later on through contrib if we wanted to. It is technically an external library. When JavaScript aggregation is on, it will be merged into one file anyway.

This patch fixes a bunch of the documentation issues that sun brought up. Looks pretty good now. For the JSDoc file header, I copied what jquery.form.js has.....

/**
 * jQuery Once Plugin v1.2
 * http://plugins.jquery.com/project/once
 *
 * Dual licensed under the MIT and GPL licenses:
 *   http://www.opensource.org/licenses/mit-license.php
 *   http://www.gnu.org/licenses/gpl.html
 */

+++ misc/textarea.js	19 Aug 2009 18:08:01 -0000
@@ -3,20 +3,16 @@
+      var staticOffset = null,
+          textarea = $(this).wrap('<div class="resizable-textarea"><span></span></div>'),
+          grippie = $('<div class="grippie"></div>').mousedown(startDrag);

Please write these on separate lines.

+++ misc/tableheader.js	19 Aug 2009 18:08:01 -0000
@@ -17,7 +17,7 @@
-    $('table.sticky-enabled thead:not(.tableHeader-processed)', context).each(function () {
+    $('table.sticky-enabled thead', context).once('table-header', function () {

I think we should live this as 'tableheader' (all-lowercase) to match the name of the behavior.

+++ misc/form.js	19 Aug 2009 18:08:01 -0000
@@ -61,8 +61,7 @@
 Drupal.behaviors.multiselectSelector = {
...
-    $('.multiselect select:not(.multiselectSelector-processed)', context)
-      .addClass('multiselectSelector-processed').change(function () {
+    $('.multiselect select', context).once('multiselect-selector').change(function () {

Same here. Though, not sure whether we couldn't strip "selector".

+++ modules/system/system.js	19 Aug 2009 18:08:01 -0000
@@ -32,7 +32,7 @@
-    if (!($('#edit-clean-url').size()) || $('.clean-url-processed, #edit-clean-url.install').size()) {
+    if (!($('#edit-clean-url').size()) || $('#edit-clean-url.install').once('clean-url').size()) {

Let's convert .size() into .length while being there.

This hunk is actually the most critical in the patch - I'm not sure whether the resulting behavior is still the same. (I really hope we split this code into a separate install.js.)

+++ includes/common.inc	19 Aug 2009 18:08:01 -0000
@@ -2943,8 +2943,9 @@
-      // jQuery itself is registered as a library.
+      // jQuery itself is registered as a library as well as jQuery.once().

I don't think it makes sense to list all required libraries here. Just state that we add the required. ;)

+++ modules/color/color.js	19 Aug 2009 18:08:01 -0000
@@ -4,10 +4,10 @@
+    if (!form.size()) {

Please use .length here, too.

+++ misc/jquery.once.js	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,79 @@
+  var cache = {}, uuid = 0;
+  /**

Can we squeeze a blank line in between there?

Aside from those issues, I think this is ready to fly.

I'm on crack. Are you, too?

Addresses all points in #53 by sun.

@kkaefer: $.size() is a dumb wrapper around {}.length. {}.length thereby avoids a function call, but is also more comprehensible to people who understand basic JavaScript, but not yet all of jQuery. I actually hope that jQuery folks will remove .size() to remove this confusion.

+++ misc/drupal.js	20 Aug 2009 03:00:05 -0000
@@ -38,7 +37,7 @@
- * Behaviors should use a class in the form behaviorName-processed to ensure
+ * Behaviors should use a $('...').once('name', function () { ... }) to ensure
  * the behavior is attached only once to a given element. (Doing so enables

Following the names that have been applied to core behaviors, I think this should state:

Behaviors should use a $(selector).once('behavior-name', function () {}) to ensure...

This review is powered by Dreditor.

Looks like this still needs work.

Incorporated those changes.

Not sure why, but some hunks got lost in my previous patch.

tagging

Great clean-up. Committed to CVS HEAD. Thanks.

Excellent! Thank you so much, Dries. Marking as Needs Documentation so we remember to add the upgrade docs.

It looks to me like this broke install.php. I found myself unable to do an install.php this morning and worked through yesterday's commits. This commit was the one where install.php ceases to work.

Attached is the screenshot of what happens to install.php. It just stops.

Installing head with jQuery is broken.

On a sqlite D7 (after applied #5 from #476010: Drupal 7+SQLite fails to install I get this as content for misc/jquery.once.js

Fatal error: Class 'SelectQueryExtender' not found in /home/clemens/htdocs/d7/d7/includes/pager.inc on line 16

Call Stack:
 0.0002 65308 1. {main}() /home/clemens/htdocs/d7/d7/index.php:0
 0.0052 413032 2. drupal_bootstrap() /home/clemens/htdocs/d7/d7/index.php:21
 0.0336 1477060 3. _drupal_bootstrap() /home/clemens/htdocs/d7/d7/includes/bootstrap.inc:1430
 0.0515 2408716 4. _drupal_bootstrap_full() /home/clemens/htdocs/d7/d7/includes/bootstrap.inc:1564
 0.0618 2936932 5. require_once('/home/clemens/htdocs/d7/d7/includes/pager.inc') /home/clemens/htdocs/d7/d7/includes/common.inc:3594


Fatal error: Class 'MergeQuery' not found in /home/clemens/htdocs/d7/d7/includes/database/database.inc on line 720

Call Stack:
 0.0002 65308 1. {main}() /home/clemens/htdocs/d7/d7/index.php:0
 0.0052 413032 2. drupal_bootstrap() /home/clemens/htdocs/d7/d7/index.php:21
 0.0336 1477060 3. _drupal_bootstrap() /home/clemens/htdocs/d7/d7/includes/bootstrap.inc:1430
 0.0515 2408716 4. _drupal_bootstrap_full() /home/clemens/htdocs/d7/d7/includes/bootstrap.inc:1564
 0.0618 2936932 5. require_once('/home/clemens/htdocs/d7/d7/includes/pager.inc') /home/clemens/htdocs/d7/d7/includes/common.inc:3594
 0.0633 2945564 6. _drupal_session_write() /home/clemens/htdocs/d7/d7/includes/session.inc:0
 0.0633 2945564 7. db_merge() /home/clemens/htdocs/d7/d7/includes/session.inc:132
 0.0633 2945564 8. DatabaseConnection->merge() /home/clemens/htdocs/d7/d7/includes/database/database.inc:1918

Hmmm ... jquery.once.js is not in HEAD

reverting and then applying the patch installs drupal

Please (re)apply the patch to HEAD

Is it only me or vertical tabs quit working ?

@yched - A lot of different Javascript stuff is broken with the latest commit from this issue.
As clements.tolboom mentionned, you can simply revert the patch in #59 and then reapply it and all should be fine. It worked for me.

It appear that the file misc/jquery.once.js was not added/committed to CVS even though the patch is creating it.

GET http://drupal.org/files/issues/drupal.jquery-once-58.patch | patch -R -f -p0
GET http://drupal.org/files/issues/drupal.jquery-once-58.patch | patch -f -p0

The only difference you should see after running these two commands is the creation of the file misc/jquery.once.js

I've attached a patch that only adds in the missing jquery.once.js file, which may be cleaner than re-applying the entire patch #59.

I committed this. thanks!

We forgot the upgrade documentations, so I added it: http://drupal.org/update/modules/6/7#jquery_once

please kindly note that in d7
$(xxx).once(function(){})

this usage seems not compatible in D8, I used $(this).once().click(function(){}) works.