Problem/Motivation

With version 1.x you where able to define a menu item like js/js_example/autocomplete-example and register a JS callback that worked on the same path.
If JS was enabled it picket up the request, otherwise it simply triggered Drupal core.
With 2.x we unfortunately lost this ability.
Problems are:

  1. Automated handling of the autocomplete ajax requests. We need to set the JS request parameters.
  2. Completely changed arguments handling in the JS callback.

Proposed resolution

  1. If an element has set #autocomplete_path and #js_callback adjust the #autocomplete_path to make it recognizable in jQueries $(document).ajaxSend().
    I tried a lot to intercept autocomplete ajax calls but unfortunately the approach in the attached patch seems to be the most stable. Unfortunately the core autocomplete doesn't expose a thing we could hook into.
  2. JS 1.x handled callback arguments like menu does with page arguments, I suggest to do the same if process request is set to FALSE and callback arguments are defined.

The attached patch also updates the example module to show how to seamlessly integrate autocomplete callbacks with JS.

Remaining tasks

Reviews needed.

User interface changes

None.

API changes

New behaviour if process request is set to FALSE and callback arguments are defined.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

markhalliwell’s picture

Status: Needs review » Needs work

Actually we can intercept the AJAX request via the /js.js file. There's already a beforeSend callback that is invoked via a global AJAX event.

We could detect if #autocomplete_path is set (should just be set to js, IMO) and then set an additional attribute data-autocomplete="true" or whatever. In the beforeSend callback, we can check if (options.$trigger.is('[data-autocomplete]')). If it has that attribute, then we can change the method of the AJAX request to POST (a GET JS module request fully bootstraps Drupal) and add in the data-js-module, data-js-callback and data-js-token values to the data being sent, which thus invokes a JS module AJAX request.

I'm not entirely sure if what I said above is 100% accurate, but I think it's more or less in the right ballpark of what needs to be accomplished.

das-peter’s picture

I tried it with $(document).ajaxSend, below to what kind of mess it became finally:

(function (window, $, undefined) {
  /**
   * Drupal autocomplete integration..
   */
  $(document)
    .ajaxSend(function(event, jqXHR, options) {
      // Search if an autocomplete field with the related base url exists.
      var autocomplete_element = false;
      jQuery('input.autocomplete').each(function(i){
        if (options.url.search(this.value) != 1) {
          autocomplete_element = this;
        }
      });
      // If there's a possible autocomplete field...
      // And it doesn't already contain js callback information.
      if ( autocomplete_element != false
        && options.type == 'GET'
        && typeof event.currentTarget != 'undefined'
        && typeof event.currentTarget.activeElement  != 'undefined'
        && $(event.currentTarget.activeElement).hasClass('form-autocomplete')
        && $(event.currentTarget.activeElement).attr('data-js-module')
        && (typeof options.data == 'undefined' || options.data.search(/js_module/) == -1)
        && options.url.search(/js_module/) == -1
      ) {
        var el = $(event.currentTarget.activeElement);
        // Add the js callback data to the request.
        options.data = options.data + '&' || '';
        var js_data = jQuery.param({
          js_module: el.attr('data-js-module'),
          js_callback: el.attr('data-js-callback'),
          js_token: el.attr('data-js-token')
        }, options.traditional );
        // Manipulate the data and the url parameter of the ajax call.
        options.data += js_data;
        options.url += ((options.url.search(/\?/) == -1) ? '?' : '') + js_data;
      }
    });
})(window, window.jQuery);

This was tested not with the "happy life" example in the JS example module but with a form that also has ajax validators, custom ajax handlers and so on. So I ended up with this huge pile of conditions.
Btw. this is an intermediate state in which I tried to replace event.currentTarget.activeElement by autocomplete_element. I scratched that and went with the approach in the patch instead.
If however options.$trigger is more reliable than the approaches I tried I'd happily go with it.
But, I couldn't figure out how options.$trigger could help, as far as I see that variable is set in locations where we've full control / knowledge over what happens - contrary to Drupal.ACDB / Drupal.jsAC which exposes almost nothing.
But, I'd like to learn how it could help if it brings us a better solution :)

FYI: I also gave overwriting Drupal.ACDB.prototype.search a try but since other modules do that already (*evil look at search api autocomplete*) I scratched that too.

I choose the current approach because:

  • We've a proper unique identifier - no huge pile of "hmm maybe it's a autocomplete request that was messed with" conditions.
  • If someone wants to mess with the ajax request too that's not an issue as long as our identifier is untouched. No matter if before or after our handling.
markhalliwell’s picture

Assigned: Unassigned » markhalliwell

Ok, I will have to take a look then. I may be missing something... but it really shouldn't need these kind of verbose conditions as there are only two that really need to be checked: if (element.is('[data-js-module][data-autocomplete]')) as we would only add these attributes if both #js_callback and #autocomplete_path are set on the form element (then we'd know it's ours to mess with).

das-peter’s picture

IRC Log:
[18:01] markcarver: Awesome! Hmm, but the problem I had was that the element that has autocompletion als can trigger other ajax request (validation) so you end up checking what kind of ajax request is running right now. What leaves you with the url - which is basically the approach I've used now.

