Saturday, June 16, 2012

Weird jQuery bug

My software was recently upgraded from jQuery 1.5.2 to 1.7.2.

Luckily we had a bunch of automated tests for our ui code and we found a lot of issues with our code that toggles checkboxes or the disabled state of a tag.  Previously you could say
$('#my_checkbox').attr('checked', 'checked'); // check the checkbox
$('#my_checkbox').attr('checked', ''); // uncheck the checkbox

$('#my_input').attr('disabled', 'disabled'); // disable the input
$('#my_input').attr('disabled', ''); // enable the input
We had code all over the place that did this. But when we used empty string, attr didn't enable or uncheck the elements.

The fixes we found were to either use a boolean value instead of a string, or you could use the removeAttr function:
$('#my_checkbox').attr('checked', true); // check the checkbox
$('#my_checkbox').attr('checked', false); // uncheck the checkbox
$('#my_checkbox').removeAttr('checked'); // alternative to uncheck the checkbox

$('#my_input').attr('disabled', true); // disable the input
$('#my_input').attr('disabled', false); // enable the input
$('#my_input').removeAttr('disabled'); // alternative to enable the input
We needed to figure out the best way to change our code with little impact and it looked like using removeAttr would cause more code since you would have to litter your code with if/else statements to figure out whether to use the attr or the removeAttr case.

So because of this we tried to enforce using booleans. But you could run into issues where you didn't know the type of the second parameter to attr.
var checked = $('#my_checkbox').attr('checked'); // get the checkbox state:
//   'checked' or ''
//...
$('#other_checkbox').attr('checked', checked); // set another checkbox to the same state
// same thing appplies to disabled
I did some digging and found that jQuery 1.6.2 had a fix to change attr('checked') to return the actual html value of checked instead of a boolean value:
<input id="my_checkbox" type="checkbox" checked="checked" />
...
var checkState = $('#my_checkbox').attr('checked'); // returns 'checked' as of jQuery 1.6.2 instead of true.  When it is unchecked, you get undefined returned.
// See this link for a list of more quirks: 
//     http://blog.jquery.com/2011/06/30/jquery-162-released/#comment-526605
So, to remedy this, we decided to always ensure you pass a boolean to the attr function for disabled or checked. Don't use empty string to uncheck or enable your element. If you still want to rely on the return string of the attr function, you can booleanize it like this:

var check_state = $('#my_checkbox').attr('checked'); // get the checkbox state:
//    'checked' or undefined
//...
$('#other_checkbox').attr('checked', !!check_state); // set another checkbox to the same state using a boolean
// !!undefined yields false, 
// !!'checked' yields true
// same thing applies to disabled
Now you know, and knowing is half the battle.

<rant>
I have to say that I'm pretty unimpressed with jQuery since no one caught this.  Changing apis that are so important, like attr, is bad mojo and will cause many developers to curse your name late at night as they debug these unexpected api upgrade bugs.  jQuery is supposed to be so awesome because it is easy going and does what you expect.  This is a step backwards for all of us.
</ rant>

1 comment:

  1. I'm not saying jQuery has a bug, I'm just saying that my software had a bug because of jQuery.

    ReplyDelete