1440468 - Proxied functions can't be passed to Function.prototype.toString.call()

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[John-David Dalton]

Attachment: safari.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.167 Safari/537.36

Steps to reproduce:

var a = function() {}
var p = new Proxy(a, {})

Function.prototype.toString.call(p)
// TypeError: Function.prototype.toString called on incompatible object


typeof p // "function"
Object.prototype.toString.call(p) // [object Function]


With at least Safari passing it to Function.prototype.toString.call will work.
Related to https://bugs.chromium.org/p/v8/issues/detail?id=7484


Actual results:

It throws an error.


Expected results:

It should not throw.

Comment 1

[John-David Dalton]

Patched in v8:
https://chromium.googlesource.com/v8/v8.git/+/f7d7b5c6a4a55baa8984525fba6d0d5e1355b3b0

Comment 2

[Tom S. (please needinfo tschuster)]

I hadn't heard about this and I am sure I agree that this change is a good idea. I am going to look at the proposal.

Comment 3

[Tom S. (please needinfo tschuster)]

https://github.com/tc39/Function-prototype-toString-revision/issues/3

Comment 4

[Jason Orendorff [:jorendorff]]

Eewwwwwwww

Comment 6

[Tom S. (please needinfo tschuster)]

Attachment: Proxied functions can't be passed to Function.prototype.toString.call(). r?

Comment 7

[Tom S. (please needinfo tschuster)]

Comment on attachment 9006002 [details] [diff] [review]
Proxied functions can't be passed to Function.prototype.toString.call(). r?

I changed the error message, because "x is not a function" doesn't really make sense while allowing all callables.

Comment 8

[Jan de Mooij [:jandem]]

evilpie++

Comment 9

[André Bargull [:anba]]

Comment on attachment 9006002 [details] [diff] [review]
Proxied functions can't be passed to Function.prototype.toString.call(). r?

Review of attachment 9006002 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM!

And good idea to adjust the error message.

Comment 10

[John-David Dalton]

By way of the `Function#toString` revisions `Function.prototype.toString.call` does allow proxied functions. This is now part of test262 and has been fixed in Chakra, V8, and JS Core, leaving FF as the only one not supporting this.

Comment 11

[Tom S. (please needinfo tschuster)]

Attachment: Fix devtools test

Comment 12

[Yulia Startsev [:yulia]]

Comment on attachment 9006103 [details] [diff] [review]
Fix devtools test

Looks good to me

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3528822f9b00
Proxied functions can't be passed to Function.prototype.toString.call(). r=anba
https://hg.mozilla.org/integration/mozilla-inbound/rev/02675ca26d6c
Fix devtools test for new Function.prototype.toString behavior. r=yulia

Comment 14

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/3528822f9b00
https://hg.mozilla.org/mozilla-central/rev/02675ca26d6c

Comment 15

[Chris Mills [:cmills]]

Note to docs team:

I have added a note covering this to the Fx 64 rel notes:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/64#JavaScript 

This probably needs describing in a sensible place in the docs, and maybe adding to BCD somehow too?

Comment 17

[Florian Scholz (Open Web Docs)]

It looks like this is part of https://github.com/tc39/Function-prototype-toString-revision and I've documented that when it got implemented initially: https://bugzilla.mozilla.org/show_bug.cgi?id=1317400#c52

The compat table (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/toString#Browser_compatibility) says Firefox 54, because that's when bug 1317400 landed. Should we update the table to say Firefox 64 and mark 54-63 as partial support for "String revision" then?

Should this be mentioned somewhere in the Proxy docs? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy

Not sure if this is really a significant change that actually requires more detailed docs. Please let me know.

Comment 18

[Tom S. (please needinfo tschuster)]

(In reply to Florian Scholz [:fscholz] (MDN) from comment #17)
> It looks like this is part of
> https://github.com/tc39/Function-prototype-toString-revision and I've
> documented that when it got implemented initially:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1317400#c52
> 
Great
> The compat table
> (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/
> Global_Objects/Function/toString#Browser_compatibility) says Firefox 54,
> because that's when bug 1317400 landed. Should we update the table to say
> Firefox 64 and mark 54-63 as partial support for "String revision" then?
> 
Sounds fine to me, or maybe just split this feature out?
> Should this be mentioned somewhere in the Proxy docs?
> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/
> Global_Objects/Proxy
> 
I don't think we need to mention this on the Proxy page, but https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/toString#Description needs to be updated about not throwing for Proxy.
> Not sure if this is really a significant change that actually requires more
> detailed docs. Please let me know.

Comment 19

[Florian Scholz (Open Web Docs)]

Thanks!

Updated
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/toString#Description
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/toString#Firefox-specific_notes