You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Jason Chiang <ja...@gmail.com> on 2011/10/21 18:26:52 UTC

Review Request: Media gadgets does not work on IE8

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

Review request for shindig, Ryan Baxter and Dan Dumont.


Summary
-------

This patch fix the IE8 rendering issue on both Sample media items gadget and Sample media items with openGadget API gadget.
All changes are on the gadget code.
Link to JIRA, https://issues.apache.org/jira/browse/SHINDIG-1648


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


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media-openGadgets/MediaUIOpenGadgets.js 1187403 
  http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media-openGadgets/Social.js 1187403 
  http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media/MediaUI.js 1187403 

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


Testing
-------

Test done on the sample container testing with both Firefox and IE8 


Thanks,

Jason


Re: Review Request: Media gadgets does not work on IE8

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

Ship it!


 Committed revision 1187546.
There seems to be a bunch of duplicate code between the gadgets, maybe you should think about abstracting the comment code out so you dont have to make changes in 2 places.

- Ryan


On 2011-10-21 16:26:52, Jason Chiang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2527/
> -----------------------------------------------------------
> 
> (Updated 2011-10-21 16:26:52)
> 
> 
> Review request for shindig, Ryan Baxter and Dan Dumont.
> 
> 
> Summary
> -------
> 
> This patch fix the IE8 rendering issue on both Sample media items gadget and Sample media items with openGadget API gadget.
> All changes are on the gadget code.
> Link to JIRA, https://issues.apache.org/jira/browse/SHINDIG-1648
> 
> 
> This addresses bug SHINDIG-1648.
>     https://issues.apache.org/jira/browse/SHINDIG-1648
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media-openGadgets/MediaUIOpenGadgets.js 1187403 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media-openGadgets/Social.js 1187403 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media/MediaUI.js 1187403 
> 
> Diff: https://reviews.apache.org/r/2527/diff
> 
> 
> Testing
> -------
> 
> Test done on the sample container testing with both Firefox and IE8 
> 
> 
> Thanks,
> 
> Jason
> 
>


Re: Review Request: Media gadgets does not work on IE8

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

Ship it!


LGTM

- Dan


On 2011-10-21 16:26:52, Jason Chiang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2527/
> -----------------------------------------------------------
> 
> (Updated 2011-10-21 16:26:52)
> 
> 
> Review request for shindig, Ryan Baxter and Dan Dumont.
> 
> 
> Summary
> -------
> 
> This patch fix the IE8 rendering issue on both Sample media items gadget and Sample media items with openGadget API gadget.
> All changes are on the gadget code.
> Link to JIRA, https://issues.apache.org/jira/browse/SHINDIG-1648
> 
> 
> This addresses bug SHINDIG-1648.
>     https://issues.apache.org/jira/browse/SHINDIG-1648
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media-openGadgets/MediaUIOpenGadgets.js 1187403 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media-openGadgets/Social.js 1187403 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media/MediaUI.js 1187403 
> 
> Diff: https://reviews.apache.org/r/2527/diff
> 
> 
> Testing
> -------
> 
> Test done on the sample container testing with both Firefox and IE8 
> 
> 
> Thanks,
> 
> Jason
> 
>


Re: Review Request: Media gadgets does not work on IE8

Posted by Jason Chiang <ja...@gmail.com>.

> On 2011-10-21 17:00:43, Dan Dumont wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media-openGadgets/MediaUIOpenGadgets.js, line 120
> > <https://reviews.apache.org/r/2527/diff/1/?file=52116#file52116line120>
> >
> >     pixel dimensions instead of percentage?

right, set both the width and height to 100 pixel for the thumbnail image.


> On 2011-10-21 17:00:43, Dan Dumont wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media/MediaUI.js, line 142
> > <https://reviews.apache.org/r/2527/diff/1/?file=52118#file52118line142>
> >
> >     I would think you still want width:'100%' and height: '100%' here.
> >     
> >     That json bag are the attributes you want set on a node...   so you'll want <img height="100%" width="100%"... > right?
> >     
> >     Or did you mean to change this to pixel dimensions instead of percentage?

see above


- Jason


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


On 2011-10-21 16:26:52, Jason Chiang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2527/
> -----------------------------------------------------------
> 
> (Updated 2011-10-21 16:26:52)
> 
> 
> Review request for shindig, Ryan Baxter and Dan Dumont.
> 
> 
> Summary
> -------
> 
> This patch fix the IE8 rendering issue on both Sample media items gadget and Sample media items with openGadget API gadget.
> All changes are on the gadget code.
> Link to JIRA, https://issues.apache.org/jira/browse/SHINDIG-1648
> 
> 
> This addresses bug SHINDIG-1648.
>     https://issues.apache.org/jira/browse/SHINDIG-1648
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media-openGadgets/MediaUIOpenGadgets.js 1187403 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media-openGadgets/Social.js 1187403 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media/MediaUI.js 1187403 
> 
> Diff: https://reviews.apache.org/r/2527/diff
> 
> 
> Testing
> -------
> 
> Test done on the sample container testing with both Firefox and IE8 
> 
> 
> Thanks,
> 
> Jason
> 
>


Re: Review Request: Media gadgets does not work on IE8

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



http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media-openGadgets/MediaUIOpenGadgets.js
<https://reviews.apache.org/r/2527/#comment6188>

    pixel dimensions instead of percentage?



http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media/MediaUI.js
<https://reviews.apache.org/r/2527/#comment6187>

    I would think you still want width:'100%' and height: '100%' here.
    
    That json bag are the attributes you want set on a node...   so you'll want <img height="100%" width="100%"... > right?
    
    Or did you mean to change this to pixel dimensions instead of percentage?


- Dan


On 2011-10-21 16:26:52, Jason Chiang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2527/
> -----------------------------------------------------------
> 
> (Updated 2011-10-21 16:26:52)
> 
> 
> Review request for shindig, Ryan Baxter and Dan Dumont.
> 
> 
> Summary
> -------
> 
> This patch fix the IE8 rendering issue on both Sample media items gadget and Sample media items with openGadget API gadget.
> All changes are on the gadget code.
> Link to JIRA, https://issues.apache.org/jira/browse/SHINDIG-1648
> 
> 
> This addresses bug SHINDIG-1648.
>     https://issues.apache.org/jira/browse/SHINDIG-1648
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media-openGadgets/MediaUIOpenGadgets.js 1187403 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media-openGadgets/Social.js 1187403 
>   http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/media/MediaUI.js 1187403 
> 
> Diff: https://reviews.apache.org/r/2527/diff
> 
> 
> Testing
> -------
> 
> Test done on the sample container testing with both Firefox and IE8 
> 
> 
> Thanks,
> 
> Jason
> 
>