You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Henry Saputra <hs...@apache.org> on 2012/01/20 17:53:37 UTC

Review Request: SHINDIG-1678 The flash transport to support RPC for IE7 has problem with multiple gadgets in a single container

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3566/
-----------------------------------------------------------

Review request for shindig and johnfargo.


Summary
-------

See "SHINDIG-1678 The flash transport to support RPC for IE7 has problem with multiple gadgets in a single container" https://issues.apache.org/jira/browse/SHINDIG-1678 for detail description about the review.

Proposed fix is to flush the handshakes objects whenever new gadgets call gadgets.rpc.setupReceiver is called to make sure the container side register the child iframe info to the flash component.


This addresses bug SHINDIG-1678.
    https://issues.apache.org/jira/browse/SHINDIG-1678


Diffs
-----

  trunk/features/src/main/javascript/features/rpc/flash.transport.js 1233272 

Diff: https://reviews.apache.org/r/3566/diff


Testing
-------

Running multiple gadgets in common container in IE7


Thanks,

Henry


Re: Review Request: SHINDIG-1678 The flash transport to support RPC for IE7 has problem with multiple gadgets in a single container

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3566/#review8616
-----------------------------------------------------------


Hey Henry, what's the status of this review?   Committed?  Not needed?

- Dan Dumont


On Jan. 25, 2012, 7:57 p.m., Henry Saputra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3566/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2012, 7:57 p.m.)
> 
> 
> Review request for shindig and johnfargo.
> 
> 
> Description
> -------
> 
> See "SHINDIG-1678 The flash transport to support RPC for IE7 has problem with multiple gadgets in a single container" https://issues.apache.org/jira/browse/SHINDIG-1678 for detail description about the review.
> 
> Proposed fix is to flush the handshakes objects whenever new gadgets call gadgets.rpc.setupReceiver is called to make sure the container side register the child iframe info to the flash component.
> 
> 
> This addresses bug SHINDIG-1678.
>     https://issues.apache.org/jira/browse/SHINDIG-1678
> 
> 
> Diffs
> -----
> 
>   trunk/features/src/main/javascript/features/rpc/flash.transport.js 1234681 
> 
> Diff: https://reviews.apache.org/r/3566/diff/
> 
> 
> Testing
> -------
> 
> Running multiple gadgets in common container in IE7
> 
> 
> Thanks,
> 
> Henry Saputra
> 
>


Re: Review Request: SHINDIG-1678 The flash transport to support RPC for IE7 has problem with multiple gadgets in a single container

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3566/#review4617
-----------------------------------------------------------

Ship it!


LGTM

- Ryan


On 2012-01-25 19:57:26, Henry Saputra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3566/
> -----------------------------------------------------------
> 
> (Updated 2012-01-25 19:57:26)
> 
> 
> Review request for shindig and johnfargo.
> 
> 
> Summary
> -------
> 
> See "SHINDIG-1678 The flash transport to support RPC for IE7 has problem with multiple gadgets in a single container" https://issues.apache.org/jira/browse/SHINDIG-1678 for detail description about the review.
> 
> Proposed fix is to flush the handshakes objects whenever new gadgets call gadgets.rpc.setupReceiver is called to make sure the container side register the child iframe info to the flash component.
> 
> 
> This addresses bug SHINDIG-1678.
>     https://issues.apache.org/jira/browse/SHINDIG-1678
> 
> 
> Diffs
> -----
> 
>   trunk/features/src/main/javascript/features/rpc/flash.transport.js 1234681 
> 
> Diff: https://reviews.apache.org/r/3566/diff
> 
> 
> Testing
> -------
> 
> Running multiple gadgets in common container in IE7
> 
> 
> Thanks,
> 
> Henry
> 
>


Re: Review Request: SHINDIG-1678 The flash transport to support RPC for IE7 has problem with multiple gadgets in a single container

Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3566/
-----------------------------------------------------------

(Updated 2012-01-25 19:57:26.438920)


Review request for shindig and johnfargo.


Changes
-------

Need to move call to flushHandshakes() into the setup() function. The reason is that the relayLoader will not be called if relayHandle has been set.


Summary
-------

See "SHINDIG-1678 The flash transport to support RPC for IE7 has problem with multiple gadgets in a single container" https://issues.apache.org/jira/browse/SHINDIG-1678 for detail description about the review.

Proposed fix is to flush the handshakes objects whenever new gadgets call gadgets.rpc.setupReceiver is called to make sure the container side register the child iframe info to the flash component.


This addresses bug SHINDIG-1678.
    https://issues.apache.org/jira/browse/SHINDIG-1678


Diffs (updated)
-----

  trunk/features/src/main/javascript/features/rpc/flash.transport.js 1234681 

Diff: https://reviews.apache.org/r/3566/diff


Testing
-------

Running multiple gadgets in common container in IE7


Thanks,

Henry


Re: Review Request: SHINDIG-1678 The flash transport to support RPC for IE7 has problem with multiple gadgets in a single container

Posted by Henry Saputra <hs...@apache.org>.

