From Bugzilla Helper: User-Agent: Mozilla/5.0 (X11; U; Linux i386; en-US; rv:0.9.9+) Gecko/20020411 BuildID: 2002041108 When passing parameters to a parent object via parentObject.call(parameters) in the child object constructor, member variables with the same name as the parameter is forced to a value of undefined even if the parameter is not used to set the member variable with its name. Reproducible: Always Steps to Reproduce: 1. Go to the URL, http://outroad.org/mozillaParamBug.html Actual Results: The first two fields on the page are set to undefined Expected Results: They should be set to values of "passed to matchAndSet" and "match, but set inside". This bug has also been verified for win32 build 2002041103, but did not exist when using mozilla 0.9.7, so some change since then must be the cause.

Confirming bug and cc'ing Brendan. Here is the reduced testcase: <SCRIPT> function parentObject(p) { this.p = 1; } function childObject() { parentObject.call(this); } childObject.prototype = parentObject; var o = new parentObject(); document.write('<br>o = new parentObject():<br>o.p = ' + o.p + '<br>'); o = new childObject(); document.write('<br>o = new childObject():<br>o.p = ' + o.p + '<br>'); </SCRIPT> In NN4.7 and IE6, the output is: ----------------------------------------------- o = new parentObject(): o.p = 1 o = new childObject(): o.p = 1 Mozilla trunk binaries 20020411xx WinNT, Linux: ----------------------------------------------- o = new parentObject(): o.p = 1 o = new childObject(): o.p = undefined As Ben says, the problem is the use of the identifier 'p' for both a property name in the parent, and parameter to the parent constructor. If we change the parameter name to anything other than 'p', the output in Mozilla is the same as in NN4.7 and IE6.

Here is an even simpler testcase of the same problem, without the .call(). =========================== function foo(arg) {}; foo.arg = 12; alert(foo.arg); =========================== the alert should display "12" but instead displays "undefined". The correct behavior occurs when using different names for the argument(s) to foo() and properties of that object.

Here's a simple fix, though I want to look more at the interactions with the rest of our argument/property machinery. I won't get a chance to do a test run for a little while, but I'll try to get one tonight.

*** Bug 138708 has been marked as a duplicate of this bug. ***

Comment on attachment 85623 [details] [diff] [review] Fall back to getting/setting on the function object if no live argv is found. Can't do that, OBJ_GET_PROPERTY and all JSObjectOps take a jsid id, not a jsval id. You'll recurse to death, too. /be

Testcase added to JS testsuite: mozilla/js/tests/js1_5/Object/regress-137000.js NOTES 1. The patch in Comment #4 fixes the test given in Comment #3, but it does not fix the original test given in Comment #2. 2. The RC4 release of SpiderMonkey (01 Jan 2002) passes all the tests above, plus Rustam's test from bug 138708. 3. The RC4a release fails in the same manner as the current trunk. So something changed between RC4 and RC4a (21 March 2002). 4. Possible candidate: the fix for bug 131510? "Crash when |arguments| defined as a variable inside function" 5. At any rate, here is a quick change log for RC4a vs. RC4: http://www.mozilla.org/js/spidermonkey/release-notes/JS_150_RC4a.html

Wanted for 1.0.1 and (if I get my act together tonight) 1.1alpha. /be

This should wait till the 1.1beta trunk opens, bake a day or so, and then go into the 1.0 branch for 1.0.1. /be

Comment on attachment 86378 [details] [diff] [review] "Unshare" arg or local-var on set, if JSPROP_SHARED sr=shaver.

In reply to phil's comment #7, the regression here came in with the patch for bug 62164. That patch changed args and local vars in a function object to have the JSPROP_SHARED attribute, which means that f.a = 42 stores no persistent 42 in a slot in the object denoted by f. The patch to fix this bug, attachment 86378 [details] [diff] [review], preserves the economy of default-shared arg and local var properties, but detects the f.a = 42 case where f is not active, and morphs f.a into an unshared property with its own slot in which to store 42. /be

*** Bug 150032 has been marked as a duplicate of this bug. ***

Have just added a new section to the above testcase, by john@statesoftware.com, from the duplicate bug 150032. The new section tests what happens if a local var in a function uses the same name as a property of the function. This complements the rest of the testcase, which considers the problem of a function parameter having the same name as a function property. All sections of the testcase are currently passing in Rhino. In SpiderMonkey I get failures even with the latest patch applied: WITHOUT PATCH WITH PATCH Sections 1,3,5,6 fail Sections 3,5,6 fail

Phil, sections 3, 5, and 6 test the old-as-Mocha (1995) feature of SpiderMonkey whereby f.a = x in a function f that has an arg or local var named a will set that arg or local var, but not any property of the function. I suppose it's time we broke that bit of compatibility. New patch in a few, /be

This rips out the pre-ECMA "extension" (more a side-effect of an implementation detail) where function f(a){return f.a}; f.a = 52; print(f(42)) would print 42 instead of 52. It preserves the expectations of the rest of the engine that args and vars are stored as properties. /be

Most recent patch fails test case in comment #3.

khanson: it works for me: [~/src/trunk-opt/mozilla/js/src]$ Linux_All_DBG.OBJ/js js> function f(a){} js> f.a=12 12 js> f.a 12 js> Are you sure you applied the patch and recompiled properly? /be

Comment on attachment 86950 [details] [diff] [review] better fix sr=shaver. You're right, it _is_ time.

I got my files mixed up. It works for me.

Most recent patch passes all sections of the above testcase. I also ran the full JS testsuite against it in the optimized JS shell on WinNT, and got no regressions -

Comment on attachment 86950 [details] [diff] [review] better fix r = khanson. Looks fine.

Fix is in the trunk, going for drivers approval for the 1.0 branch. /be

This is crashing an optimized mozilla win32 trunk build pulled this evening. I've backed out and reapplied this patch a couple of times, and with the patch I crash, without I run fine. The crash is in function 'Variables' of jsparse.c. I'll file a separate bug with more detail in a moment (or is this already a known issue).

Filed bug 151066 for the crash.

Note the above patch has been backed out of the trunk. See bug 151066: ------- Additional Comment_ #2 From John Morrison 2002-06-12 02:05 ------- Brendan backed out rev 3.105 of jsinterp.c which avoids the crash in my win32 opt. build, pending further investigation.

*** Bug 150859 has been marked as a duplicate of this bug. ***

Comment on attachment 86950 [details] [diff] [review] better fix a=asa (on behalf of drivers) for checkin to 1.1

Fixed on branch and trunk. Thanks again, jrgm and rpotts. /be

Fixed, I say! /be

Verified Fixed. The above testcase passes in the debug/optimized JS shell on WinNT, Linux, and Mac9.1. Tested on both the trunk and -rMOZILLA_1_0_BRANCH.