markhalliwell’s picture

Just realized my suggestion in #1 about simply setting the #autocomplete_path to just js won't work. This then requires a module to explicitly define this one as a dependency.

In order to keep this module optional, we'd also likely need to lookup the menu item associated with the #autocomplete_path at some point in the request to extract the components like access callback/arguments if set. Not entirely sure how that might work yet, but just making a note for when I actually get around to looking at this in more detail.

das-peter’s picture

I think it is sensible to require developers to add '#js_callback' => array('js_example' => 'autocomplete_example'), to the form element they have already '#autocomplete_path' on.
As I'm not so optimistic that we can provide a generic approach for the currently required implementation of hook_js_info() based on the '#autocomplete_path' / it's menu item. We hardly can figure out the dependencies (includes, modules, bootstrap level) of the autocomplete callback - which means we would have to use a full bootstrap, basically reversing the performance gain :)

So in my opinion all we should ensure is that other modules can integrate with JS without declaring a hard dependency on it and with as few conditions (if (module_exists('js')) {) as possible.

das-peter’s picture

FileSize
5.77 KB

Found an issue with the existing patch.
If no token is required we don't have a proper handle to identify the autocomplete request.
If token is missing another token is generated to have an identifier.

das-peter’s picture

Status: Needs work » Needs review
FileSize
6.07 KB
1.15 KB

Updated the existing patch so that it works with the changes made related to SA-CORE-2015-003: Cross-site Scripting - Autocomplete system - Drupal 6 and 7
Set it to needs review for some attention ;)

markhalliwell’s picture

Status: Needs review » Needs work
Related issues: +#2485667: Properly handle anonymous user token check.

I'm not yet entirely sure what the best way to handle this is TBH.

That being said, it also includes the patch from this related issue which essentially nullifies the purpose of a token, so definitely setting back to CNW.

markhalliwell’s picture

Title: [Regression] Seamlessly support core autocomplete » Support core #autocomplete_path
Assigned: markhalliwell » Unassigned
Category: Bug report » Feature request

I've actually got this working quite well with the patch I'm about to commit.

Replace all MODULENAME instances with the machine name of the module implementing this support:


/**
 * Implements hook_menu().
 */
function MODULENAME_menu() {
  // Provide a non JS Callback fallback menu entry.
  $items['my_module/autocomplete'] = array(
    'title' => 'My Module Autocomplete',
    'page callback' => 'MODULE_js_callback_autocomplete',
    'access callback' => 'user_access',
    'access arguments' => array('access content'),
    'type' => MENU_CALLBACK,
    // Most autocompletes invoke drupal_json_output() in their callback.
    // However, to get this working with both core and JS callback, the
    // return value from the callback needs to be the same for both.
    'delivery callback' => 'drupal_json_output',
  );
  return $items;
}

/**
 * Implements hook_js_info().
 */
function MODULENAME_js_info() {
  $info['autocomplete'] = array(
    // Optional, but highly recommended since tokens are disabled.
    'access callback' => 'user_access',
    'access arguments' => array('access content'),
    'bootstrap' => DRUPAL_BOOTSTRAP_SESSION,

    // Bypass the JS Callback module's normal delivery callback. This is
    // important so it works with what core autocomplete.js expects.
    'delivery callback' => 'drupal_json_output',

    // Add dependencies (depends what is used to generate the autocomplete).
    'dependencies' => array('user'),

    // Core autocomplete.js uses GET, not POST.
    'methods' => array('GET'),

    // Disable token checks since this is just a simple DB query
    // and it will likely be needed for anonymous users too.
    'token' => FALSE,

  );
  return $info;
}

/**
 * Implements MODULE_js_callback_CALLBACK().
 */
function MODULENAME_js_callback_autocomplete() {
  $json = array();

  // Core autocomplete.js appends the query string to the end of the
  // menu hook entry path above. So, in this case, the query string
  // should be the last argument, 2:
  // $_GET['q'] = array(
  //   0 => 'my_module',
  //   1 => 'autocomplete',
  //   2 => 'abc' // <-- query string
  // );
  if ($query = arg(2)) {
    // Do your querying code here with the query string the user provided.
    // The following example is from user_autocomplete().
    $result = db_select('users')->fields('users', array('name'))->condition('name', db_like($query) . '%', 'LIKE')->range(0, 10)->execute();
    foreach ($result as $user) {
      $json[$user->name] = check_plain($user->name);
    }
  }

  // Just return the JSON array, do not print it out.
  return $json;
}

Then, in some form somewhere, add the following to your textfield:

$form['text'] = array(
  '#type' => 'textfield',

  // Core fallback path.
  '#autocomplete_path' => 'my_module/autocomplete',

  // Enabled JS Callback support.
  '#js_callback' => array('MODULENAME' => 'autocomplete'),
);

  • markcarver committed 0854b60 on 7.x-2.x
    Issue #2419961 by das-peter, markcarver: Support core #autocomplete_path
    
markhalliwell’s picture

Status: Needs work » Fixed
FileSize
3.44 KB

Status: Fixed » Closed (fixed)

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