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

RESOLVED FIXED in Firefox 64

Status

()

defect
P3
normal
RESOLVED FIXED
a year ago
3 months ago

People

(Reporter: john.david.dalton, Assigned: evilpie)

Tracking

(Blocks 1 bug, {dev-doc-complete})

58 Branch
mozilla64
Points:
---

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(3 attachments)

(Reporter)

Description

a year ago
Posted image 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.
Status: UNCONFIRMED → NEW
Component: Untriaged → JavaScript Engine
Ever confirmed: true
Product: Firefox → Core
(Assignee)

Comment 2

a year ago
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.
Eewwwwwwww
Priority: -- → P3

Updated

11 months ago
Blocks: test262
Duplicate of this bug: 1487046
(Assignee)

Updated

8 months ago
Assignee: nobody → evilpies
(Assignee)

Comment 7

8 months ago
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.
Attachment #9006002 - Flags: review?(andrebargull)
evilpie++
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.
Attachment #9006002 - Flags: review?(andrebargull) → review+
(Reporter)

Comment 10

8 months ago
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.
(Assignee)

Comment 11

8 months ago
Attachment #9006103 - Flags: review?(ystartsev)
Comment on attachment 9006103 [details] [diff] [review]
Fix devtools test

Looks good to me
Attachment #9006103 - Flags: review?(ystartsev) → review+

Comment 13

8 months ago
Pushed by evilpies@gmail.com:
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
(Assignee)

Updated

8 months ago
Keywords: dev-doc-needed

Comment 14

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3528822f9b00
https://hg.mozilla.org/mozilla-central/rev/02675ca26d6c
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64

Updated

8 months ago
Target Milestone: mozilla64 → mozilla63

Updated

8 months ago
Target Milestone: mozilla63 → mozilla64
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?
(Assignee)

Updated

5 months ago
Duplicate of this bug: 1508968
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.
Flags: needinfo?(evilpies)
(Assignee)

Comment 18

5 months ago
(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.
Flags: needinfo?(evilpies)
You need to log in before you can comment on or make changes to this bug.