> On 2012-01-20 17:24:54, Stanton Sievers wrote:
> > LGTM.  Thanks Henry!  To clarify, is this only an issue in IE7 or would it be an issue with any browser that uses the flash transport?  Theoretically you could force RPC to always use flash, regardless of the browser, by setting rpctx=flash in the query params.  See rpc.js#getTransport()

You were right, this is happening when using flash transport not just IE7.


> On 2012-01-20 17:24:54, Stanton Sievers wrote:
> > trunk/features/src/main/javascript/features/rpc/flash.transport.js, line 47
> > <https://reviews.apache.org/r/3566/diff/1/?file=70156#file70156line47>
> >
> >     Out of curiosity, how did you come up with this number?

It was a number I used to make sure the timeout is not too small because when container register a gadget via call to gadgets.rpc.setupReceiver() it will call flushHandshakes function. 
So the timeout interval to call flushHandshakes() function is only needed when relayHandle['setup'] somehow is not ready when flushHandshakes() is called.


- Henry


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3566/#review4496
-----------------------------------------------------------


On 2012-01-20 16:53:37, Henry Saputra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3566/
> -----------------------------------------------------------
> 
> (Updated 2012-01-20 16:53:37)
> 
> 
> Review request for shindig and johnfargo.
> 
> 
> Summary
> -------
> 
> See "SHINDIG-1678 The flash transport to support RPC for IE7 has problem with multiple gadgets in a single container" https://issues.apache.org/jira/browse/SHINDIG-1678 for detail description about the review.
> 
> Proposed fix is to flush the handshakes objects whenever new gadgets call gadgets.rpc.setupReceiver is called to make sure the container side register the child iframe info to the flash component.
> 
> 
> This addresses bug SHINDIG-1678.
>     https://issues.apache.org/jira/browse/SHINDIG-1678
> 
> 
> Diffs
> -----
> 
>   trunk/features/src/main/javascript/features/rpc/flash.transport.js 1233272 
> 
> Diff: https://reviews.apache.org/r/3566/diff
> 
> 
> Testing
> -------
> 
> Running multiple gadgets in common container in IE7
> 
> 
> Thanks,
> 
> Henry
> 
>


Re: Review Request: SHINDIG-1678 The flash transport to support RPC for IE7 has problem with multiple gadgets in a single container

Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3566/#review4496
-----------------------------------------------------------

Ship it!


LGTM.  Thanks Henry!  To clarify, is this only an issue in IE7 or would it be an issue with any browser that uses the flash transport?  Theoretically you could force RPC to always use flash, regardless of the browser, by setting rpctx=flash in the query params.  See rpc.js#getTransport()


trunk/features/src/main/javascript/features/rpc/flash.transport.js
<https://reviews.apache.org/r/3566/#comment10086>

    Out of curiosity, how did you come up with this number?


- Stanton


On 2012-01-20 16:53:37, Henry Saputra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3566/
> -----------------------------------------------------------
> 
> (Updated 2012-01-20 16:53:37)
> 
> 
> Review request for shindig and johnfargo.
> 
> 
> Summary
> -------
> 
> See "SHINDIG-1678 The flash transport to support RPC for IE7 has problem with multiple gadgets in a single container" https://issues.apache.org/jira/browse/SHINDIG-1678 for detail description about the review.
> 
> Proposed fix is to flush the handshakes objects whenever new gadgets call gadgets.rpc.setupReceiver is called to make sure the container side register the child iframe info to the flash component.
> 
> 
> This addresses bug SHINDIG-1678.
>     https://issues.apache.org/jira/browse/SHINDIG-1678
> 
> 
> Diffs
> -----
> 
>   trunk/features/src/main/javascript/features/rpc/flash.transport.js 1233272 
> 
> Diff: https://reviews.apache.org/r/3566/diff
> 
> 
> Testing
> -------
> 
> Running multiple gadgets in common container in IE7
> 
> 
> Thanks,
> 
> Henry
> 
>


Re: Review Request: SHINDIG-1678 The flash transport to support RPC for IE7 has problem with multiple gadgets in a single container

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3566/#review4535
-----------------------------------------------------------

Ship it!


LGTM

- Dan


On 2012-01-20 16:53:37, Henry Saputra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3566/
> -----------------------------------------------------------
> 
> (Updated 2012-01-20 16:53:37)
> 
> 
> Review request for shindig and johnfargo.
> 
> 
> Summary
> -------
> 
> See "SHINDIG-1678 The flash transport to support RPC for IE7 has problem with multiple gadgets in a single container" https://issues.apache.org/jira/browse/SHINDIG-1678 for detail description about the review.
> 
> Proposed fix is to flush the handshakes objects whenever new gadgets call gadgets.rpc.setupReceiver is called to make sure the container side register the child iframe info to the flash component.
> 
> 
> This addresses bug SHINDIG-1678.
>     https://issues.apache.org/jira/browse/SHINDIG-1678
> 
> 
> Diffs
> -----
> 
>   trunk/features/src/main/javascript/features/rpc/flash.transport.js 1233272 
> 
> Diff: https://reviews.apache.org/r/3566/diff
> 
> 
> Testing
> -------
> 
> Running multiple gadgets in common container in IE7
> 
> 
> Thanks,
> 
> Henry
> 
>