> On Dec. 9, 2013, 5:49 p.m., Chris Geer wrote:
> > I ran into a problem with two of the default demo gadgets after applying 
> > this patch.
> > 
> > 1) "Open Views Demo" (upper right default gadget) - when you press the 
> > "Open Sidebar" button the sidebar will pop out but all the elements will 
> > disappear. This isn't 100% consistent but it happens 9 our of 10 times.
> > 
> > 2) "Gadget View Type" (bottom right default gadget) - this gadget should 
> > display several buttons and text boxes but instead shows "Default View". 
> > This only happens when the gadget can't detect what view it's in. I did 
> > have this show up once correctly so I"m guessing it's a timing issue 
> > somewhere but I'm not sure where.
> > 
> > Integration tests pass so we can fix these two issues I think we are good.
> 
> Stanton Sievers wrote:
>     I can reproduce #1 and will investigate the issue.  A cursory glance 
> reveals that certain callbacks in Shindig are being executed twice, once with 
> the correct data and again with incomplete data.
>     
>     I can also reproduce #2 but I don't think it's a problem related to the 
> Shindig upgrade.  If I set breakpoints in page.jsp I can see that the 
> onInitHandler is being fired before the defaultView is set to "home".  Down 
> the line in rave_opensocial#renderWidget, the render params get populated 
> with the result of stateManager.getDefaultView(), which returns 'default' and 
> not 'home'.  I can also see this if I look at the iframe url which has 
> view=default on it.  A Rave fix for this issue could be to move the call to 
> setDefaultView('home') in page.jsp into the onInitHandler callback instead of 
> outside it.
> 
> Chris Geer wrote:
>     I can also reproduce #2 on the system prior to the patch so I retract 
> that bug from this patch. I will file a JIRA on that.

Followup on #1.  The issue with the interaction between the jQuery slide effect 
and the way site holder's are handled in Shindig.  If I remove the slide effect 
from the sidebar view in rave_ui.js, I can't reproduce the issue.  What happens 
with the slide is that jQuery puts an effects-wrapper div around the gadget 
iframe for doing the slide transition.  When complete, it removes the 
effects-wrapper div by re-parenting the iframe to the parent of the 
effects-wrapper.  The trouble is this causes the iframe to reload and in some 
cases (depending on the timing) this also means that the SiteHolder gets 
disposed, which is what is turning up undefined later.

I've tried this on Rave trunk without this patch and I am able to reproduce the 
issue albeit less frequently.  In Rave trunk you can see that the sidebar 
"flickers" when it renders, which is caused by the reloading of the iframe.  
You can also see it in the Network tab of the browser dev tools in some cases 
because the feature JS and/or ifr requests will get cancelled due to the reload.


- Stanton


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


On Dec. 6, 2013, 3:42 p.m., Stanton Sievers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14841/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2013, 3:42 p.m.)
> 
> 
> Review request for rave.
> 
> 
> Bugs: RAVE-1068
>     https://issues.apache.org/jira/browse/RAVE-1068
> 
> 
> Repository: rave
> 
> 
> Description
> -------
> 
> With the release of Shindig 2.5.0-update1[1] I think it would be good to 
> upgrade Rave to a release version of Shindig.
> 
> [1] 
> http://mail-archives.apache.org/mod_mbox/www-announce/201310.mbox/%3CCAB56zCWRFeCeaqkatiN2PPxfDMvx5ncVcFfSe4-EtTo2u3vy%2Bg%40mail.gmail.com%3E
> 
> 
> Diffs
> -----
> 
>   https://svn.apache.org/repos/asf/rave/trunk/pom.xml 1541204 
>   
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core-api/src/main/java/org/apache/rave/model/ApplicationData.java
>  1541204 
>   
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/impl/ApplicationDataImpl.java
>  1541204 
>   
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaApplicationData.java
>  1541204 
>   
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaApplicationDataRepository.java
>  1541204 
>   
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/model/conversion/JpaApplicationDataConverterTest.java
>  1541204 
>   
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaApplicationDataRepositoryTest.java
>  1541204 
>   
> https://svn.apache.org/repos/asf/rave/trunk/rave-providers/rave-opensocial-provider/rave-opensocial-core/src/main/java/org/apache/rave/opensocial/service/impl/DefaultActivityStreamsService.java
>  1541204 
>   
> https://svn.apache.org/repos/asf/rave/trunk/rave-providers/rave-opensocial-provider/rave-opensocial-core/src/main/java/org/apache/rave/opensocial/service/impl/DefaultAppDataService.java
>  1541204 
>   
> https://svn.apache.org/repos/asf/rave/trunk/rave-providers/rave-opensocial-provider/rave-opensocial-core/src/main/java/org/apache/rave/opensocial/service/impl/DefaultPersonService.java
>  1541204 
>   
> https://svn.apache.org/repos/asf/rave/trunk/rave-providers/rave-opensocial-provider/rave-opensocial-core/src/test/java/org/apache/rave/opensocial/service/AppDataServiceTest.java
>  1541204 
>   
> https://svn.apache.org/repos/asf/rave/trunk/rave-providers/rave-opensocial-provider/rave-opensocial-core/src/test/java/org/apache/rave/opensocial/service/DefaultActivityStreamsServiceTest.java
>  1541204 
>   
> https://svn.apache.org/repos/asf/rave/trunk/rave-providers/rave-opensocial-provider/rave-opensocial-server/rave-shindig-resources/src/main/resources/rave.shindig.properties
>  1541204 
>   
> https://svn.apache.org/repos/asf/rave/trunk/rave-providers/rave-opensocial-provider/rave-opensocial-server/rave-shindig-resources/src/main/webapp/WEB-INF/classes/containers/default/container.js
>  1541204 
> 
> Diff: https://reviews.apache.org/r/14841/diff/
> 
> 
> Testing
> -------
> 
> Existing unit tests and integration tests pass.
> 
> 
> Thanks,
> 
> Stanton Sievers
> 
>

Reply via email to