Closed Bug 342359 Opened 18 years ago Closed 18 years ago

overriding ReferenceError doesn't stick

Categories

(Core :: JavaScript Engine, defect, P2)

Product:
Core
Component:
JavaScript Engine
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.9alpha1

People

(Reporter: jminta, Assigned: mrbkap)

References

Details

(Keywords: verified1.8.1)

Attachments

(3 files)

Fix
(deleted), patch
brendan
: review+
Details | Diff | Splinter Review
js1_5/Exceptions/regress-342359.js
(deleted), text/plain
Details
Make that work too
(deleted), patch
brendan
: review+
jst
: superreview+
Details | Diff | Splinter Review
As reported by shaver/mrbkap: js> ReferenceError = 5 5 js> ReferenceError 5 js> foo.blitz typein:4: ReferenceError: foo is not defined js> ReferenceError function ReferenceError() { [native code] } Also visible on JS 1.7 alpha branch.
Attached patch Fix (deleted) — — Splinter Review
I blame Mccabe for using 1 class for all of the exception classes. If he didn't, we'd be able to use JS_InitClass directly, instead of hand-unrolling it and forgetting to actually set the constructor value in the global object. This patch makes sure we do that.
Assignee: general → mrbkap
Status: NEW → ASSIGNED
Attachment #226562 - Flags: review?(shaver)
Attachment #226562 - Flags: review?(brendan)
Comment on attachment 226562 [details] [diff] [review] Fix Looks good -- thanks for fixing. /be
Attachment #226562 - Flags: review?(brendan) → review+
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Priority: -- → P2
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
Attachment #226562 - Flags: review?(shaver)
I still see this in the browser 20060622 trunk build when evaluating the test in Jesse's jsshell.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
If you do it twice, it appears to stick... ReferenceError = 5 5 foo.blitz ReferenceError on line 1: foo is not defined ReferenceError function ReferenceError() { [native code] } ReferenceError = 5 5 foo.blitz ReferenceError on line 1: foo is not defined ReferenceError 5
Attached file js1_5/Exceptions/regress-342359.js (deleted) —
But trunk appears to pass this test. If the failure is just a problem with using the jsshell, we can mark this fixed again.
Attached patch Make that work too (deleted) — — Splinter Review
Resolve implementations have to make sure they initialize standard classes even when assigning so that given the above testcase, we initialize the exception classes before we assign to the name ReferenceError. Otherwise when the class is defined, it'll overwrite the names.
Attachment #226701 - Flags: review?(brendan)
Comment on attachment 226701 [details] [diff] [review] Make that work too Sure. Anything else to comment or bracket with not-assigning guards in nsDOMClassInfo.cpp? /be
Attachment #226701 - Flags: review?(brendan) → review+
Attachment #226701 - Flags: superreview?(jst)
Blocks: 335267
Comment on attachment 226701 [details] [diff] [review] Make that work too sr=jst
Attachment #226701 - Flags: superreview?(jst) → superreview+
Really fixed this time.
Status: REOPENED → RESOLVED
Closed: 18 years ago → 18 years ago
Resolution: --- → FIXED
verified fixed 1.9a1 20060630 win/mac(ppc|tel)/linux
Status: RESOLVED → VERIFIED
Blocks: js1.7
Flags: in-testsuite+
fixed by Bug 336373 on the 1.8.1 branch. verified fixed 1.8.1 with windows/macppc/linux 20060707
Keywords: verified1.8.1
forgot to check this in. Checking in regress-342359.js; /cvsroot/mozilla/js/tests/js1_5/Exceptions/regress-342359.js,v <-- regress-342359.js initial revision: 1.1
(In reply to comment #13) > /cvsroot/mozilla/js/tests/js1_5/Exceptions/regress-342359.js,v <-- this now fails on trunk due to bug 376957.
Checking in regress-342359.js; /cvsroot/mozilla/js/tests/js1_5/Exceptions/regress-342359.js,v <-- regress-342359.js new revision: 1.4; previous revision: 1.3 done
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: