var x = 0 and x = 0 may set x differently due to JS API bug

VERIFIED FIXED in mozilla0.8

Status

()

VERIFIED FIXED
18 years ago
17 years ago

People

(Reporter: brendan, Assigned: brendan)

Tracking

({js1.5})

Trunk
mozilla0.8
js1.5
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments)

(Assignee)

Description

18 years ago
JS_Evaluate*Script* and JS_ExecuteScript all call js_Execute, passing the "obj"
API param as the "chain" formal param.  js_Execute uses that chain object for
the scope chain of the new execution context, and also as the variables object
(ECMA parlance) of the new frame when called from these API entry points (not
when called from eval or eval-like entry points).

That's wrong: the variables object should be the global object, which is the
last object on the scope chain that starts at "obj" (API param) or "chain"
(js_Execute param).  Trivial patch in a minute, please review and approve ASAP.
Thanks,

/be
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
Keywords: js1.5, mozilla0.8
Target Milestone: --- → mozilla0.8
(Assignee)

Comment 1

18 years ago
Created attachment 22637 [details] [diff] [review]
proposed fix
(Assignee)

Comment 2

18 years ago
Created attachment 22639 [details] [diff] [review]
larger fix that eliminates an unnecessary js_Execute param (fun is induced by down, or should be, if down is non-null)

Comment 3

18 years ago
r=mccabe.

I also ran the testsuite and saw no new failures with this bug.  (Good to be
careful with scoping changes.)

Comment 4

18 years ago
sr=jband
(Assignee)

Comment 5

18 years ago
Fix checked in -- thanks all!

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 6

18 years ago
This fix caused bugs 65860 and 65828. We're going to back it out.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 7

18 years ago
Changes backed out ... r=attinasi@netscape.com (sheriff)

mozilla/js/src/jsapi.c     revision 3.83
mozilla/js/src/jsdbgapi.c  revision 3.22
mozilla/js/src/jsemit.c    revision 3.44
mozilla/js/src/jsinterp.c  revision 3.68
mozilla/js/src/jsinterp.h  revision 3.18
mozilla/js/src/jsobj.c     revision 3.76
mozilla/js/src/jsscript.c  revision 3.26
(Assignee)

Comment 8

18 years ago
Curses -- prefs and xpinstall count on an API bug.  I'll do a patch that cleans
up the internals, adds a better API, and keeps the old APIs working as before.

/be
Status: REOPENED → ASSIGNED
(Assignee)

Comment 9

18 years ago
Created attachment 22965 [details] [diff] [review]
backward compatible patch adds JSOPTION_VAROBJFIX

Comment 10

18 years ago
Seems like an obscure flag to tweak for those not in the know; I'd add a verbose
comment.
(Assignee)

Comment 11

18 years ago
Created attachment 23030 [details] [diff] [review]
mccabe requested a major comment in jsapi.h explaining it all (what a concept! ;-)

Comment 12

18 years ago
sr=jband

Comment 13

18 years ago
r=mccabe.

Wow, big comment!  Maybe 'Use of this option recommended' in it's own paragraph
early, for lazy readers.

Bye.  See me soon at mike+mozilla@meer.net, or bang on Dawn if it doesn't
change.
(Assignee)

Comment 14

18 years ago
This time for sure!

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago18 years ago
*Sorry for the SPAM*
Mid Air collision / bugzilla cleanup
Current bug state Resolved and NO Resolution.
Status: RESOLVED → REOPENED
marking Resolved / fixed
Status: REOPENED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED

Comment 17

18 years ago
Marking Verified - 
Status: RESOLVED → VERIFIED

Updated

18 years ago
Keywords: mozilla0.8
You need to log in before you can comment on or make changes to this bug.