rave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stanton Sievers" <siever...@gmail.com>
Subject Re: Review Request 14841: Upgrade to Shindig 2.5.0-update1
Date Mon, 09 Dec 2013 21:49:23 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message