Ticket #5398 (closed bug: fixed)

Opened 5 years ago

Last modified 4 years ago

Remote-with-cache demo does not break if (cache.term == request.term) but executes another request

Reported by: thewolfram Owned by:
Priority: minor Milestone: 1.8.2
Component: ui.autocomplete Version: 1.8
Keywords: demo Cc:
Blocking: Blocked by:

Description

Small error in a demo: In case the new search term and the cached term are equal or the new term contains the cached term, the cached data is (partially) used but another GET is executed nevertheless. The $.ajax should go in an else block.

MacOSX, FF 3.6

Change History

comment:1 Changed 5 years ago by lambacck

The if condition is also probably needs tweaking. It currently says:

if (new RegExp(cache.term).test(request.term) && cache.content && cache.content.length < 13) {
    var matcher = new RegExp($.ui.autocomplete.escapeRegex(request.term), "i");
    response($.grep(cache.content, function(value) {
        return matcher.test(value.value)
    }));
}

It should be:

if (new RegExp($.ui.autocomplete.escapeRegex(cache.term)).test(request.term) && cache.content && cache.content.length < 13) {
    var matcher = new RegExp($.ui.autocomplete.escapeRegex(request.term), "i");
    response($.grep(cache.content, function(value) {
        return matcher.test(value.value);
    }));
    return;
}

Note that cache.term in the if condition is now wrapped in a call to $.ui.autocomplete.escapeRegex to match the matcher RegExp?. Also the original bug is fixed by adding a return to the if block.

Also matcher tests value.value, this is a deviation from how the list of object works in the autocomplete itself. It uses:

return matcher.test( value.label || value.value || value );

comment:2 Changed 5 years ago by lambacck

Oh yeah. The first if block can also get a return statement after the response call. We could also improve the demo by creating a make cached autocomplete function. Something like:

function caching_auto_complete(item, url, options) {
    var cache = {};
    options.source = function(request, response) {
        if (cache.term == request.term && cache.content) {
            response(cache.content);
            return;
        }
        if (new RegExp($.ui.autocomplete.escapeRegex(cache.term)).test(request.term) && 
                cache.content && cache.content.length < 13) {
            var matcher = new RegExp($.ui.autocomplete.escapeRegex(request.term), "i");
            response($.grep(cache.content, function(value) {
                // NOTE change to match core autocomplete logic
                return matcher.test(value.label || value.value || value)
            }));
            return;
        }
        $.ajax({
            url: url,
            dataType: "json",
            data: request,
            success: function(data) {
                cache.term = request.term;
                cache.content = data;
                response(data);
            }
        });
    }
    $(item).autocomplete(options);
    
}

caching_auto_complete('#birds', "search.php", {
            minLength: 2,
            select: function(event, ui) {
                log(ui.item ? ("Selected: " + ui.item.value + " aka " + ui.item.id) : 
                    "Nothing selected, input was " + this.value);
            }});

This is almost completely untested.

comment:3 Changed 5 years ago by scott.gonzalez

  • Status changed from new to closed
  • Resolution set to fixed
  • Milestone changed from TBD to 1.9

Fixed in  e087e7d.

comment:4 Changed 4 years ago by rdworth

  • Milestone changed from 1.9 to 1.8.2
Note: See TracTickets for help on using tickets.