194227 - [AxPlugin] Events parameters are sent in the wrong order

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[Adam Lock]

Events are fired with their parameters in the reverse order. The mistake crept
in during checkin of bug 178542.

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=XPConnect.cpp&root=/cvsroot&subdir=mozilla/embedding/browser/activex/src/plugin&command=DIFF_FRAMESET&rev1=1.17&rev2=1.18

Patch follows to correct the issue

Comment 1

[Adam Lock]

Attachment: Patch

Simple patch grabs disp args in reverse order.

Proper ordering got lost in the churn of the event sink rewrite. The old
version got it right, the new one didn't.

Comment 2

[Brendan Eich [:brendan]]

Where are actual arguments being reversed?

/be

Comment 3

[Adam Lock]

In code like this:

<script for="someObject" event="FSCommand(command, args)">
</script>

Instead of (command, args) it is sent as (args, command) because the plugin code
that processes the args from the control is not putting them in the right order.

The reason is that IDispatch::Invoke sends its arguments in an array with the
args in reverse order, e.g.

pDispParams->rgvarg[0] = arg3;
pDispParams->rgvarg[1] = arg2;
pDispParams->rgvarg[2] = arg1;

The plugin used to have some code to put them in the proper order, but
unfortunately it was overlooked during a big code merge.

This small patch reinstates the correct behaviour.

Comment 4

[David Bradley]

Comment on attachment 115015 [details] [diff] [review]
Patch

r=dbradley

Comment 5

[Adam Lock]

Comment on attachment 115015 [details] [diff] [review]
Patch

Brendan, can you give this an sr? Thanks

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 115015 [details] [diff] [review]
Patch

I was thrown by the comment, cuz it seemed to leave open the possibility that
the bizarre reason was not by design of IDispatch.  Maybe clarify it to say
"arguments are listed backwards, intentionally, in rgvarg." 
sr=[e-mail] in any event.

/be

Comment 7

[Adam Lock]

Thanks, I'll change the comment at checkin

Comment 8

[Adam Lock]

Comment on attachment 115015 [details] [diff] [review]
Patch

Requesting 1.3 apporval. ActiveX specific, low risk, obvious fix for
regression.

Comment 9

[Asa Dotzler [:asa]]

Comment on attachment 115015 [details] [diff] [review]
Patch

a=asa (on behalf of drivers) for checkin to 1.3

Comment 10

[Adam Lock]

Fix is checked in

Comment 11

[Charles Rosendahl]

[AxPlugin] -> Ashish

Comment 12

[Ashish Bhatt]

Verified on 2003-02-21