Integer interpreted as string causing fnUpdate() error

Netson

Hi,

as I was trying to update rows in a datatable today, I found out that the first update usually works, however, successive updates throw an error message: oData is undefined (line 6008 in the unminified datatables 1.8.2 javascript file).

After some testing with firefox/firebug, I noticed that the variables iStart and iEnd were of type String instead of type Integer. This explains why the function works the first time ("0"+1 is considered 1, but "1"+1 will end up as "11"), but not consecutive times.

The fix I used was fairly easy; change the following lines (6002 & 6003 in jquery.dataTables.js version 1.8.2) from:

[code]
/* Allow the collection to be limited to just one row */
if ( typeof iIndividualRow != 'undefined' )
{
iStart = iIndividualRow;
iEnd = iIndividualRow+1;
}
[/code]

to this:

[code]
/* Allow the collection to be limited to just one row */
if ( typeof iIndividualRow != 'undefined' )
{
iStart = Number(iIndividualRow);
iEnd = Number(iIndividualRow)+1;
}
[/code]

By typecasting these variables to Integer, the problem is solved and speed is actually improved a little as well! :)

I don't have an easy code extract to post with it, but I think the fix speaks for itself...
Any remarks, questions, etc are welcome :)

edit: I am using Firefox 9.0.1 with Firebug 1.8.4 on Windows 7 x64 with JQuery 1.7.1 and datatables 1.8.2

allan

The _fnGetTdNodes function expects an integer to be given, not anything else - so I guess the question is, what was passing in a string, as it is likely there is a bug there, rather than in _fnGetTdNodes.

Do you know where the call was coming from that was passing in a string? Was your second parameter being passed to fnUpdate a string? It expects either an integer or an element node.

Thanks,
Allan

Netson

Hey Allan,

it seems that the string gets initialized here (row 1941):

[code]
this.fnUpdate = function( mData, mRow, iColumn, bRedraw, bAction )
{
var oSettings = _fnSettingsFromNode( this[_oExt.iApiIndex] );
var iVisibleColumn, i, iLen, sDisplay;
var iRow = (typeof mRow == 'object') ?
_fnNodeToDataIndex(oSettings, mRow) : mRow;
[/code]

The last line (where iRow is set) sets iRow to string; in my case it is not an object so it simply takes the mRow value, which is already of type string when entering the function.

A bit more testing revealed that I was actually passing in a string myself when calling the function fnUpdate. I use the function .fnGetPosition() to fetch the row ID, but instead of directly inputting that into the fnUpdate function, I store it in a hidden form field and retrieve that later when actually calling fnUpdate().

This storing in a hidden form field likely causes the conversion to string.

So even though this seems to be a user error, it would be nice if the datatables code would either throw an error when the second parameter passed to fnUpdate is not an int or an object, or if it would simply convert any non-object to integer.

So my (altered) suggestion would be the following:

[code]
var iRow = (typeof mRow == 'object') ?
_fnNodeToDataIndex(oSettings, mRow) : Number(mRow);
[/code]

Hope that makes sense! :)

allan

Super - thanks for the follow up and confirming that the string was being passed in. The only thing I would say about your corrected code for fnUpdate is that you don't want to cast mRow to a number if it was found to be an object - other than that, it looks like a good idea!

Before I add it to DataTables I'm going to think about it a little, simply because I'm wondering where to draw the line in terms of what is accepted for API parameters and validation. In this case and possibly a few others it is trivial to do the type conversion, but should other parameters be checked as well for their type. Generally I've said that what is documented is supported (expected), other inputs would not be valid, and I would rather not add a lot of weight to the size of the code base doing validation - hence my concern about adding in what appears to be a trivial change :-).

Regards,
Allan

Netson

You're right, I was a little over enthousiastic in typecasting the mRow variable! :)

Although I understand your concern, basic type checking makes sense from my point of view. Even though it was my own fault, it took me several hours to figure out what I had done wrong. Some basic type checking would have made my life a lot easier and I think a lot of people would have given up instead of using a perfectly great plugin! :)

Thanks for the feedback; now I have to worry about my next issue: updating the entire table contents upon changing a select menu on the same page... :)

(I edited my suggestion according to your remark)