Since the latest change in bug 281988, ChatZilla's been having a little problem with getting the location of its content windows. To get the error, in any very recent Firefox build, open ChatZilla and do this: /eval this.frame.contentWindow.location.href It seems /eval this.frame.contentWindow shows that the Window object is being wrapped by XPCNativeWrapper (which I'm told is perfectly correct), however the exception is not.

The problem here is that we end up in XPC_NW_NewResolve, decide we need to delegate to the unwrapped object, go to do the OBJ_DEFINE_PROPERTY() property thing. This calls into nsWindowSH::AddProperty, which throws for the "location" property. So just getting window.location fails. So the problem, it seems, is that the // All we need to do is define the property in obj if it exists in // the wrapped native's object. comment isn't what we're doing. We're trying to define the property in the wrapped native (because our AddProperty hook just passes things along here). I tried just skipping the OBJ_DEFINE_PROPERTY call, but that breaks other things... Perhaps we should consider forwarding to the other class hook first, and if that doesn't resolve anything doing what we do now? Or would that not work?

Not enough... document.location has the same issue. And the AddProperty hook on nsNodeSH does weird wrapper-preserve stuff, like I said on IRC. It really feels like we don't want to be calling AddProperty here when the property is "already there". The question is whether we can detect this last case. I guess we can do this for now if we have no better ideas...

brendan says r+a=him

Fixed.

Verified in trunk Firefox, dated 2005-05-22 23:30:05.76 BST.

I'd like to get this in now, since we know the JSClass.delProperty forwarding is not useful (delProperty, like addProperty, is a notification callback that does not actually remove the id'd property). /be

Comment on attachment 184290 [details] [diff] [review] followup fix for correct delete property bypass r+sr=jst

Comment on attachment 184290 [details] [diff] [review] followup fix for correct delete property bypass Checked in, thanks. /be