Hi there, we are noticing an issue where DynamicForm.compareValues(NaN,NaN,null,true); always returns false. It seems that method is not checking for isNaN() when inspecting the parameters passed since NaN==NaN will always return false. This is causing calls for exiting the editing a grid cell to incorrectly report that some values which are NaN have changed when calling _storeEditValue() > fieldValuesAreEqual() > compareValues()
Is the following patch of DynamicForm.compareValues() sensible and should it be applied to the framework? See the comment below like this: //updated to compare isNaN values
Is the following patch of DynamicForm.compareValues() sensible and should it be applied to the framework? See the comment below like this: //updated to compare isNaN values
Code:
compareValues : function (value1, value2, field, exactEquality) {
if (field) {
if(field.type) {
var simpleType = isc.SimpleType.getType(field.type);
if (simpleType && simpleType.compareValues) {
return simpleType.compareValues(value1, value2, field) == 0;
}
if (field.type == "valueMap") {
if (isc.isAn.Array(value1) && isc.isAn.Array(value2)) {
return value1.equals(value2)
} else if (isc.isAn.Object(value1) && isc.isAn.Object(value2)) {
for (var i in value1) {
if (value2[i] != value1[i]) return false;
}
for (var j in value2) {
if (value1[j] != value2[j]) return false;
}
// everything matched
return true;
}
}
}
if (isc.isA.Date(value1) && isc.isA.Date(value2)) {
if ((isc.SimpleType.inheritsFrom(field.type, "date") &&
!isc.SimpleType.inheritsFrom(field.type, "datetime")) ||
value1.logicalDate || value2.logicalDate)
{
return (isc.DateUtil.compareLogicalDates(value1, value2) == 0);
} else if (isc.SimpleType.inheritsFrom(field.type, "time") || value1.logicalTime || value2.logicalTime) {
return (isc.Time.compareLogicalTimes(value1, value2) == 0);
} else {
return (isc.DateUtil.compareDates(value1, value2) == 0);
}
}
}
if (isc.isAn.Array(value1) && isc.isAn.Array(value2)) {
if (value1.length != value2.length) return false;
for (var i = 0; i < value1.length; i++) {
if (!isc.DynamicForm.compareValues(value1[i], value2[i], field)) {
return false;
}
}
return true;
}
// handle having values set to Number, String etc instance
// IE var foo = new Number(2); rather than just var foo = 2;
// This returns true for isA.Object()
if (isc.isA.Number(value1) || isc.isA.String(value1) || isc.isA.Boolean(value1)) {
value1 = value1.valueOf();
}
if (isc.isA.Number(value2) || isc.isA.String(value2) || isc.isA.Boolean(value2)) {
value2 = value2.valueOf();
}
// if either value is a DateRange, compare the start and end attributes
if ((value1 && value1._constructor == "DateRange") || (value2 && value2._constructor == "DateRange")) {
return isc.DynamicForm.compareValues(value1 && value1.start, value2 && value2.start, field) &&
isc.DynamicForm.compareValues(value1 && value1.end, value2 && value2.end, field);
}
//updated to compare isNaN values
if ((exactEquality && value1 === value2) || (!exactEquality && value1 == value2) || (isNaN(value1) && isNaN(value2))) {
return true;
}
if (isc.isAn.Object(value1) && isc.isAn.Object(value2)) {
var recursive = isc.DynamicForm.compareValuesRecursive,
tempObj = isc.addProperties({}, value2),
isSGWT = isc.Browser.isSGWT
;
for (var attr in value1) {
if (isSGWT && (attr == isc.gwtRef || attr == isc.gwtModule)) {
// assume SGWT wrapper and module reference always match
} else if (recursive) {
if (!isc.DynamicForm.compareValues(value1[attr], value2[attr])) {
return false;
}
} else {
if (value2[attr] != value1[attr]) return false;
}
delete tempObj[attr];
}
// tempObj should now be empty if they match
for (var attr in tempObj) {
return false;
}
return true;
}
return false;
},
Comment