780542 - constructor functions return undefined in web console

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Watilin]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/17.0 Firefox/17.0
Build ID: 20120805030518

Steps to reproduce:

(On Nightly 17.0a1 with a fresh profile)

On a page containing this script :

function F() { this.x = 1; }

I opened the web console and typed "new F()".


Actual results:

the console returned "undefined".


Expected results:

It should have returned "({x:1})".

However, when I type "new window.F()" I get the right result. This bug seems related to https://bugzilla.mozilla.org/show_bug.cgi?id=752404 although the latter have been resolved.

Comment 1

[Loic]

In the latest Nightly, if I type your instructions in the web console, I see:

--
[10:43:51.175] function F() { this.x = 1; }
[10:43:51.180] undefined
--
[10:43:56.231] new F()
[10:43:56.251] ({x:1})

Comment 2

[Watilin]

Of course it works when the constructor has been declared into the console.
I figured out it also works when the constructor is used inside another function declared into the page :

function createF() { return new F(); }

When typing "createF()" in the console, I get the correct "({x:1})". It seems that the "new" operator has trouble to work wih the global context.

Comment 3

[Watilin]

Attachment: Embedded functions "F" and "createF"

This page contains the declaration of functions "F" and "createF" to be used in the Web Console.

Comment 4

[Loic]

I followed your testcase and I found this regression range:

mozilla-central:
good=2012-07-14
bad=2012-07-15
changelog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0602e44ac248&tochange=57abb5f70e01

Suspected bug to confirm:
Boris Zbarsky — Bug 771429. Instead of using bound functions for the functions we get off the sandbox proto, use a function proxy. That allows property gets on the functions to get through. r=bholley

Comment 5

[Boris Zbarsky [:bzbarsky]]

This is totally a JS engine bug; I assumed that proxy construction actually worked correctly, but it very much doesn't.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Make construction via a function proxy with an object in the constructor slot actually construct instead of just calling.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Requesting tracking for the regression.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Make construction via a function proxy with an object in the constructor slot actually construct instead of just calling.

Comment 9

[Eddy Bruel [:ejpbruel]]

Comment on attachment 650145 [details] [diff] [review]
Make construction via a function proxy with an object in the constructor slot actually construct instead of just calling.

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

Looking good :-)

Comment 10

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a047c51aafd0

Eddy, how do you feel about taking this on branches?  Seems like we need to do this if we want to fix the web console behavior there....

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 650145 [details] [diff] [review]
Make construction via a function proxy with an object in the constructor slot actually construct instead of just calling.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 771429 
User impact if declined: Calling non-DOM constructors in the web console doesn't
   work right.
Testing completed (on m-c, etc.): Passes attached automated tests.
Risk to taking this patch (and alternatives if risky): I don't know.  Eddy is a
   better source for this.
String or UUID changes made by this patch: None.

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a047c51aafd0

Comment 13

[Boris Zbarsky [:bzbarsky]]

For what it's worth, Eddy thinks risk is low.

As far as alternatives if we decide this is too risky, I see several:

1)  Ship with this problem.
2)  Back out bug 771429 and ship with its problem instead of this one.
3)  Back out bug 771429 and fix it in some other way.  I'm not quite sure how, and it
    would almost certainly be riskier than this patch.
4)  Change the function proxy code so consumers can pick between the old and new behavior
    and only opt in sandboxes to the new behavior for now.  This is probably doable if
    really needed.  I think.

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 650145 [details] [diff] [review]
Make construction via a function proxy with an object in the constructor slot actually construct instead of just calling.

I think we're ok to not do a backout and to take this on branches.

Comment 15

[Joe Drew (not getting mail)]

This was pushed to aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/4cbf6aa4f022
and then backed out for making aurora orange. https://hg.mozilla.org/releases/mozilla-aurora/rev/c2d5393d005c

Comment 16

[Joe Drew (not getting mail)]

Similarly landed on beta: https://hg.mozilla.org/releases/mozilla-beta/rev/ad7dd2476dfe
and then backed out: https://hg.mozilla.org/releases/mozilla-beta/rev/aa331c131a65

Comment 17

[Joe Drew (not getting mail)]

Only partially landed on aurora:  https://hg.mozilla.org/releases/mozilla-aurora/rev/9211b0e0edad and hence backed out: https://hg.mozilla.org/releases/mozilla-aurora/rev/9211b0e0edad

Comment 18

[Eddy Bruel [:ejpbruel]]

Pushed to aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/52956b3e7be2

Hopefully wont be backed out this time

Comment 19

[Eddy Bruel [:ejpbruel]]

And again to beta:
https://hg.mozilla.org/releases/mozilla-beta/rev/bc0a3117d9b4

Comment 20

[Eddy Bruel [:ejpbruel]]

D'oh

Comment 21

[Boris Zbarsky [:bzbarsky]]

For what it's worth, it looks like the first thing that got pushed was the old, obsolete, patch.  The one that didn't have approvals...

Comment 22

[Paul Silaghi, QA [:pauly]]

Able to see the issue on FF 15b4.
Verified fixed on FF 15b5 on Win XP and Win 7 using the STR in comment 0 and the testcase.
Removing verifyme for now. Please add again if you can think of any edge cases related to this that should be tested.

Comment 23

[Paul Silaghi, QA [:pauly]]

Verified fixed on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/20100101 Firefox/16.0b4
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 17.0a2 (2012-09-24)