Okay, Vidur and I looked into this a bit. Seems to be a problem with our handling of 'with'. We need to talk to some JS guys and figure out our problem.

Changing summary to more specific info.

Brendan said to bug you guys with this. We seem to be having a problem where a getProperty call inside a 'with' statement is passing us the with object, not the object we're looking for. He said this might be a regression. Thoughts?

Brendan said to bug you guys with this. We seem to be having a problem where a getProperty call inside a 'with' statement is passing us the with object, not the object we're looking for. He said this might be a regression. Thoughts?

Brendan said to bug you guys with this. We seem to be having a problem where a getProperty call inside a 'with' statement is passing us the with object, not the object we're looking for. He said this might be a regression. Thoughts?

Before I look into this, you might want to try again with the latest JS-engine, which just landed. I think that norris fixed one or two 'with' bugs and it's possible this is one of them.

Never mind what I just said about trying this with the latest version of the JS engine. I just built nglayout and the problem's still there. I can see the problem in the property code, but I'm not sure what the best way to fix it is. I'm reassigning this to myself.

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

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

Putting on ss: radar.

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

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

This bug needs help from javascript team to fix this. shaver? mlm? Need help ASAP please.

This bug did not make ss: train for this release. Removing ss: freom summary.

Scott, when can you get this done? I've got yet another bug that's related to this.

Sorry for letting this sit on the backburner for so long. I'm checking to see if mccabe can handle it - if not, I'll get to it at the beginning of next week.

I poked around a www.loa.com with a recent SeaMonkey and didn't run into any trouble. Fur, what's the problem in the property code that you mention? Do you have a testcase I can use?

This was a bug that I had asked your advice on a long time ago - someone made a deliberate change to the per-property getter code, but it was not clear why/how the change was introduced or what else might break by fixing it. As I recall, the wrong object gets passed to a per-property-getter when a property was accessed inside a with statement. The With object gets passed in instead of the With's prototype object. In this case, the getter expected a host object, so it assert-botched in JS_GET_PRIVATE(). You can try some of the other bugs which DUPLICATE the problem, e.g. bug 1221 uses http://www.microsoft.com/presspass/trial/default.htm

Setting all current Open Critical and Major to M3

This definitely still exists. Mike, Vidur and I decided you can't move in here till you fix this. If you still can reproduce it, just come to one of our machines.

I just tried a release build (Mar 2nd) on Windows NT and could not get any of the cited URLs to crash. Maybe this bug only affects the debug builds.

per leger, assigning QA contacts to all open bugs without QA contacts according to list at http://bugzilla.mozilla.org/describecomponents.cgi?product=Browser

Related bug 1322 - http://developer.netscape.com/viewsource/index_frame.html, page which used to crash but now does not crash but will not layout, think it is same issue?

Christine, is this yours?

apparently, it's only a problem with host objects (like window and document), and not reproducible from the engine alone. i'm not maintaining any html regression stuff so it'd be better if you guys held on to this one. if that's okay with you. i think this reproduces the problem: <script> with ( document ) { open(); write ( lastModified ); close(); } </script> or you could just enter the url javascript:with ( document ) { open(); write ( lastModified ); close(); } (just in case they change content of the URL above).

sounds like a problem that has been around awhile. We ought to try and nail it form M4.

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

Setting this bug to m5. Yes, I hope to get to it rsn.

whoops, actually changing the setting...

Hi guys, I reported bug 4160 which seems to be the same error. You can see this crash in the current CVS build on Wed Apr 14, Solaris 5.6 Sparc with egcs. You can see this crash by going to the URL http://developer.netscape.com/tech/java/index.html. ./viewer Warning: MOZILLA_FIVE_HOME not set. nsComponentManager: Using components dir: /project/neon/users/mo/mozilla/mozilla/solaris/dist/bin/components Going to create the event queue GFX: dpi=96 t2p=0.0666667 p2t=15 depth=8 Using '/project/neon/users/mo/mozilla/mozilla/solaris/dist/bin' as the resource: base Got thew event queue from the service Calling gdk_input with event queue Reading file... Reading file...Done Note: frameverifytree is disabled Note: verifyreflow is disabled Assertion failure: OBJ_GET_CLASS(cx, obj)->flags & JSCLASS_HAS_PRIVATE, at ../../../js/src/jsapi.c:1081 Abort (core dumped)

Fur: what change was it that caused this behaviour? I could spend some time this weekend looking at it (fiancee out of town!), since I think mccabe is too doomed for M5 already.

I don't remember the details, except that it only shows up if per-property getters are used. The code around the crash was heavily modified from a previous version (that did not have this bug), but I couldn't figure out why the changes were made.

