Bob Clary [:bc] (inactive) Reporter
	Description • 17 years ago

ecma_2/Statements/forin-002.js
ecma_3/Object/class-001.js

fail in the browser since the String value of a XOW now shows the wrapper and not the underlying object.

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 1 • 17 years ago

The problem here was that eval was wrapping too many things. I also changed things so that the function wrapper for eval is constant, allowing window.eval === window.eval.

	Jeff Walden [:Waldo]
	Comment 2 • 17 years ago

(In reply to comment #1)
> I also changed things so that the function wrapper for eval is constant,
> allowing window.eval === window.eval.

Note: this should have a test somewhere.

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 3 • 17 years ago

It's covered by one of the broken tests here (that's how I knew that I had to do this).

	Johnny Stenback (:jst)
	Comment 4 • 17 years ago

Comment on attachment 275858 [details] [diff] [review]
Fix

- In WrapSameOriginProp():

+  JSObject *wrappedObj = JSVAL_TO_OBJECT(*vp);
+  if (JS_ObjectIsFunction(cx, wrappedObj) &&
+      JS_GetFunctionNative(cx, reinterpret_cast<JSFunction *>
+                                               (JS_GetPrivate(cx, wrappedObj))) ==
+      XPCWrapper::sEvalNative) {
+    return XPC_XOW_WrapFunction(cx, outerObj, wrappedObj, vp);
+  }
+
+  const char *name = JS_GET_CLASS(cx, wrappedObj)->name;
+  if (XPC_XOW_ClassNeedsXOW(name)) {
+    return XPC_XOW_WrapObject(cx, GetGlobalObject(cx, outerObj), vp);
+  }

Might be faster if you reorder this code to do the XPC_XOW_ClassNeedsXOW() call before the function fu, as window objects etc are probably more common here than functions. Either way works tho...

- In XPCWrapper::NewResolve():

+  // Hack alert: we only do this for same-origin calls on XOWs: we want
+  // to preserve 'eval' on the wrapper itself since that code needs
+  // eval to be wrapped.

Um, what code needs eval to be wrapped? :)

r+sr=jst

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 5 • 17 years ago

Fix checked into trunk.

	Bob Clary [:bc] (inactive) Reporter
	Comment 7 • 17 years ago

<http://test.bclary.com/tests/mozilla.org/js/js-test-driver-standards.html?test=ecma_3/Object/class-001.js;language=type;text/javascript> still fails with 

FAILED! expected: [reported from test()] Expected value 'Window', Actual value 'XPCCrossOriginWrapper' 

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 8 • 17 years ago

The test explicitly calls Object.prototype.toString.call(window). I can't really fix that.

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 9 • 17 years ago

In fact, I don't want to fix that. Bob, can you change the test to use |this + ''| or something? Marking this as fixed again.

	Bob Clary [:bc] (inactive) Reporter
	Comment 10 • 17 years ago

The test is explicitly calling Object.prototype.toString. Changing that would affect the other types where the default toString would differ. How about I check for the wrapper and just skip it ala

--- ecma_3/Object/class-001.js  26 May 2007 00:19:30 -0000      1.6
+++ ecma_3/Object/class-001.js  5 Oct 2007 20:34:47 -0000
@@ -59,7 +59,14 @@
 status = 'the global object';
 actual = getJSClass(this);
 expect = GLOBAL;
-addThis();
+if (expect == 'Window' && actual == 'XPCCrossOriginWrapper')
+{
+  print('Skipping global object due to XPCCrossOriginWrapper. See bug 390946');
+}
+else
+{
+  addThis();
+}

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 11 • 17 years ago

That seems fair to me.

	Bob Clary [:bc] (inactive) Reporter
	Comment 12 • 17 years ago

done deal.
/cvsroot/mozilla/js/tests/ecma_3/Object/class-001.js,v  <--  class-001.js
new revision: 1.7; previous revision: 1.6

	Bob Clary [:bc] (inactive) Reporter
	Comment 13 • 17 years ago

verified fixed.