310097 - XPCDispConvert will not convert SAFEARRAY of VARIANT

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Garrett Davis]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.12) Gecko/20050915 Firefox/1.0.7 (ax)
Build Identifier: 

When SAFEARRAY parameters are passed into JavaScript from the Microsoft COM
world, XPCDispConvert will handle all SAFEARRAY types other than VARIANT.  An
application developer who wishes to integrate their Mozilla application with an
existing COM application will find (has found) this limiting.

Reproducible: Always

Steps to Reproduce:
1. Create a JavaScript object with one method on it (say "Fire")
2. Create a JavaScript wrapped ActiveX object using GeckoActiveXObject.
3. Pass this JavaScript to the ActiveX object as a parameter to a method that
accepts an IDispatch. This will create an IDispatch tearoff object that wraps
the JavaScript object.
4. Call the "Fire" method on the tearoff object using IDispatch::Invoke passing
a SAFEARRAY of VARIANT as one of the parameters.
5. The call will always fail with an HRESULT of 0x80040005.

Actual Results:  
Call into tearoff object always fails when one parameter is a SAFEARRAY of VARIANT.

Expected Results:  
Call into tearoff object should succeed if SAFEARRAY contains valid VARIANT data.

Comment 1

[Garrett Davis]

Attachment: Changes to XPCDispConvert::COMArrayToJSArray()

I cannot test this on the trunk because the COM-to-JS bridge (not XPCOM) is
broken right now.  I tested this with a 1.8a5 build where COM
(GeckoActiveXObject) was working.

Comment 2

[Adam Guthrie]

Garret, you need to make a diff file of your patch (usually something like `cvs
diff -p -u -8` works fine) and then request review from someone on your patch.
([e-mail] would be the appropriate reviewer in this case.)

Comment 3

[David Bradley]

Attachment: Diff version of previous patch

This is the diff version of the previous patch.

Thanks again Garret.

Comment 4

[David Bradley]

Comment on attachment 198995 [details] [diff] [review]
Diff version of  previous patch

r=dbradley

I was expecting array of variants to be returned in the same fashion as other
types, using byref.

Comment 5

[David Bradley]

Oh, also Garret, apply the patch in bug 311775 to a trunk build and test this
and report back. I'll try and get these checked in all together.

Comment 6

[Garrett Davis]

(In reply to comment #5)
> Oh, also Garret, apply the patch in bug 311775 to a trunk build and test this
> and report back. I'll try and get these checked in all together.

I have tested this bug along with 311775 and have verified that both bugs are
indeed fixed. Does anyone need to create a test case for these bugs? If so, let
me know how I can help.

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 198995 [details] [diff] [review]
Diff version of  previous patch

sr=jst

Comment 8

[Garrett Davis]

(In reply to comment #5)
> Oh, also Garret, apply the patch in bug 311775 to a trunk build and test this
> and report back. I'll try and get these checked in all together.

Could you land this on the trunk please David? I'd like to start getting this incorporated into my builds without having to apply the patch all the time.

Thanks

Comment 9

[Johnny Stenback (:jst)]

I just landed this on the trunk. FIXED.

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 198995 [details] [diff] [review]
Diff version of  previous patch

Maybe for 1.8 branch (Firefox 2), not 1.8.0.x