Shindig
  1. Shindig
  2. SHINDIG-1763

Container lang and country setting don't take effect

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5.0, 3.0.0
    • Fix Version/s: 2.5.0-beta2
    • Component/s: Javascript
    • Labels:
      None
    1. langCountry.patch
      4 kB
      sun qiao yun
    2. langCountry.patch
      0.9 kB
      sun qiao yun

      Activity

      Hide
      jiraposter@reviews.apache.org added a comment -

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

      (Updated 2012-04-27 02:22:36.737977)

      Review request for Ryan Baxter and Dan Dumont.

      Summary
      -------

      If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored.
      For example: the ifr is like :
      http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0

      They should be substituted by the browser's setting in client side.

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

      Diffs


      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177

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

      Testing
      -------

      Thanks,

      qiaoyun

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/ ----------------------------------------------------------- (Updated 2012-04-27 02:22:36.737977) Review request for Ryan Baxter and Dan Dumont. Summary ------- If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored. For example: the ifr is like : http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0 They should be substituted by the browser's setting in client side. This addresses bug SHINDIG-1763 . https://issues.apache.org/jira/browse/SHINDIG-1763 Diffs http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177 Diff: https://reviews.apache.org/r/4906/diff Testing ------- Thanks, qiaoyun
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      Please use single quotes for Strings in javascript.

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js
      <https://reviews.apache.org/r/4906/#comment16116>

      I saw some posts that say on IE you are supposed to use window.navigator.userLanguage. The userLanguage property gets the language from the Regional Settings in the control panel on windows as opposed to the browser language. I assume navigator.language also works on IE, have you tested this? If both properties work on IE, I guess it depends on whether we want to be consistent across browsers or not, I think consistency is best.

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js
      <https://reviews.apache.org/r/4906/#comment16115>

      I think some helper functions to get the lang and country make sense here. I would suggest putting them in the util.js container file.

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js
      <https://reviews.apache.org/r/4906/#comment16114>

      won't this override the lang and country property if they were already set by the server? If they are already set I would rather us not do this logic and use what the server told us to use.

      • Ryan

      On 2012-04-27 02:22:36, qiaoyun sun wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4906/

      -----------------------------------------------------------

      (Updated 2012-04-27 02:22:36)

      Review request for Ryan Baxter and Dan Dumont.

      Summary

      -------

      If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored.

      For example: the ifr is like :

      http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0

      They should be substituted by the browser's setting in client side.

      This addresses bug SHINDIG-1763.

      https://issues.apache.org/jira/browse/SHINDIG-1763

      Diffs

      -----

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177

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

      Testing

      -------

      Thanks,

      qiaoyun

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/#review7299 ----------------------------------------------------------- Please use single quotes for Strings in javascript. http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js < https://reviews.apache.org/r/4906/#comment16116 > I saw some posts that say on IE you are supposed to use window.navigator.userLanguage. The userLanguage property gets the language from the Regional Settings in the control panel on windows as opposed to the browser language. I assume navigator.language also works on IE, have you tested this? If both properties work on IE, I guess it depends on whether we want to be consistent across browsers or not, I think consistency is best. http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js < https://reviews.apache.org/r/4906/#comment16115 > I think some helper functions to get the lang and country make sense here. I would suggest putting them in the util.js container file. http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js < https://reviews.apache.org/r/4906/#comment16114 > won't this override the lang and country property if they were already set by the server? If they are already set I would rather us not do this logic and use what the server told us to use. Ryan On 2012-04-27 02:22:36, qiaoyun sun wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/ ----------------------------------------------------------- (Updated 2012-04-27 02:22:36) Review request for Ryan Baxter and Dan Dumont. Summary ------- If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored. For example: the ifr is like : http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0 They should be substituted by the browser's setting in client side. This addresses bug SHINDIG-1763 . https://issues.apache.org/jira/browse/SHINDIG-1763 Diffs ----- http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177 Diff: https://reviews.apache.org/r/4906/diff Testing ------- Thanks, qiaoyun
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      I think this review needs to step back and look at what that setting means in general, because it's more than just setting the language and is not a very straight-forward concept.
      I'd really just recommend that you set that setting to false instead unless you have some need to flip this switch...

      In any case, see my inline comments.

      Also, please add ssievers to the review.

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js
      <https://reviews.apache.org/r/4906/#comment16117>

      Please do not do this here. There are functions defined in the common container's service to get the language for a gadget already.

      See: osapi.container.Service.prototype.getLanguage and getLocale

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js
      <https://reviews.apache.org/r/4906/#comment16118>

      I think this code needs to be safer about how it replaces these query parameters. Perhaps check to see if they are templated first and only replace them if there are not real values present.

      • Dan

      On 2012-04-27 02:22:36, qiaoyun sun wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4906/

      -----------------------------------------------------------

      (Updated 2012-04-27 02:22:36)

      Review request for Ryan Baxter and Dan Dumont.

      Summary

      -------

      If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored.

      For example: the ifr is like :

      http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0

      They should be substituted by the browser's setting in client side.

      This addresses bug SHINDIG-1763.

      https://issues.apache.org/jira/browse/SHINDIG-1763

      Diffs

      -----

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177

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

      Testing

      -------

      Thanks,

      qiaoyun

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/#review7301 ----------------------------------------------------------- I think this review needs to step back and look at what that setting means in general, because it's more than just setting the language and is not a very straight-forward concept. I'd really just recommend that you set that setting to false instead unless you have some need to flip this switch... In any case, see my inline comments. Also, please add ssievers to the review. http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js < https://reviews.apache.org/r/4906/#comment16117 > Please do not do this here. There are functions defined in the common container's service to get the language for a gadget already. See: osapi.container.Service.prototype.getLanguage and getLocale http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js < https://reviews.apache.org/r/4906/#comment16118 > I think this code needs to be safer about how it replaces these query parameters. Perhaps check to see if they are templated first and only replace them if there are not real values present. Dan On 2012-04-27 02:22:36, qiaoyun sun wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/ ----------------------------------------------------------- (Updated 2012-04-27 02:22:36) Review request for Ryan Baxter and Dan Dumont. Summary ------- If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored. For example: the ifr is like : http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0 They should be substituted by the browser's setting in client side. This addresses bug SHINDIG-1763 . https://issues.apache.org/jira/browse/SHINDIG-1763 Diffs ----- http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177 Diff: https://reviews.apache.org/r/4906/diff Testing ------- Thanks, qiaoyun
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      (Updated 2012-05-07 09:09:55.012671)

      Review request for Ryan Baxter, Dan Dumont and Stanton Sievers.

      Changes
      -------

      Thanks Ryan and Dan, have updated the patch according to your comments.

      Summary
      -------

      If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored.
      For example: the ifr is like :
      http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0

      They should be substituted by the browser's setting in client side.

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

      Diffs (updated)


      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177

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

      Testing
      -------

      Thanks,

      qiaoyun

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/ ----------------------------------------------------------- (Updated 2012-05-07 09:09:55.012671) Review request for Ryan Baxter, Dan Dumont and Stanton Sievers. Changes ------- Thanks Ryan and Dan, have updated the patch according to your comments. Summary ------- If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored. For example: the ifr is like : http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0 They should be substituted by the browser's setting in client side. This addresses bug SHINDIG-1763 . https://issues.apache.org/jira/browse/SHINDIG-1763 Diffs (updated) http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177 Diff: https://reviews.apache.org/r/4906/diff Testing ------- Thanks, qiaoyun
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js
      <https://reviews.apache.org/r/4906/#comment16840>

      While there's no public API to get the Service instance object, you can still get it from the site.

      self.site_.service_, I think will do it. Normally I don't like accessing private variables on an object, but holder and site are so tightly coupled anyway that I'm fine with it. Dan and Ryan may have an opinion here as well.

      • Stanton

      On 2012-05-07 09:09:55, qiaoyun sun wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4906/

      -----------------------------------------------------------

      (Updated 2012-05-07 09:09:55)

      Review request for Ryan Baxter, Dan Dumont and Stanton Sievers.

      Summary

      -------

      If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored.

      For example: the ifr is like :

      http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0

      They should be substituted by the browser's setting in client side.

      This addresses bug SHINDIG-1763.

      https://issues.apache.org/jira/browse/SHINDIG-1763

      Diffs

      -----

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177

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

      Testing

      -------

      Thanks,

      qiaoyun

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/#review7629 ----------------------------------------------------------- http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js < https://reviews.apache.org/r/4906/#comment16840 > While there's no public API to get the Service instance object, you can still get it from the site. self.site_.service_, I think will do it. Normally I don't like accessing private variables on an object, but holder and site are so tightly coupled anyway that I'm fine with it. Dan and Ryan may have an opinion here as well. Stanton On 2012-05-07 09:09:55, qiaoyun sun wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/ ----------------------------------------------------------- (Updated 2012-05-07 09:09:55) Review request for Ryan Baxter, Dan Dumont and Stanton Sievers. Summary ------- If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored. For example: the ifr is like : http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0 They should be substituted by the browser's setting in client side. This addresses bug SHINDIG-1763 . https://issues.apache.org/jira/browse/SHINDIG-1763 Diffs ----- http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177 Diff: https://reviews.apache.org/r/4906/diff Testing ------- Thanks, qiaoyun
      Hide
      jiraposter@reviews.apache.org added a comment -

      On 2012-05-07 12:03:44, Stanton Sievers wrote:

      > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js, line 269

      > <https://reviews.apache.org/r/4906/diff/2/?file=107252#file107252line269>

      >

      > While there's no public API to get the Service instance object, you can still get it from the site.

      >

      > self.site_.service_, I think will do it. Normally I don't like accessing private variables on an object, but holder and site are so tightly coupled anyway that I'm fine with it. Dan and Ryan may have an opinion here as well.

      I agree.

      • Dan

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

      On 2012-05-07 09:09:55, qiaoyun sun wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4906/

      -----------------------------------------------------------

      (Updated 2012-05-07 09:09:55)

      Review request for Ryan Baxter, Dan Dumont and Stanton Sievers.

      Summary

      -------

      If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored.

      For example: the ifr is like :

      http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0

      They should be substituted by the browser's setting in client side.

      This addresses bug SHINDIG-1763.

      https://issues.apache.org/jira/browse/SHINDIG-1763

      Diffs

      -----

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177

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

      Testing

      -------

      Thanks,

      qiaoyun

      Show
      jiraposter@reviews.apache.org added a comment - On 2012-05-07 12:03:44, Stanton Sievers wrote: > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js , line 269 > < https://reviews.apache.org/r/4906/diff/2/?file=107252#file107252line269 > > > While there's no public API to get the Service instance object, you can still get it from the site. > > self.site_.service_, I think will do it. Normally I don't like accessing private variables on an object, but holder and site are so tightly coupled anyway that I'm fine with it. Dan and Ryan may have an opinion here as well. I agree. Dan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/#review7629 ----------------------------------------------------------- On 2012-05-07 09:09:55, qiaoyun sun wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/ ----------------------------------------------------------- (Updated 2012-05-07 09:09:55) Review request for Ryan Baxter, Dan Dumont and Stanton Sievers. Summary ------- If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored. For example: the ifr is like : http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0 They should be substituted by the browser's setting in client side. This addresses bug SHINDIG-1763 . https://issues.apache.org/jira/browse/SHINDIG-1763 Diffs ----- http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177 Diff: https://reviews.apache.org/r/4906/diff Testing ------- Thanks, qiaoyun
      Hide
      jiraposter@reviews.apache.org added a comment -

      On 2012-05-07 12:03:44, Stanton Sievers wrote:

      > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js, line 269

      > <https://reviews.apache.org/r/4906/diff/2/?file=107252#file107252line269>

      >

      > While there's no public API to get the Service instance object, you can still get it from the site.

      >

      > self.site_.service_, I think will do it. Normally I don't like accessing private variables on an object, but holder and site are so tightly coupled anyway that I'm fine with it. Dan and Ryan may have an opinion here as well.

      Dan Dumont wrote:

      I agree.

      Me too

      • Ryan

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

      On 2012-05-07 09:09:55, qiaoyun sun wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4906/

      -----------------------------------------------------------

      (Updated 2012-05-07 09:09:55)

      Review request for Ryan Baxter, Dan Dumont and Stanton Sievers.

      Summary

      -------

      If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored.

      For example: the ifr is like :

      http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0

      They should be substituted by the browser's setting in client side.

      This addresses bug SHINDIG-1763.

      https://issues.apache.org/jira/browse/SHINDIG-1763

      Diffs

      -----

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177

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

      Testing

      -------

      Thanks,

      qiaoyun

      Show
      jiraposter@reviews.apache.org added a comment - On 2012-05-07 12:03:44, Stanton Sievers wrote: > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js , line 269 > < https://reviews.apache.org/r/4906/diff/2/?file=107252#file107252line269 > > > While there's no public API to get the Service instance object, you can still get it from the site. > > self.site_.service_, I think will do it. Normally I don't like accessing private variables on an object, but holder and site are so tightly coupled anyway that I'm fine with it. Dan and Ryan may have an opinion here as well. Dan Dumont wrote: I agree. Me too Ryan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/#review7629 ----------------------------------------------------------- On 2012-05-07 09:09:55, qiaoyun sun wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/ ----------------------------------------------------------- (Updated 2012-05-07 09:09:55) Review request for Ryan Baxter, Dan Dumont and Stanton Sievers. Summary ------- If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored. For example: the ifr is like : http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0 They should be substituted by the browser's setting in client side. This addresses bug SHINDIG-1763 . https://issues.apache.org/jira/browse/SHINDIG-1763 Diffs ----- http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177 Diff: https://reviews.apache.org/r/4906/diff Testing ------- Thanks, qiaoyun
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      (Updated 2012-05-08 02:17:02.558232)

      Review request for Ryan Baxter, Dan Dumont and Stanton Sievers.

      Changes
      -------

      Thanks! updated the patch, if it is OK, could you please help to commit? Thanks a lot!

      Summary
      -------

      If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored.
      For example: the ifr is like :
      http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0

      They should be substituted by the browser's setting in client side.

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

      Diffs (updated)


      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177

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

      Testing
      -------

      Thanks,

      qiaoyun

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/ ----------------------------------------------------------- (Updated 2012-05-08 02:17:02.558232) Review request for Ryan Baxter, Dan Dumont and Stanton Sievers. Changes ------- Thanks! updated the patch, if it is OK, could you please help to commit? Thanks a lot! Summary ------- If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored. For example: the ifr is like : http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0 They should be substituted by the browser's setting in client side. This addresses bug SHINDIG-1763 . https://issues.apache.org/jira/browse/SHINDIG-1763 Diffs (updated) http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177 Diff: https://reviews.apache.org/r/4906/diff Testing ------- Thanks, qiaoyun
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      Ship it!

      LGTM. Please add "shindig" as a group reviewer. Also ensure that the patch has been uploaded to the JIRA and that you've granted ASF license for inclusion.

      • Stanton

      On 2012-05-08 02:17:02, qiaoyun sun wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4906/

      -----------------------------------------------------------

      (Updated 2012-05-08 02:17:02)

      Review request for Ryan Baxter, Dan Dumont and Stanton Sievers.

      Summary

      -------

      If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored.

      For example: the ifr is like :

      http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0

      They should be substituted by the browser's setting in client side.

      This addresses bug SHINDIG-1763.

      https://issues.apache.org/jira/browse/SHINDIG-1763

      Diffs

      -----

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177

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

      Testing

      -------

      Thanks,

      qiaoyun

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/#review7676 ----------------------------------------------------------- Ship it! LGTM. Please add "shindig" as a group reviewer. Also ensure that the patch has been uploaded to the JIRA and that you've granted ASF license for inclusion. Stanton On 2012-05-08 02:17:02, qiaoyun sun wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/ ----------------------------------------------------------- (Updated 2012-05-08 02:17:02) Review request for Ryan Baxter, Dan Dumont and Stanton Sievers. Summary ------- If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored. For example: the ifr is like : http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0 They should be substituted by the browser's setting in client side. This addresses bug SHINDIG-1763 . https://issues.apache.org/jira/browse/SHINDIG-1763 Diffs ----- http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177 Diff: https://reviews.apache.org/r/4906/diff Testing ------- Thanks, qiaoyun
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      (Updated 2012-05-09 01:57:56.136560)

      Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers.

      Summary
      -------

      If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored.
      For example: the ifr is like :
      http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0

      They should be substituted by the browser's setting in client side.

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

      Diffs


      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177

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

      Testing
      -------

      Thanks,

      qiaoyun

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/ ----------------------------------------------------------- (Updated 2012-05-09 01:57:56.136560) Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers. Summary ------- If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored. For example: the ifr is like : http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0 They should be substituted by the browser's setting in client side. This addresses bug SHINDIG-1763 . https://issues.apache.org/jira/browse/SHINDIG-1763 Diffs http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177 Diff: https://reviews.apache.org/r/4906/diff Testing ------- Thanks, qiaoyun
      Hide
      sun qiao yun added a comment -

      updated the patch according to your suggestion and submit the patch.

      Show
      sun qiao yun added a comment - updated the patch according to your suggestion and submit the patch.
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      I am seeing JS Unit test failures in GadgetHolderTest.testRenderWithoutRenderParams and GadgetHolderTest.testRenderWithRenderRequests after applying this patch.

      • Ryan

      On 2012-05-09 01:57:56, qiaoyun sun wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4906/

      -----------------------------------------------------------

      (Updated 2012-05-09 01:57:56)

      Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers.

      Summary

      -------

      If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored.

      For example: the ifr is like :

      http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0

      They should be substituted by the browser's setting in client side.

      This addresses bug SHINDIG-1763.

      https://issues.apache.org/jira/browse/SHINDIG-1763

      Diffs

      -----

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177

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

      Testing

      -------

      Thanks,

      qiaoyun

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/#review7733 ----------------------------------------------------------- I am seeing JS Unit test failures in GadgetHolderTest.testRenderWithoutRenderParams and GadgetHolderTest.testRenderWithRenderRequests after applying this patch. Ryan On 2012-05-09 01:57:56, qiaoyun sun wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/ ----------------------------------------------------------- (Updated 2012-05-09 01:57:56) Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers. Summary ------- If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored. For example: the ifr is like : http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0 They should be substituted by the browser's setting in client side. This addresses bug SHINDIG-1763 . https://issues.apache.org/jira/browse/SHINDIG-1763 Diffs ----- http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177 Diff: https://reviews.apache.org/r/4906/diff Testing ------- Thanks, qiaoyun
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      (Updated 2012-05-10 06:49:27.713021)

      Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers.

      Changes
      -------

      change the corresponding js unit test

      Summary
      -------

      If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored.
      For example: the ifr is like :
      http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0

      They should be substituted by the browser's setting in client side.

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

      Diffs (updated)


      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177
      http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1331168

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

      Testing
      -------

      Thanks,

      qiaoyun

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/ ----------------------------------------------------------- (Updated 2012-05-10 06:49:27.713021) Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers. Changes ------- change the corresponding js unit test Summary ------- If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored. For example: the ifr is like : http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0 They should be substituted by the browser's setting in client side. This addresses bug SHINDIG-1763 . https://issues.apache.org/jira/browse/SHINDIG-1763 Diffs (updated) http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177 http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1331168 Diff: https://reviews.apache.org/r/4906/diff Testing ------- Thanks, qiaoyun
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      Ship it!

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js
      <https://reviews.apache.org/r/4906/#comment17071>

      Small nit on trailing whitespace.

      • Stanton

      On 2012-05-10 06:49:27, qiaoyun sun wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4906/

      -----------------------------------------------------------

      (Updated 2012-05-10 06:49:27)

      Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers.

      Summary

      -------

      If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored.

      For example: the ifr is like :

      http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0

      They should be substituted by the browser's setting in client side.

      This addresses bug SHINDIG-1763.

      https://issues.apache.org/jira/browse/SHINDIG-1763

      Diffs

      -----

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1331168

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

      Testing

      -------

      Thanks,

      qiaoyun

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/#review7762 ----------------------------------------------------------- Ship it! http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js < https://reviews.apache.org/r/4906/#comment17071 > Small nit on trailing whitespace. Stanton On 2012-05-10 06:49:27, qiaoyun sun wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/ ----------------------------------------------------------- (Updated 2012-05-10 06:49:27) Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers. Summary ------- If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored. For example: the ifr is like : http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0 They should be substituted by the browser's setting in client side. This addresses bug SHINDIG-1763 . https://issues.apache.org/jira/browse/SHINDIG-1763 Diffs ----- http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177 http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1331168 Diff: https://reviews.apache.org/r/4906/diff Testing ------- Thanks, qiaoyun
      Hide
      Ryan Baxter added a comment -

      Committed revision 1336646

      Show
      Ryan Baxter added a comment - Committed revision 1336646
      Hide
      jiraposter@reviews.apache.org added a comment -

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

      Ship it!

      LGTM. I removed the whitespace for you. Please close the review.
      Committed revision 1336646

      • Ryan

      On 2012-05-10 06:49:27, qiaoyun sun wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/4906/

      -----------------------------------------------------------

      (Updated 2012-05-10 06:49:27)

      Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers.

      Summary

      -------

      If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored.

      For example: the ifr is like :

      http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0

      They should be substituted by the browser's setting in client side.

      This addresses bug SHINDIG-1763.

      https://issues.apache.org/jira/browse/SHINDIG-1763

      Diffs

      -----

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177

      http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1331168

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

      Testing

      -------

      Thanks,

      qiaoyun

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/#review7766 ----------------------------------------------------------- Ship it! LGTM. I removed the whitespace for you. Please close the review. Committed revision 1336646 Ryan On 2012-05-10 06:49:27, qiaoyun sun wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4906/ ----------------------------------------------------------- (Updated 2012-05-10 06:49:27) Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers. Summary ------- If we don't set this property in shindig.properties: shindig.urlgen.use-templates-default=false, the lang and country info will not pass to the ifr url, so the lang and country are ignored. For example: the ifr is like : http://localhost:8080/shindig/gadgets/ifr?url=http%3A%2F%2Flocalhost%3A8080%2Fshindig%2Fsamplecontainer%2Fexamples%2FSocialHelloWorld.xml&container=default&view=default&lang=%25lang%25&country=%25country%25&debug=1&nocache=1&sanitize=%25sanitize%25&v=0a811629d8d99f87d3544b65058211fe&st=john.doe%3Ajohn.doe%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3Acont%3Ahttp%253A%252F%252Flocalhost%253A8080%252Fshindig%252Fsamplecontainer%252Fexamples%252FSocialHelloWorld.xml%3A0%3Adefault%3A1335423393&testmode=0&parent=http%3A%2F%2Flocalhost%3A8080&mid=0 They should be substituted by the browser's setting in client side. This addresses bug SHINDIG-1763 . https://issues.apache.org/jira/browse/SHINDIG-1763 Diffs ----- http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.site.gadget/gadget_holder.js 1331177 http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/gadget_holder_test.js 1331168 Diff: https://reviews.apache.org/r/4906/diff Testing ------- Thanks, qiaoyun

        People

        • Assignee:
          Unassigned
          Reporter:
          sun qiao yun
        • Votes:
          0 Vote for this issue
          Watchers:
          0 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development