Problem/Motivation

Right now to trigger an AJAX event ajax.js does

    // Bind the ajaxSubmit function to the element event.
    $(ajax.element).on(element_settings.event, function (event) {
      if (!drupalSettings.ajaxTrustedUrl[ajax.url] && !Drupal.url.isLocal(ajax.url)) {
        throw new Error(Drupal.t('The callback URL is not local and not trusted: !url', {'!url': ajax.url}));
      }
      return ajax.eventResponse(this, event);
    });

element_settings.event is usually "click" or "mousedown". But there is no namespacing, which means if you want to unbind the event you do it for all click events. Also the callback is an anonymous function so you can't target that either.

jQuery documentation says:

Remove just one previously bound handler by passing it as the third argument:

Unbind all delegated event handlers by their namespace:
<code>
var validate = function() {
  // Code to validate form entries
};
 
// Delegate events under the ".validator" namespace
$( "form" ).on( "click.validator", "button", validate );
 
$( "form" ).on( "keypress.validator", "input[type='text']", validate );
 
// Remove event handlers in the ".validator" namespace
$( "form" ).off( ".validator" );

Proposed resolution

I suggest we namespace the events with "drupalAjax" or any better name. IE:

    // Bind the ajaxSubmit function to the element event.
    element_settings.event = element_settings.event + '.drupalAjax';
    $(ajax.element).on(element_settings.event, function (event) {
      if (!drupalSettings.ajaxTrustedUrl[ajax.url] && !Drupal.url.isLocal(ajax.url)) {
        throw new Error(Drupal.t('The callback URL is not local and not trusted: !url', {'!url': ajax.url}));
      }
      return ajax.eventResponse(this, event);
    });

Comments

tic2000 created an issue. See original summary.

nod_’s picture

Agreed this is a good idea.

tic2000’s picture

tic2000’s picture

tic2000’s picture

Status: Active » Needs review
tic2000’s picture

phenaproxima’s picture

I think this patch makes sense, and on a technical level it's RTBC from me. However, I suspect it constitutes an API change, since it potentially alters the way other JavaScript code interacts with the AJAX system. Marking for 8.1.x and requesting review from the higher-ups.

tic2000’s picture

alters the way other JavaScript code interacts with the AJAX system

If by this you mean that now it can be done also in a different way, you are right. If you mean that it changes the old way, then no. The old way works too.

The way namespacing works is:

$(selector).on('click', doSomething);
// You can unbind that, but you will also unbind all other click events
$(selector).off('click');


$(selector).on('click.namespace', doSomething);
// You can unbind that, but you will also unbind all other click events
$(selector).off('click');

// You can unbind that, and all other events with the same namespace on the selector
$(selector).off('.namespace');

// Or you can unbind only this exact event you bound
$(selector).off('click.namespace');
lokapujya’s picture

Why are there not any places being updated where the namespace is now being used in an unbind?

droplet’s picture

Issue tags: +API addition

Should we add `drupal` into our namespace in all new API addition.

tic2000’s picture

Why are there not any places being updated where the namespace is now being used in an unbind?

Because no code in core unbinds ajax behavior. There are uses of .off(), but most of them unbind a namespaced event, or unbind an event which is not namespaced, but none was bound by Drupal.Ajax.

Should we add `drupal` into our namespace in all new API addition.

In core some use the module name, but most don't. Node preview does ".preview", dialog.js does ".dialog". None of the namespaced events in core are using "drupal". That's for a discussion in the coding standards.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

sardara’s picture

+1, I also support the idea of namespacing.

nod_’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs subsystem maintainer review

I'd also go with .ajax event namespace are less likely to cause issues than data attributes. Not sure it's an API addition though.

Patch still applies so RTBC :p

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 4: ajax-4-do-not-test.patch, failed testing.