From c27938a1239e8634ac45c706188ba3482febab1b Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Tue, 21 Apr 2015 14:19:45 -0400 Subject: [PATCH] Fix bug in `isWritable` utility function Only attempt to re-set the property value in cases where it was successfully modified as part of the function's execution. This avoids errors when the underlying value is not writable. Rename the internal result-tracking variable to make this more clear. --- harness/propertyHelper.js | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/harness/propertyHelper.js b/harness/propertyHelper.js index d415f2fa96..c5b5e0f7c1 100644 --- a/harness/propertyHelper.js +++ b/harness/propertyHelper.js @@ -31,7 +31,7 @@ function isWritable(obj, name, verifyProp, value) { var newValue = value || "unlikelyValue"; var hadValue = Object.prototype.hasOwnProperty.call(obj, name); var oldValue = obj[name]; - var result; + var writeSucceeded; try { obj[name] = newValue; @@ -41,16 +41,21 @@ function isWritable(obj, name, verifyProp, value) { } } - result = (verifyProp && isEqualTo(obj, verifyProp, newValue)) || + writeSucceeded = (verifyProp && isEqualTo(obj, verifyProp, newValue)) || isEqualTo(obj, name, newValue); - if (hadValue) { - obj[name] = oldValue; - } else { - delete obj[name]; + // Revert the change only if it was successful (in other cases, reverting + // is unnecessary and may trigger exceptions for certain property + // configurations) + if (writeSucceeded) { + if (hadValue) { + obj[name] = oldValue; + } else { + delete obj[name]; + } } - return result; + return writeSucceeded; } function verifyEqualTo(obj, name, value) {