311775 - objects created with GeckoActiveXObject are broken

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[David Bradley]

The patch in bug 273953 apparently broken the IDispatch support. It's testing
for a JSObject on the tearoff that is found and bailing if one doesn't exist. I
don't believe this test is necessary. I suspect the original code was calling
FindTearOff with a value of true for needsJSobject and everything was ok. I
didn't provide that for LocateTearOff because I didn't think that was necessary.
Unfortunately I forgot the remove the subsequent test in the caller.

Many thanks to Garret Davis for tracking this down.

Comment 1

[David Bradley]

Attachment: Removes the unecessary JSObject test

Looking for reviews

Comment 2

[Garrett Davis]

(In reply to comment #1)
> Created an attachment (id=198991) [edit]
> Removes the unecessary JSObject test
> 
> Looking for reviews

I have built and tested this patch in the latest trunk and have verified that
objects created with GeckoActiveXObject are no longer broken.

Comment 3

[Brendan Eich [:brendan]]

Need to fix this for 1.8 / Firefox 1.5, for embedding opportunities/partners who
need to whitelist ActiveX plugins and ship the glue.

/be

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 198991 [details] [diff] [review]
Removes the unecessary JSObject test

sr=me, in advance of jst's, to get this moving toward branch approval.

/be

Comment 5

[Scott MacGregor]

jst, can you review this soon for us? Thanks!

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 198991 [details] [diff] [review]
Removes the unecessary JSObject test

r=jst

Comment 7

[Brendan Eich [:brendan]]

Comment on attachment 198991 [details] [diff] [review]
Removes the unecessary JSObject test

dbradley, can you land this on the trunk ASAP?	Thanks,

/be

Comment 8

[David Bradley]

I'll have some time this evening.

Comment 9

[David Bradley]

Patch checked in

Comment 10

[Brendan Eich [:brendan]]

Ok, this may not get trunk testing just by being checked in.  Can people who do
test it verify the bug, and we can get the patch plus'ed for 1.8 branch checkin?
 Thanks,

/be

Comment 11

[Asa Dotzler [:asa]]

David and Garrett, can you guys verify this? We're coming into the final days of
the release and need to have all changes in ASAP. Thanks.

Comment 12

[David Bradley]

My Cygwin install is botched, tried to get it working last night without luck.
I'm going to try again and get a good build this evening. If someone else can
verify this, that would be great, otherwise I'll try again tonight.

Comment 13

[Garrett Davis]

(In reply to comment #11)
> David and Garrett, can you guys verify this? We're coming into the final days of
> the release and need to have all changes in ASAP. Thanks.

I have verified this on MOZILLA_1_8_BRANCH. I can now call properties and
methods on GeckoActiveXObject's using firefox.exe on the 1.8 branch.

Comment 14

[Benjamin Smedberg]

I'm pretty sure this was *not* checked in on the 1.8 branch...

Comment 15

[Garrett Davis]

(In reply to comment #14)
> I'm pretty sure this was *not* checked in on the 1.8 branch...

You're correct, it wasn't checked in.  I verified the patch by applying the
patch to 1.8, building, and testing.  I'm sorry I wasn't specific.

Comment 16

[Benjamin Smedberg]

ah, ok ;-)

Comment 17

[Scott MacGregor]

I landed this on the 1.8 branch tonight for David.