Closed Bug 500691 Opened 13 years ago Closed 13 years ago

Function.prototype no longer extends native functions

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla1.9.2a1

People

(Reporter: cwolves, Assigned: mrbkap)

References

Details

(Keywords: regression, testcase, verified1.9.1.1)

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090616 Firefox/3.5
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090616 Firefox/3.5

This is either a regression or a security change, but I can't find any documentation that it was changed on purpose.  Run:


javascript:Function.prototype.foo=10;alert((function(){}).foo);alert(alert.foo);

You "should" get two alerts, that both say "10".  In FF3.5, the 2nd alert says undefined.

In FF3.0, Safari4, IE7, IE8 you get two alerts that say 10.


Further, all functions under document seem to behave as expected:

document.write
document.writeln
document.createElement
...etc

Reproducible: Always
In short, Function.prototype doesn't seem to extend native functions under the window object.
Regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-08-21+14%3A00%3A00&enddate=2008-08-21+19%3A00%3A00
Assignee: nobody → general
Status: UNCONFIRMED → NEW
Component: General → JavaScript Engine
Ever confirmed: true
Keywords: regression, testcase
OS: Mac OS X → All
Product: Firefox → Core
QA Contact: general → general
Hardware: x86 → All
Version: unspecified → Trunk
WFM on mozilla-central nightly, built from

http://hg.mozilla.org/mozilla-central/rev/92b6868ef3f4

On what release(s) with what exact buildconfig tips is this broken?

/be
OSX: Nightly build fails for me (3.6a1, 20090626), as does 3.5b (20090616).

Windows: 3.5 (20090624) fails.
these are just base installs, no options changed and I didn't build them myself.
Mark, can you attach the testcase you're using?
Literally, it's that javascript: URI, or:

javascript:Function.prototype.foo=10;alert((function(){}).foo + ', ' + alert.foo);

I get:

"10, undefined"

javascript:Function.prototype.foo=10;alert((function(){}).foo + ', ' + document.write.foo);

gives: "10, 10"
or more simply:

javascript:Function.prototype.foo=10;alert(alert.foo?'Success!':'Fail!');
Assignee: general → jwalden+bmo
Status: NEW → ASSIGNED
javascript:alert(Function===alert.constructor?'Success!':'Fail!')

javascript:alert(Function.prototype===alert.__proto__?'Success!':'Fail!')

Blake is investigating since this is probably more up his alley.
Blake said he had a proto-patch that might fix the bug but still needs a good bit of work.
Assignee: jwalden+bmo → nobody
Component: JavaScript Engine → DOM
QA Contact: general → general
Assignee: nobody → mrbkap
This is a regression from bug 451154. What happens is that we initialize function and object classes on the *outer* window and (after connecting the inner and outer objects) call XPCWrappedNativeScope::SetGlobal(outer window). This function looks up Function.prototype on the global. Before bug 451154, looking up Function would call the getter nsWindowSH::GetProperty, forwarding the property lookup to the inner window and retrieving the correct value. Now, there is no such getter, and the function is effectively a no-op. I suspect that we can fix this bug by calling ClearScope on the outer window earlier in SetNewDocument.
Blocks: 451154
Attached patch Proposed fix (obsolete) — Splinter Review
The comments in the patch should hopefully explain this...

Basically, there's a call in XPConnect (XPCWrappedNativeScope::SetGlobal) that looks up Function.prototype and Object.prototype on the global and we were not calling ClearScope in the right spot. A tunnel-vision view of nsGlobalWindow::SetNewDocument with the stuff that mhammond moved out to nsJSEnvironment.cpp inlined looks like:

SetNewDocument():
  - ClearScope(outer window)
  - Maybe free inner objects(now-old inner window)
  * WrapNative(newInnerWindow)
  - outer window.mInnerWindow = new nsGlobalWindow()
  1 ClearScope(outer window)
  2 xpconnect->InitClasses(outer window)
  - Do prototype fixup dance with outer & new inner window

* Causes Function and Object to be re-created on the outer window.

Before this patch, 1 and 2 above were reversed, meaning that the call to SetGlobal was effectively a no-op.
Attachment #386220 - Flags: superreview?(bzbarsky)
Attachment #386220 - Flags: review?(jst)
I'll attach a mochitest tomorrow (out of time tonight).
Attached patch With mochitestSplinter Review
As above, but with a mochitest.
Attachment #386220 - Attachment is obsolete: true
Attachment #386324 - Flags: superreview?(bzbarsky)
Attachment #386324 - Flags: review?(jst)
Attachment #386220 - Flags: superreview?(bzbarsky)
Attachment #386220 - Flags: review?(jst)
Comment on attachment 386324 [details] [diff] [review]
With mochitest

>@@ -1386,16 +1386,17 @@ NS_IMPL_CYCLE_COLLECTION_CLASS(nsJSConte
>+  (void)tmp;

Why?

>+ok(Function===alert.constructor, "alert's constructor is our Function");

Worth it to also check window.Function here?

With those nits, looks scary but ok.
Attachment #386324 - Flags: superreview?(bzbarsky) → superreview+
(In reply to comment #15)
> >+  (void)tmp;
> Why?

Otherwise I get a warning, "tmp is not used" there.

> Worth it to also check window.Function here?

Sure.
Comment on attachment 386324 [details] [diff] [review]
With mochitest

r=jst, but leave the first hunk out and fix that in the macros in a new bug.
Attachment #386324 - Flags: review?(jst) → review+
http://hg.mozilla.org/mozilla-central/rev/e7cf1bed5a0e
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: blocking1.9.1.1?
Resolution: --- → FIXED
Attachment #386324 - Flags: approval1.9.1.1?
Flags: wanted1.9.1.x+
Flags: blocking1.9.1.1?
Flags: blocking1.9.1.1+
Comment on attachment 386324 [details] [diff] [review]
With mochitest

Approved for 1.9.1.1. a=ss for release-drivers.
Attachment #386324 - Flags: approval1.9.1.1? → approval1.9.1.1+
Can we please get this on 1.9.1.1 ASAP?
Flags: blocking1.9.1.1+ → blocking1.9.1.1-
Verified fixed on trunk and 1.9.1 with builds on all platforms like

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090714 Minefield/3.6a1pre ID:20090714050656

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.1pre) Gecko/20090715 Shiretoko/3.5.1pre ID:20090715031842
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
Target Milestone: --- → mozilla1.9.2a1
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.