Announcement

Collapse
No announcement yet.
X
  • Filter
  • Time
Clear All
new posts

    question about DynamicForm.compareValues method and handling of NaN values

    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


    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;
    },

    #2
    Hi there, we also see a similar issue with undefined values not being evaluated properly by the fieldValuesAreEqual() method when called in the storeUpdatedEditorValue() method. The method storeUpdatedEditorValue() forces the value returned from _parseEditorValue() to null if it is undefined before it is passed to setEditValue(). However, the oldValue remains as undefined in the _storeEditValue() method which is called by storeUpdatedEditorValue() > setEditValue(). As a result, a call to fieldValuesAreEqual() will return a true because value has been forced to null in storeUpdatedEditorValue() while oldValue is undefined.

    These issues with NaN (in my initial message) and undefined values are causing unneeded calls to calculateRecordSummaries() in the storeUpdatedEditorValue() method because the changed variable is being erroneously set to true.

    We are implementing workarounds on our end for these issues but wanted to let you know and get your feedback.

    Comment

    Working...
    X