Saturday, November 17, 2012

Interesting bug & how I squashed it



I was working on a Scrollview implementation using YUI3 library for my personal web site. The idea was to make a re-usable JS for travel slideshows. Basically, for Scrollview implementation, you take a list structure and then configure the swiping to work vertically or horizontally with some additional configurations.

I had the added complexity that I was building my DOM on the fly from a Flickr feed via YQL open tables. So, the object has a constructor function which creates an instance variable for the container where the Scrollview will plug in, an overlay container and elements within the overlay, which will be used to show a larger version of the images once the preview is clicked from the Scrollview slider.

There is a getPhotos method, which fetches the photo metadata from YQL using the Flickr photoset ID.  The getPhotos method then builds the DOM for the Scrollview container (a list) and then attaches some additional listeners to the container.

I noticed a bug where sometimes the whole set of photos was not scrollable in the slider. Each page reload would have the slider end in different spot - almost never including the entire set. I made the assumption that Scrollview was dynamically calculating the "width" or "height" of the scrollable area (right assumption), and that I had a timing bug (wrong assumption). Below is how the method started out:


getPhotos : function(e) {
  var t = this,
  handleJSONP = function(data) {
    var html = '<ul>';
    for (i=0, x=data.query.results.photo.length; i<x; i++) {
      html += '<li><a href="http://farm'+data.query.results.photo[i].farm+'.staticflickr.com/'+data.query.results.photo[i].server+'/'+data.query.results.photo[i].id+'_'+data.query.results.photo[i].secret+'_c.jpg" target="_blank"><img src="http://farm'+data.query.results.photo[i].farm+'.staticflickr.com/'+data.query.results.photo[i].server+'/'+data.query.results.photo[i].id+'_'+data.query.results.photo[i].secret+'_q.jpg" alt="'+data.query.results.photo[i].title+'"/></a></li>';
     }
    t.container.set("innerHTML", html + '</ul>');
    t.pluginScrollView();
    
  },
  url = "http://query.yahooapis.com/v1/public/yql?q=select%20*%20from%20flickr.photosets.photos(0)%20where%20(photoset_id%3D'72157631913782316'%20and%20api_key%3D'dd0a6e3d9ef751ea725c2c0b8ec81259')&format=json&diagnostics=true&callback={callback}";
  Y.jsonp(url, handleJSONP);
},

So, I first thought that the timing issue was that the DOM had not finished constructing before I called the pluginScrollView method. So, I modified the code to be as follows:

getPhotos : function(e) {
  var t = this,
  handleJSONP = function(data) {
    var html = '<ul>';
    for (i=0, x=data.query.results.photo.length; i<x; i++) {
      html += '<li><a href="http://farm'+data.query.results.photo[i].farm+'.staticflickr.com/'+data.query.results.photo[i].server+'/'+data.query.results.photo[i].id+'_'+data.query.results.photo[i].secret+'_c.jpg" target="_blank"><img src="http://farm'+data.query.results.photo[i].farm+'.staticflickr.com/'+data.query.results.photo[i].server+'/'+data.query.results.photo[i].id+'_'+data.query.results.photo[i].secret+'_q.jpg" alt="'+data.query.results.photo[i].title+'"/></a></li>';
     if (i === x-1) {
      t.container.set("innerHTML", html + '</ul>');
      t.pluginScrollView();
     }
    }
  },
  url = "http://query.yahooapis.com/v1/public/yql?q=select%20*%20from%20flickr.photosets.photos(0)%20where%20(photoset_id%3D'72157631913782316'%20and%20api_key%3D'dd0a6e3d9ef751ea725c2c0b8ec81259')&format=json&diagnostics=true&callback={callback}";
  Y.jsonp(url, handleJSONP);
},

This way, I would not call the pluginScrollView method until the last iteration through the photo data (ensuring the DOM would be complete before instantiating ScrollView on the container). But, the bug persisted. I finally realized when inspecting the DOM in Chrome and Firebug that the <li> elements containing the photos had no width. And, I had failed to set a width and height on the images in the method that built the DOM. So, it turns out that fixing either problem resolved the bug. 

html += '<li><a href="http://farm'+data.query.results.photo[i].farm+'.staticflickr.com/'+data.query.results.photo[i].server+'/'+data.query.results.photo[i].id+'_'+data.query.results.photo[i].secret+'_c.jpg" target="_blank"><img src="http://farm'+data.query.results.photo[i].farm+'.staticflickr.com/'+data.query.results.photo[i].server+'/'+data.query.results.photo[i].id+'_'+data.query.results.photo[i].secret+'_q.jpg" alt="'+data.query.results.photo[i].title+'" width="150" height="150"/></a></li>';

The above fixes the bug. Additionally, it was fixable in the CSS with the following change:

#scrollable li {
  display:inline-block;
  *display:inline;
  *zoom:1;
  padding:0 3px;
  width:150px;
}

Arguably, from a performance standpoint, the former fix is better. Likely both should be employed.  I hope others are able to learn from both my mistake and my bug fixing process. 

The result can be viewed here: 


No comments: