_fnGetCellData doesn't test for column existence

_fnGetCellData doesn't test for column existence

sebastienbarresebastienbarre Posts: 24Questions: 1Answers: 0
edited February 2013 in Bug reports
Hi. Could _fnGetCellData test if a column exists before getting its settings?
The issue is here:
[code]
function _fnGetCellData( oSettings, iRow, iCol, sSpecific )
{
var sData;
var oCol = oSettings.aoColumns[iCol];
var oData = oSettings.aoData[iRow]._aData;
if ( (sData=oCol.fnGetData( oData, sSpecific )) === undefined )
[/code]
This last line is a Javascript error when oCol is undefined, which happens when oSettings.aoColumns[iCol] doesn't exist.

Putting a breakpoint here and looking at the stack trace, it seems to happen when _fnFilterColumn is called but the column the table was filtered on is not in the table anymore (by design, we decided to remove it). The sorting was serialized with the table state and then restored, but the column isn't there anymore.
[code]
function _fnFilterColumn ( oSettings, sInput, iColumn, bRegex, bSmart, bCaseInsensitive )
{
if ( sInput === "" )
{
return;
}
var iIndexCorrector = 0;
var rpSearch = _fnFilterCreateSearch( sInput, bRegex, bSmart, bCaseInsensitive );
for ( var i=oSettings.aiDisplay.length-1 ; i>=0 ; i-- )
{
var sData = _fnDataToSearch( _fnGetCellData( oSettings, oSettings.aiDisplay[i], iColumn, 'filter' ),
[/code]
That last line calls fnGetCellData.
Would you advise a workaround?
Thanks

Replies

  • allanallan Posts: 61,438Questions: 1Answers: 10,052 Site admin
    This is an interesting one - you note that you were filtering on a specific column, but then removed that column but kept the filtering. This results in a Javascript error I presume? I think this is actually correct, since anything else could result in an undefined action - what should happen when you try to filter on column 10 in a 5 column table?

    Having said that, I can absolutely see that this is a bit of a pain! DataTables 1.10 is going to change the API so you'd do something like `table.column( '{column-selector}' ).filter( '{filter}' )` with the column selector being the column name or column index, and allowing multiple columns to be selected and therefore filtered at the same time. This last part is important since if the selector matches no columns, there is nothing to filter, and no filter is applied.

    So yes, something like what you are looking for will be implemented in 1.10, but for the new API rather than the old one.

    Thanks for the feedback!

    Allan
  • sebastienbarresebastienbarre Posts: 24Questions: 1Answers: 0
    [quote]you note that you were filtering on a specific column, but then removed that column but kept the filtering. This results in a Javascript error I presume?
    [/quote]
    Yes, which impacts the rest of the page, and prevents the table from rendering correctly.
    The issue here is that I've no control over that. The filtering settings is saved automatically when the state of the table is saved (though fnStateSave), which is a feature we like. So if we have somebody in the company filtering on a column and suddenly we (the developers) remove that column from the table, the whole table stops rendering correctly for this guy, and there isn't much we can do. That's why I was suggesting you put an extra safeguard here?
  • allanallan Posts: 61,438Questions: 1Answers: 10,052 Site admin
    The problem is that the same could apply to any input and if I were to check every input parameter for validity, it would add considerable weight to the core library. I feel that altering the table but not the logic being applied, there isn't much DataTables can do about that - say for example a new column was added at the start of the table, the filtering indexes would be offset by one. Nothing DataTables can do about that.

    I think the new API with column naming for the column selectors will help, since unknown names will be slightly ignored, but unknown indexes will likely still throw an error.

    Allan
  • sebastienbarresebastienbarre Posts: 24Questions: 1Answers: 0
    Yes, we are careful about adding columns. Maybe I didn't phrase my problem correctly: unless I missed something, there is nothing I can do here, no logic to apply. We are not doing custom filtering here, there is nothing in our Javascript that makes explicit reference to the column we removed. It's the built-in filtering in Datatable that, when restored, tries to access a column index that is not there anymore, a setting that is buried in the state of the table. So I don't know how or where to intercept that problem.
  • allanallan Posts: 61,438Questions: 1Answers: 10,052 Site admin
    Sorry - I think I've misunderstood. So the filtering that is applied is a column filter, but it is coming from the cookie? If so that most certainly is a bug. It should be throwing the old state away if there is a difference in the number of columns, since all bets are off for the difference between the tables at that point.

    Allan
  • sebastienbarresebastienbarre Posts: 24Questions: 1Answers: 0
    Yes, that's what is happening. Note that I also use the ColVis plugin in that table, and it serializes its own state to the table state if I'm not mistaken (a good thing), so the problem might be there too. Either way, this is all happening as soon as we invoke Datatable and it deserializes its state, that's why we can't control what's going on. Thanks
  • allanallan Posts: 61,438Questions: 1Answers: 10,052 Site admin
    Got it - thanks for pointing this out and sorry for my misunderstanding the issue. Unfortunately the fix isn't trivial, but I will look into how best to fix it and update here.

    Allan
  • sebastienbarresebastienbarre Posts: 24Questions: 1Answers: 0
    Thanks. I just got burned by this one again today, was wondering if you had an update. No rush, just curious. Thanks.
  • allanallan Posts: 61,438Questions: 1Answers: 10,052 Site admin
    Not yet - sorry. Haven't had a chance to look at this yet. It is on my to-do list though :-)

    Allan
  • sebastienbarresebastienbarre Posts: 24Questions: 1Answers: 0
    This just bit me really hard again, with a lot of people in the company not being able to see any data in a table because we had removed a column. And it's not easy to explain them in details how to clean their cookies or HTML local storage, across different browsers. Any quick workaround I could apply?
  • allanallan Posts: 61,438Questions: 1Answers: 10,052 Site admin
    I'm afraid there are no quick workarounds for this issue - it requires a couple of changes in the DataTables core.

    I've just committed the required changes into the 1.10 development branch. This is the change set:
    https://github.com/DataTables/DataTables/commit/86fd198 .

    You might be able to pull the changes into 1.9.4, or you could try 1.10 if you are feeling brave :-). Either way, this change will be in 1.10! Thanks from brining it to my attention!

    Regards,
    Allan
  • sebastienbarresebastienbarre Posts: 24Questions: 1Answers: 0
    edited May 2013
    Thanks a lot Allan, I was able to pull the changes into 1.9.4 without problem. Now pushing to production server, crossing my fingers :)
This discussion has been closed.