310102 - XPCDispTearOff::Invoke() processes DISPARAMS in wrong direction

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: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.12) Gecko/20050915 Firefox/1.0.7 (ax)

IDispatch::Invoke() expects parameters to be ordered like the "__cdecl" calling
convention and not the "__pascal" calling convention.  Therefore, anyone calling
an XPCDispTearOff method will pass the params ordered from right-to-left, but
the Invoke() handler is processing the params from left-to-right.

Reproducible: Always

Steps to Reproduce:
1. Call multi-parameter method using Invoke().
2. The JavaScript method parameters will be reversed.

Actual Results:  
The parameters are reversed.

Expected Results:  
The parameters should not be reversed.

Comment 1

[Garrett Davis]

Attachment: Diff of patch reversing the processing order of DISPPARAMS

Comment 2

[David Bradley]

Comment on attachment 199083 [details] [diff] [review]
Diff of patch reversing the processing order of DISPPARAMS

r=dbradley

I was playing with this myself. Patch looks good. NOTE: this is IDispatch only
code, so no risk outside of IDispatch

Comment 3

[David Bradley]

And thanks again Garrett for stepping up.

Comment 4

[Johnny Stenback (:jst)]

Comment on attachment 199083 [details] [diff] [review]
Diff of patch reversing the processing order of DISPPARAMS

sr=jst

Comment 5

[Garrett Davis]

(In reply to comment #2)
> (From update of attachment 199083 [details] [diff] [review] [edit])
> r=dbradley
> 
> I was playing with this myself. Patch looks good. NOTE: this is IDispatch only
> code, so no risk outside of IDispatch
> 

Could you land this for me please David? I'd like to get this incorporated into my builds without having to apply the patch.

Thanks

P.S. I would like to see about getting myself permission to assign bugs to myself and/or the ability to land patches myself.

Comment 6

[Johnny Stenback (:jst)]

Fix landed on the trunk. FIXED.

Comment 7

[Johnny Stenback (:jst)]

For the record, this caused VC6 builds to fail, this change is needed in addition to landing this patch:

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/js/src/xpconnect/src&command=DIFF_FRAMESET&file=XPCDispTearOff.cpp&rev1=1.15&rev2=1.16&root=/cvsroot

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 199083 [details] [diff] [review]
Diff of patch reversing the processing order of DISPPARAMS

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