Editor doesn't notice boolean value change from null to false.

ptmkarvinen

Link to test case:
https://drive.google.com/drive/folders/1pK-uhxnis_TOIaOnqlFLqhVpqEHq18MG

Contains an index.html with the DataTable, a db.json with hard-coded sample data returned by the server, and a video demonstration of the issue.

Also attached to this post is index_with_data.html, which is the index.html with the sample data added in as a comment, for posterity.

Debugger code (debug.datatables.net):
https://debug.datatables.net/azenip

Description of problem:
If you have serverSide processing turned on and server is returning boolean value for a field with null being a potential choice, editor doesn't notice the value change from null to false. Any other editing combination works, even false -> null, which still sends a request to backend.

Due to how our backend is designed, the data cannot be handled as anything but boolean, so converting it to string on backend isn't a possible option at this moment (although this would fix the frontend issue). Similarly, setting submit type to something other than "changed" fixes the problem, but is far from optimal due to large quantities of fields and data.

Are there any other possible work-arounds for this, or some potential for a bug fix?

ptmkarvinen

Oh right,
Editor version: 1.9.5
DataTables version: 1.10.21 (1.10.22 also tested)

ptmkarvinen

Ok, so, I took a look at the editor code for a bit, and apparently the fix is as simple as not using loose type checking for plain value comparison in _deepCompare. I don't think I should post any editor code here, but if there are any dev team members that happen to read this I'll be glad to discuss this with more detail.

I'm a bit hesitant to hack the solution in as a fix myself, since that'd make updates a bit more of a hassle. Also, maybe there's a reason why strict value comparison isn't enabled by default?

ptmkarvinen

Upon further testing, the solution I mentioned above is flawed, since the way null booleans seem to be handled is by considering them empty strings, so the type handling fails (it worked since that field was always sent due to failed comparison). Also, empty input values are considered empty objects. So, kind of inconsistent here, but workable.

I wrote a quick and dirty custom compare function that handles booleans correctly, might need to tweak it later if issues arise with other datatypes.

function customCompare(o1,o2){
    // new is boolean
    if(typeof o1 === 'boolean' && typeof o2 === 'string'){
        return o2.toString() === o1.toString();
    }
    // old is boolean
    else if(typeof o2 === 'boolean' && typeof o1 === 'object'){
        return o1 == o2;
    }
    // all empty values
    else if(!o1 && !o2){
        return true;
    }
    else{
        return o1 == o2;
    }
}

The issue does seem a little tricky considering the construction of the grid, so might not be a simple patch to fix this after all.

allan

Hi,

Thanks for the updates and your insight into this. It's an interesting problem this - the problem is that most values are written into DOM input elements, which means they are always read back as strings. The exception to that rule is the select field type where we store the original value and use that.

What we probably need to do is sort out our typing support properly (and also allow for null values, which goes hand in hand with this).

If you can live with your workaround for now, that is probably the way to go!

Regards,
Allan

ptmkarvinen

Hey Allan!

I think we can manage with a custom solution for now, the issue is relatively small in the grand spectrum of things. I did notice select fields acting kinda strange when it comes to submitting data as well: when using batch editing some null fields tend to get sent even if value remains unchanged, which seems related to this problem and should be possible to fix in similar fashion.

Official patch would be nice just so we don't have to keep hotfixing releases, but what you're suggesting definitely sounds better than just tackling this particular issue head on.

Thank you for the info!