[webkit-reviews] review denied: [Bug 128788] Implement a few more Array prototype functions in JS : [Attachment 224141] Patch

[bugzilla-daemon at webkit.org]

Gavin Barraclough <barraclough at apple.com> has denied Oliver Hunt
<oliver at apple.com>'s request for review:
Bug 128788: Implement a few more Array prototype functions in JS
https://bugs.webkit.org/show_bug.cgi?id=128788

Attachment 224141: Patch
https://bugs.webkit.org/attachment.cgi?id=224141&action=review

------- Additional Comments from Gavin Barraclough <barraclough at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=224141&action=review


r- due to defineOwnProperty issue – unless I'm wrong & there is magic to
prevent the prototype chain being consulted.

> Source/JavaScriptCore/builtins/Array.prototype.js:63
> +    if (typeof callback !== "function")

OOI, I think there is a change in behavior here – I think the code currently
checks whether the object is callable (as specified), whereas this is checking
for functions. This means that callable non-functions will behave differently
in these functions than in the previous implementation (and in host functions
still written as native code) – e.g rexeps, callable API objects.

Probably not a big issue, but we should probably resolve this at some point.

> Source/JavaScriptCore/builtins/Array.prototype.js:96
> +	       result[nextIndex++] = current;

This should be defineOwnProperty, if the prototype chain contains numeric
setters or read only properties I think this will do the wrong thing?

> Source/JavaScriptCore/builtins/Array.prototype.js:122
> +	   result[i] = callback. at call(thisArg, array[i], i, array)

This should be defineOwnProperty, if the prototype chain contains numeric
setters or read only properties I think this will do the wrong thing?