jQuery Search Suggestion Generator

Question

I'm currently using a PHP shopping cart, and finally decided to implement a search suggestion feature.

There are some already available solutions out there, but I wanted to gain control over how mine is implemented and how it functions.

I'm asking for some assistance here and code review on two things:

1. Can anyone spot any security flaws with my implementation (PHP security is already dealt with I'm explain later)

2. I've thrown in two variables, into the JavaScript, recurringRequest and request. These two variables perform the following task:

   1. The request variable initiates the request based on the characters that a user has typed into the search input box.
   2. The recurringRequest variable stores a boolean, true or false, and is used to check if the user is still typing, and if they are, then it aborts the request variable and stops it from processing past requests, so that we can facilitate minimal server load.

Am I doing it correctly?

My PHP page, searchsuggestion.php is a simply copy/paste of the original shopping cart search page code, with the exception that it removes all the other content (header, footer, everything is gone). All this page does is takes the get variable "keyword" value - and performs a search of the database for products matching the keyword.

I've used htmlentities on the returned results, so that it returns it in plain text. The reason I did this was because I want to show images in the search suggestions, but I don't want these images being loaded by the jQuery. Which is why in the jQuery code, you will see the replace command converting the tags back to html entities [(t.replace('&lt','<').replace('&gt', '>'));]. This provides near-instant results, and loads the images after the search suggestion list has already been populated, and slid down, so that the user can review search results instantly while typing instead of the lag that it produced when I initially had it loading the images from the PHP page via the jQuery.

Here is the full JavaScript:

var recurringRequest = false; // var to check if user is still typing
var request; // var to store request to abort if user is still typing

$( document ).ready(function() {   

// this function will unhide the search suggestion list
// when a user focuses on the search input box
$("input[name='keyword']").focus(function() {
$("#ssugg").slideToggle("fast");
});



// this function starts generating search suggestions when
// the user types into the search input box
$("input[name='keyword']").keyup(function() {

var value=$.trim($("input[name='keyword']").val());

if(value.length>1) // only pay attention to two characters or more
{
value = value.split(' ').join('+'); // replace spaces with + signs


if (recurringRequest) {
request.abort(); // abort a the previous request if the user is still typing
}

recurringRequest = true;

// this block of code performs the request, adds the results which are
// in htmlentities form to an always-hidden div, then converts it and 
// adds it to the slide down search suggestion list, and then empties
// out the always-hidden div

request =  $( "#ssugghidden" ).load( "searchsuggestion.php&keyword="+value+
" #searchsuggestlist", function() {
var t = $( "#ssugghidden" ).text();
$( "#ssugg" ).html(t.replace('&lt','<').replace('&gt', '>'));
$( "#ssugghidden" ).empty();
});


// finally set this to false because the request was allowed to fully complete
recurringRequest = false;

}

else
{
$( "#ssugg" ).empty(); // empty the div if less 2 characters were typed
}

});


// this simply hides the search suggestion list if the user focuses away from the search input box
$("input[name='keyword']").blur(function() {
// hide drop down suggestions
$("#ssugg").slideToggle("fast");
});

});

The request variable and recurringRequest variable was something I saw in another already coded plugin. The plugin didn't explain that it would reduce server load by aborting a previous request that was still being processed, but it did specify that it aborts the request should the user be still typing. It did mention that it was mostly if the user was typing very quickly, so that it is likely unneeded, and won't help much, but I'd like to ensure maximum performance always.

Answer 1

First off, your indentation could need some repairs. Whether tabs or spaces, 2 or 4 chars, doesn't matter as long as it is consistent.

var recurringRequest = false; // var to check if user is still typing
var request; // var to store request to abort if user is still typing

They're globals, and nobody wants globals. They're easily visible, modifiable, clobber-able, all the reasons why we avoid globals. An easy way to avoid it is to define them in your $(document).ready(...) handler.

request =  $( "#ssugghidden" ).load( "searchsuggestion.php&keyword="+value+
" #searchsuggestlist", function() {
var t = $( "#ssugghidden" ).text();
$( "#ssugg" ).html(t.replace('&lt','<').replace('&gt', '>'));
$( "#ssugghidden" ).empty();
});

A few things.

First, consider using $.get instead of jquery.load. jquery.load is a shorthand for a GET request that loads up the selected elements with the result. But in your case, you're doing some post-processing. You wouldn't want jquery.load to render something only to have it take it back.

Another benefit of $.get is that you can provide your params as an object (the second argument) instead of you having to manually construct the query string. jQuery will automatically convert it to a proper query string.

Third, you appear to return unprepared HTML as your response. Consider formatting it in advance in PHP so you avoid this post-processing in JS. Alternatively, you can return JSON and have the JS reconstruct the HTML. There are templating libraries like Mustache that take in a template and an object, and renders HTML for you.

// finally set this to false because the request was allowed to fully complete
recurringRequest = false;

Just so you know, jquery.load is asynchronous. Async operations are not "complete" the line after you call them. They run in parallel to the current thread. The callback is there for the engine to run once it's complete and when it has a chance to run it. In contrast, PHP's file_get_contents is synchronous. After the call, it's done.

To remedy this, put recurringRequest = false; inside the callback function.

In general, this is better suited as a jQuery plugin. Here's how I would have done it:

// Usage
$('.selector').autocomplete({
  url: 'searchsuggestion.php',
  lengthBeforeRequest: 4
});


// Plugin definition

;(function($){

  var defaultOptions = {
    url: '',
    lengthBeforeRequest: 2
  };

  // $.fn is just a fancy shorthand for jQuery.prototype.
  $.fn.autocomplete = function(userOptions){
    // in here, `this` is $('.selector');
    return this.each(function(){

      // shorthand reference to the current element.
      var element = $(this);

      // if autocomplete is already attached, do nothing.
      if(element.data('myautocomplete')) return;

      // mark the element as having autocomplete
      element.data('myautocomplete', true);

      // the suggestion list, dynamically created
      var suggestionList = $('<div/>').insertAfter(element);

      // options for this autocomplete
      var options = $.extend({}, defaultOptions, userOptions);

      // holding value for the current request
      var currentRequest = null;

      // toggle on either focus or blur
      element.on('focus blur', function(){
        suggestionList.slideToggle("fast")
      });

      element.on('keyup', function(){
        // do nothing if less than length
        var inputValue = element.val();
        if(inputValue.length < options.lengthBeforeRequest) return;

        // abort the current request
        if(currentRequest) currentRequest.abort();

        var newRequest = $.get(options.url, { keyword: inputValue });

        // Wait for the request to finish to add the result
        newRequest.then(function(result){
          suggestionList.html(result);
        });

        // Store a reference to the request
        currentRequest = newRequest;

      });
    });
  };
}(jQuery));