327571 - "ASSERTION: nsDOMClassInfo::GetProperty Don't call me!" when getting an input element by name through the prototype of another dom node

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Jesse Ruderman]

###!!! ASSERTION: nsDOMClassInfo::GetProperty Don't call me!: 'Error', file /Users/admin/trunk/mozilla/dom/src/base/nsDOMClassInfo.cpp, line 3327

Comment 1

[Jesse Ruderman]

Attachment: testcase

Comment 2

[Jesse Ruderman]

I've been hitting this assertion a lot when playing with bug 326633, so I'd appreciate a fix.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

jst and I talked this over and there's no easy and performant way to fix this bug for real. Currently, XPCWrappedNative uses a different class getter based on whether or not a given class has the flag WantGetProperty(). This is to avoid the test for whether or not |this| has that flag. Unfortunately, mutable prototypes make this optimization violate the assertions in nsDOMClassInfo. Fixing this in any way in XPConnect will likely break other places that don't depend on this behavior (i.e., any such fix would not be specific enough) and would also hurt performance in a case where I don't think we care _too_ much what we do, as long as we don't crash. I have a patch to turn the assertions into warnings, since this seems like a valid case that will hit them.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Make this case not assert

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 212665 [details] [diff] [review]
Make this case not assert

rs=me.  We need a bug to make __proto__ readonly as well as permanent for DOM stuff, if not for XPConnect stuff.

/be

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 212665 [details] [diff] [review]
Make this case not assert

r=jst

People rely on changing DOM object prototypes, some of the JS libraries out there that implement outerHTML and other IE:ism's do just that...

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Do they really set a new __proto__ to do that? Don't they just set properties on the existing prototypes.

Comment 9

[Johnny Stenback (:jst)]

(In reply to comment #8)
> Do they really set a new __proto__ to do that? Don't they just set properties
> on the existing prototypes.
> 


Sorry, I was replying based on me misunderstanding something mrbkap told me in person. No, generally people add/remove from the existing protos, I've very rarely, if ever, seen real code out there that replaces a DOM object's __proto__ property.

Comment 10

[Jesse Ruderman]

I can verify that this is fixed on Mac, but has a bug been filed based on comment 5?

Comment 11

[Jesse Ruderman]

Crashtest checked in.