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.
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:
- Automated handling of the autocomplete ajax requests. We need to set the JS request parameters.
- Completely changed arguments handling in the JS callback.
Proposed resolution
- 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. - JS 1.x handled callback arguments like menu does with
page arguments
, I suggest to do the same ifprocess request
is set to FALSE andcallback 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.
Comment | File | Size | Author |
---|---|---|---|
#12 | support_core-2419961-12.patch | 3.44 KB | markhalliwell |
Comments
Comment #1
markhalliwellActually 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 tojs
, IMO) and then set an additional attributedata-autocomplete="true"
or whatever. In the beforeSend callback, we can checkif (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 thedata-js-module
,data-js-callback
anddata-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.
Comment #2
das-peter CreditAttribution: das-peter commentedI tried it with
$(document).ajaxSend
, below to what kind of mess it became finally: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
byautocomplete_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 toDrupal.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:
Comment #3
markhalliwellOk, 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).Comment #4
das-peter CreditAttribution: das-peter commentedIRC 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.
Comment #5
markhalliwellJust realized my suggestion in #1 about simply setting the
#autocomplete_path
to justjs
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 likeaccess 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.Comment #6
das-peter CreditAttribution: das-peter commentedI 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.Comment #7
das-peter CreditAttribution: das-peter commentedFound 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.
Comment #8
das-peter CreditAttribution: das-peter commentedUpdated 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 ;)
Comment #9
markhalliwellI'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.
Comment #10
markhalliwellI'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:Then, in some form somewhere, add the following to your textfield:
Comment #12
markhalliwell