OK, here's my fix: jsscope.h: /* * These macros are designed to decouple getter and setter from sprop, by * passing obj2 (in whose scope sprop lives, and in whose scope getter and * setter might be stored apart from sprop -- say in scope->opTable[i] for * a compressed getter or setter index i that is stored in sprop). */ -#define SPROP_GET(cx,sprop,obj,obj2,vp) ((sprop)->getter(cx,obj,sprop->id,vp)) -#define SPROP_SET(cx,sprop,obj,obj2,vp) ((sprop)->setter(cx,obj,sprop->id,vp)) +#define SPROP_GET(cx,sprop,obj,obj2,vp) ((sprop)->getter(cx,obj2,sprop->id,vp)) +#define SPROP_SET(cx,sprop,obj,obj2,vp) ((sprop)->setter(cx,obj2,sprop->id,vp)) This certainly seems like the right thing: we want to be calling the getting with the object on which the property was found, not the one on which we started the search. REGRESSION: This breaks __proto__. All other property accesses (including __parent__) seem to work correctly, though. js_LookupProperty doesn't find __proto__ in the inital obj, so it goes and finds it in the prototype, and then because of the fix it actually uses the prototype for the getter call. The prototype's __proto__ is null, so that's what it returns. I _think_ the right fix is to teach js_LookupProperty about the __proto__ magic, although maybe we should add a flag to JSScopeProperty that tells us to use the original object instead. I'll work up a diff to that effect and attach it here, if it helps.

The attached diff makes __proto__ do the right thing, and doesn't seem to trip any tests (though I'm running them manually, so I might have missed some). Can someone (fur? mccabe? brendan? norris?) comment?

Hack, cough, groan. Fur, hurry up with JS2 willya. This fix looks ok, it uses a spare flag rather than one (or three) atom compares for the ___foo__ properties. A couple of comments: 1. Doesn't __parent__ need JSPROP_ORIG_OBJ too? 2. Use (uint8) attrs rather than attrs & 0xff when storing in sprop->attrs, and make it easier on the optimizer. 3. Use js_proto_str not "__proto__" in js_InitObjectClass. Thanks for fixing this, shaver. /be

OK, sure. christine reports that my patch also breaks "Boolean.prototype.__proto__" and "Function.__proto__", which is true. __proto__ continues to work for objects created in JS, but seems to fail for those created by js_NewObject. I'm not quite sure why, but I'll take a longer look today.

OK, the new diff should be much better. ``function f() { }; f.__proto__'' works, as do ``Boolean.prototype__proto__'' and the like. (I think that JS2 should pass both obj and obj2 to getters, for the record, so that the getter implementor can choose whatever behaviour it wants.)

Not done yet (I broke ``arguments''). If you want to stick this in M5, I'll take the bug assigned to me and commit to fixing arguments ASAP. I think arguments is a lesser breakage than ``with (document)'', so this might be a forward-progress move, but it's the JS group's call.

If Tinderbox clears up by tomorrow, I plan to drop the SpiderMonkey140_BRANCH onto mozilla then. We've run the full test suite on most platforms with the so it's a known quantity and we have high confidence in it. I have a slight preference for us waiting until just after M5 to put in this "with" fix.

Shaver and I were just talking about this (again), and we have a slightly cleaner solution that will allow us to flag magic properties in their JSPropertySpec initializers. We need to do that for arguments as well as __proto__ and __parent__, probably for a few more magic ones (in jsfun.c at least). I'd like to see the fix diffs before deciding yea or nay for M5, but it is up to you guys: say "wait" if you must. /be

I'm now seeing weird property-cache interactions with __caller__ (the sprop that comes out the second time is _not_ what goes in), so I agree that we should wait until after M5. Continuing to plug away...

More thoughts: There are a lot of functions out there that depend on the current, arguably broken behaviour. Many of those are in the engine itself, and I can fix those, but I'm afraid of what will happen with all the other host objects out there in other people's code. I want to look at this some more, because ``with (new String("foo")) { print (length); }'' works as I would expect, so there's something subtle here.

This is not going to be fixed for M5, embarrassingly enough. Also, I think it's my bug now.

Hi all. I noticed that there was a change that might fix this bug. I did a CVD build today (Tue May 11) and went to this url http://developer.netscape.com/tech/java/index.html and I still got the crash. FindBookmarkShortcut: in='http://developer.netscape.com/tech/java/index.html' out='' Assertion failure: OBJ_GET_CLASS(cx, obj)->flags & JSCLASS_HAS_PRIVATE, at ../../../js/src/jsapi.c:1083 Abort (core dumped) I hope that helps

Third time's a charm. Newsgroup and email discussions have led me to believe that this fix (attached above) is the right mix of ``better behaviour'' and ``unintrusive surgery''. I haven't run the JS test suite yet, but I will do so tomorrow AM and then likely commit the change on the branch and tip. And then I will drink in celebration, for 1223 will be dead, dead, dead.

No apparent regressions in the JS test suite, so I've checked it in. We no longer crash on http://www.loa.com, so I'm marking it fixed.

Verified on 1999-05-18-09 build on Windows NT.

bulk reassigning all bugs from shaver's @netscape account to his @mozill acount

Marking Verified again, as the status changed to Resolved by the bulk reassign.