Skip to content
Permalink
Browse files

Autocomplete: Don't handle remote data if it's not the most recent re…

…quest. Fixes #5982 - Autocomplete: Race condition causes incorrect suggestions.
  • Loading branch information...
scottgonzalez committed Aug 24, 2010
1 parent 1cca969 commit f115b48d2bd79aff1f65fb895d1ebc9517d82edc
Showing with 7 additions and 2 deletions.
  1. +7 −2 ui/jquery.ui.autocomplete.js
@@ -210,7 +210,8 @@ $.widget( "ui.autocomplete", {
},

_initSource: function() {
var array,
var self = this,
array,
url;
if ( $.isArray(this.options.source) ) {
array = this.options.source;
@@ -220,7 +221,11 @@ $.widget( "ui.autocomplete", {
} else if ( typeof this.options.source === "string" ) {
url = this.options.source;
this.source = function( request, response ) {
$.getJSON( url, request, response );
self.xhr = $.getJSON( url, request, function( data, status, xhr ) {
if ( xhr === self.xhr ) {
response.apply( this, arguments );
}
});
};
} else {
this.source = this.options.source;

5 comments on commit f115b48

@jzaefferer

This comment has been minimized.

Copy link
Member

replied Aug 24, 2010

An addition or instead of the check within the success-callback, we could abort any previous still-running requests (self.xhr.abort()). That prevents the callback from getting called, and could have positive effects on performance.

@scottgonzalez

This comment has been minimized.

Copy link
Member Author

replied Aug 25, 2010

There's a bug where calling xhr.abort() triggers the success callback, so we should probably add the xhr.abort() call but leave the check.

@badsyntax

This comment has been minimized.

Copy link

replied Aug 25, 2010

Perhaps I'm speaking out of context here, if so please ignore my comment, but I don't believe xhr.abort() executing the success function is a bug, I believe that is the default behavior. I /think/ this change was added in 1.4.2. A simple test: http://badsyntax.co.uk/tests/jquery-xhr-abort.html and perhaps a bit of insight on why this is the default behavior. http://groups.google.com/group/jquery-dev/browse_thread/thread/3d8f7ac78c9b0117?pli=1

@scottgonzalez

This comment has been minimized.

Copy link
Member Author

replied Aug 25, 2010

Thanks for the background, but it seems like there's still some debate about what the expected behavior should be. There's still an open ticket for this: http://dev.jquery.com/ticket/6060 (and there have been more that were closed duplicates). In any case, I've added the abort call and kept this check in da2be6a.

@badsyntax

This comment has been minimized.

Copy link

replied Aug 25, 2010

I had the impression this behavior wasn't up for debate, certainly I'd prefer it if success wasn't executed on abort. Thanks for the link, keeping an eye on this.

Please sign in to comment.
You can’t perform that action at this time.