This is a regression from the original AJAX system in CTools.

The throbber is placed on the item in 'beforeSubmit'. However, that before function only runs on form submissions. It does not run on normal $.ajax calls. For those, you need to use beforeSend, which is always called.

beforeSubmit is really only interesting if you intend to modify the form values, which the current beforeSubmit does not do. Therefore, we need to change the beforeSubmit we currently use to before Send. However, to maintain the easy ability to add functionality to beforeSend, we should retain an empty function.

Patch forthcoming.

CommentFileSizeAuthor
#11 969346-before-submit-should-be-before-send.patch2.86 KBmerlinofchaos
#10 drupal.ajax-options-throbber.10.patch3.49 KBsun
#1 969346-before-submit-should-be-before-send.patch2.24 KBmerlinofchaos
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
Status: Active » Needs review
FileSize
969346-before-submit-should-be-before-send.patch2.24 KB

Here is the aforementioned patch.

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented

subscribe

mfer’s picture

mfer CreditAttribution: mfer commented

Is there an callback in core or an example module that can be used to test this out with links?

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

Grab the example for my ajax presentation today off of my blog at http://angrydonuts.com -- that contains simple links that do stuff with the throbber.

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia commented

I fully agree with this. I'd RTBC, but it would be nice to get confirmation from someone that it works as expected.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

I ran my demo code with this patch on, plus did some normal AJAX stuff (like poll) just to test. I'm confident this is working well. If effulgentsia is in agreement maybe we should RTBC this?

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia commented
Status: Needs review » Reviewed & tested by the community
sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Reviewed & tested by the community » Needs review 
+++ misc/ajax.js	11 Nov 2010 22:56:35 -0000
@@ -37,6 +37,8 @@ Drupal.behaviors.AJAX = {
+      element_settings.progress = { 'type': 'throbber' };

Is 'type' a reserved token?

+++ misc/ajax.js	11 Nov 2010 22:56:35 -0000
@@ -241,9 +247,17 @@ Drupal.ajax.prototype.beforeSerialize = 
 /**
- * Handler for the form redirection submission.
+ * Modify form values prior to form submission.
  */
 Drupal.ajax.prototype.beforeSubmit = function (form_values, element, options) {
+  // This function is left empty to make it simple to override for modules
+  // that wish to add functionality here.
+}

I wonder whether we cannot find a way to expose all possible callback handlers on the ajax prototype... ideally, in a compact way (i.e., only defining empty default functions), and possibly, all of them being routed through a generic

Drupal.ajax.prototype.invoke = function (method, args)

+++ misc/ajax.js	11 Nov 2010 22:56:35 -0000
@@ -241,9 +247,17 @@ Drupal.ajax.prototype.beforeSerialize = 
+/**
+ * Prior to the AJAX request being sent, do anything we need to do.
+ */
+Drupal.ajax.prototype.beforeSend = function (xmlhttprequest, options) {

"we" shouldn't be used under normal circumstances, as it usually hides away actual meaning. Something along the lines of:

"Prepare the AJAX request before it is sent."

(or similar)

Powered by Dreditor.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

Is 'type' a reserved token?

Irrelevant to this patch. The progress bar is already in use with that token.

I wonder whether we cannot find a way to expose all possible callback handlers on the ajax prototype... ideally, in a compact way (i.e., only defining empty default functions), and possibly, all of them being routed through a generic

Would probably be best to do that through a separate patch. We're running real short on time here.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
FileSize
drupal.ajax-options-throbber.10.patch3.49 KB

Alright, so how about this?

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
FileSize
969346-before-submit-should-be-before-send.patch2.86 KB

Rerolled with only sun's js doc improvement.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Reviewed & tested by the community

Yes, let's do this.

#954804: All .js in /misc should be registered as a library is going to be a "required" follow-up (without hard dependency on this one, but manual testing revealed breaking JS, because progress.js is not loaded).

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Ok. Since this represents a regression from CTools, it's sort of in "now or never" mode. According to sun, this function rename shouldn't actually break anything, since CTools is the only project that uses it.

Committed to HEAD.

Status: Fixed » Closed (fixed)

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