[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

[Howard M. Lewis Ship (JIRA)]

[ 
https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681430#action_12681430
 ] 

Howard M. Lewis Ship commented on TAP5-577:
---

I'm not sure if this really qualifies as breaking backwards compatibility, 
since so much of this behavior was internal in Tapestry 5.0.

The key features haven't changed:

- A users locale is initially determined from browser headers
- A specific locale may be selected via PersistentLocale.set()
- After invoking set(), later requests ignore the header and use the preferred 
locale

The big difference is that 5.0 used a client-side cookie, and 5.1 (by default) 
encodes the locale into the URL path.

Assuming users don't care about URLs, they won't see any difference ... except 
that when first accessing the 5.1 application they may revert to the default 
locale (determined from browser headers). However, this might occur anyway, if 
their cookie was deleted or expired.

The remaining difference is in how the locale is persisted between requests. 
Tapestry 5.1 already includes the ability to turn off encoding of the locale 
into the URL path. 

You three steps should work in theory.

What we have here is a change in behavior but not functionality; i.e., a change 
in the implementation of persistent locales. I don't see this as a backwards 
compatibility problem as the vast majority of users (who were not mucking about 
in the internals of Tapestry to customize persistent locales) will not see a 
difference in the upgrade (just slightly different URLs).

> TAP5-422 changes break persistent locale backwards compatibility.
> -
>
> Key: TAP5-577
> URL: https://issues.apache.org/jira/browse/TAP5-577
> Project: Tapestry 5
>  Issue Type: Bug
>  Components: tapestry-core
>Affects Versions: 5.1.0.0, 5.1.0.1
>Reporter: Andy Blower
>Priority: Critical
>
> I think that the changes made in T5.1 for TAP5-422 break backwards 
> compatibility with T5.0's locale persistence. In T5.0 it was a simple matter 
> to override the default cookie persistence by creating a custom 
> implementation of the PersistentLocale service and contributing it to be used 
> instead of the standard internal T5 implementation.
> The TAP5-422 changes broke backwards compatibility because anyone who's 
> created their own implementation of PersistentLocale, or just wants the 5.0 
> cookie persistence behaviour, would have found that it's a lot more work and 
> involves some heavy changes to Tapestry internals. Now with the recent 
> changes for TAP5-418 (committed yesterday), the situation had been alleviated 
> somewhat by allowing the the hard-wired URl locale persistence to be switched 
> off using a new symbol.
> However, I still think that this breaks backwards compatibility in two ways:
> 1) By changing the default behaviour of locale persistence so that anyone 
> relying on the locale persistence behaviour of 5.0 will have to make 
> non-trivial changes when they upgrade to 5.1 to keep the same operation.
> 2) By requiring so much work for anyone wanting to keep the 5.0 cookie 
> persistence behaviour or define their own custom locale persistence. (In 5.0 
> it was easy to figure out and implement a custom locale persistence method)
> From my analysis of the changes made by TAP5-422 & TAP5-418, I think anyone 
> wanting non-URL based locale persistence will need to do the following when 
> upgrading from 5.0 to 5.1:
> 1) Set the ENCODE_LOCALE_INTO_PATH symbol to false.
> 2) Create an implementation of PersistentLocale and contribute it to the IOC. 
> (copied from the standard 5.0 code if the old default cookie persistence is 
> desired)
> 3) Create a custom filter written and created to do the same job as the 5.0 
> LocalizationFilter and contribute it to the IOC RequestHandler. This filter 
> will need to call the LocalizationSetter setLocaleFromLocaleName() method 
> instead of the old setThreadLocale() method. 
> My suggested resolution would be to re-instate the 5.0 cookie persistence 
> (LocalizationFilter & PersistentLocaleImpl) and have the new 
> ENCODE_LOCALE_INTO_PATH symbol default to false allowing 5.1 to work the same 
> way as 5.0 out of the box. If the symbol is set to true, then the 
> LocalizationFilter is disabled (not contributed to RequestHandler) and the 
> PersistentLocale service will need to just store the locale (not set it in a 
> cookie) for later use by LinkSourceImpl. 
> LocalizationSetterImpl.setLocaleFromLocaleName(String localeName) would also 
> need changing back to overriding the passed localeName if a persistent one 
> had been set into the PersistentLocale service. There may by a much better 
> solution than this as I've not spent much time on it, but I though I should 
> try to be helpful as possibl

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

[Fernando (JIRA)]

[ 
https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681447#action_12681447
 ] 

Fernando commented on TAP5-577:
---

This just sounds like a HUGE change in behavior, without easily reverting to 
old way of doing things.

1) we can't cache any generated content across users!
2) we now totally mess up how we generate urls ( if we do/have to do it in 
javascript, or external services )
3) though the idea of putting locales in the url sounds cool, you can't just 
force that on us!!
  that is a huge change in site architecture, url planning, etc etc.
4) users might not care about urls, but we do, browsers do, caches do.

So I like the suggestion that we implement the old simpler more industry 
standard method be default, but let us easily change it to experiment and try 
out new things..


> TAP5-422 changes break persistent locale backwards compatibility.
> -
>
> Key: TAP5-577
> URL: https://issues.apache.org/jira/browse/TAP5-577
> Project: Tapestry 5
>  Issue Type: Bug
>  Components: tapestry-core
>Affects Versions: 5.1.0.0, 5.1.0.1
>Reporter: Andy Blower
>Priority: Critical
>
> I think that the changes made in T5.1 for TAP5-422 break backwards 
> compatibility with T5.0's locale persistence. In T5.0 it was a simple matter 
> to override the default cookie persistence by creating a custom 
> implementation of the PersistentLocale service and contributing it to be used 
> instead of the standard internal T5 implementation.
> The TAP5-422 changes broke backwards compatibility because anyone who's 
> created their own implementation of PersistentLocale, or just wants the 5.0 
> cookie persistence behaviour, would have found that it's a lot more work and 
> involves some heavy changes to Tapestry internals. Now with the recent 
> changes for TAP5-418 (committed yesterday), the situation had been alleviated 
> somewhat by allowing the the hard-wired URl locale persistence to be switched 
> off using a new symbol.
> However, I still think that this breaks backwards compatibility in two ways:
> 1) By changing the default behaviour of locale persistence so that anyone 
> relying on the locale persistence behaviour of 5.0 will have to make 
> non-trivial changes when they upgrade to 5.1 to keep the same operation.
> 2) By requiring so much work for anyone wanting to keep the 5.0 cookie 
> persistence behaviour or define their own custom locale persistence. (In 5.0 
> it was easy to figure out and implement a custom locale persistence method)
> From my analysis of the changes made by TAP5-422 & TAP5-418, I think anyone 
> wanting non-URL based locale persistence will need to do the following when 
> upgrading from 5.0 to 5.1:
> 1) Set the ENCODE_LOCALE_INTO_PATH symbol to false.
> 2) Create an implementation of PersistentLocale and contribute it to the IOC. 
> (copied from the standard 5.0 code if the old default cookie persistence is 
> desired)
> 3) Create a custom filter written and created to do the same job as the 5.0 
> LocalizationFilter and contribute it to the IOC RequestHandler. This filter 
> will need to call the LocalizationSetter setLocaleFromLocaleName() method 
> instead of the old setThreadLocale() method. 
> My suggested resolution would be to re-instate the 5.0 cookie persistence 
> (LocalizationFilter & PersistentLocaleImpl) and have the new 
> ENCODE_LOCALE_INTO_PATH symbol default to false allowing 5.1 to work the same 
> way as 5.0 out of the box. If the symbol is set to true, then the 
> LocalizationFilter is disabled (not contributed to RequestHandler) and the 
> PersistentLocale service will need to just store the locale (not set it in a 
> cookie) for later use by LinkSourceImpl. 
> LocalizationSetterImpl.setLocaleFromLocaleName(String localeName) would also 
> need changing back to overriding the passed localeName if a persistent one 
> had been set into the PersistentLocale service. There may by a much better 
> solution than this as I've not spent much time on it, but I though I should 
> try to be helpful as possible.
> (It should be noted that this is purely a product of my analysis of the 5.1 
> code, I have not found the time to actually run T5.1 and test this out - I 
> should be able to do this in about a week and a half, but I'm currently 
> approaching a major milestone deadline. Hopefully someone else will find the 
> time to prove or disprove my hypothesis.)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

[Howard M. Lewis Ship (JIRA)]

[ 
https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681470#action_12681470
 ] 

Howard M. Lewis Ship commented on TAP5-577:
---

Using cookies to track the user's locale is not industry default.  I beg to 
differ ... the majority of localized applications I've used encode the locale 
into the URL.

I don't see how this intersects caching of content ... unless you expect to 
cache English and German text as if it were interchangeable.  

> TAP5-422 changes break persistent locale backwards compatibility.
> -
>
> Key: TAP5-577
> URL: https://issues.apache.org/jira/browse/TAP5-577
> Project: Tapestry 5
>  Issue Type: Bug
>  Components: tapestry-core
>Affects Versions: 5.1.0.0, 5.1.0.1
>Reporter: Andy Blower
>Priority: Critical
>
> I think that the changes made in T5.1 for TAP5-422 break backwards 
> compatibility with T5.0's locale persistence. In T5.0 it was a simple matter 
> to override the default cookie persistence by creating a custom 
> implementation of the PersistentLocale service and contributing it to be used 
> instead of the standard internal T5 implementation.
> The TAP5-422 changes broke backwards compatibility because anyone who's 
> created their own implementation of PersistentLocale, or just wants the 5.0 
> cookie persistence behaviour, would have found that it's a lot more work and 
> involves some heavy changes to Tapestry internals. Now with the recent 
> changes for TAP5-418 (committed yesterday), the situation had been alleviated 
> somewhat by allowing the the hard-wired URl locale persistence to be switched 
> off using a new symbol.
> However, I still think that this breaks backwards compatibility in two ways:
> 1) By changing the default behaviour of locale persistence so that anyone 
> relying on the locale persistence behaviour of 5.0 will have to make 
> non-trivial changes when they upgrade to 5.1 to keep the same operation.
> 2) By requiring so much work for anyone wanting to keep the 5.0 cookie 
> persistence behaviour or define their own custom locale persistence. (In 5.0 
> it was easy to figure out and implement a custom locale persistence method)
> From my analysis of the changes made by TAP5-422 & TAP5-418, I think anyone 
> wanting non-URL based locale persistence will need to do the following when 
> upgrading from 5.0 to 5.1:
> 1) Set the ENCODE_LOCALE_INTO_PATH symbol to false.
> 2) Create an implementation of PersistentLocale and contribute it to the IOC. 
> (copied from the standard 5.0 code if the old default cookie persistence is 
> desired)
> 3) Create a custom filter written and created to do the same job as the 5.0 
> LocalizationFilter and contribute it to the IOC RequestHandler. This filter 
> will need to call the LocalizationSetter setLocaleFromLocaleName() method 
> instead of the old setThreadLocale() method. 
> My suggested resolution would be to re-instate the 5.0 cookie persistence 
> (LocalizationFilter & PersistentLocaleImpl) and have the new 
> ENCODE_LOCALE_INTO_PATH symbol default to false allowing 5.1 to work the same 
> way as 5.0 out of the box. If the symbol is set to true, then the 
> LocalizationFilter is disabled (not contributed to RequestHandler) and the 
> PersistentLocale service will need to just store the locale (not set it in a 
> cookie) for later use by LinkSourceImpl. 
> LocalizationSetterImpl.setLocaleFromLocaleName(String localeName) would also 
> need changing back to overriding the passed localeName if a persistent one 
> had been set into the PersistentLocale service. There may by a much better 
> solution than this as I've not spent much time on it, but I though I should 
> try to be helpful as possible.
> (It should be noted that this is purely a product of my analysis of the 5.1 
> code, I have not found the time to actually run T5.1 and test this out - I 
> should be able to do this in about a week and a half, but I'm currently 
> approaching a major milestone deadline. Hopefully someone else will find the 
> time to prove or disprove my hypothesis.)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

[Fernando (JIRA)]

[ 
https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681482#action_12681482
 ] 

Fernando commented on TAP5-577:
---

fair. ok. so I'm only left with : "fear of change". :)

and allowing us to slowly switch to it, bit by bit, rather than forcing us to 
do it next time I upgrade :)
just to make sure that there really are no issues that we have to deal with ( i 
do worry about previously generated urls, manually generated urls, javascript, 
etc ) :)

> TAP5-422 changes break persistent locale backwards compatibility.
> -
>
> Key: TAP5-577
> URL: https://issues.apache.org/jira/browse/TAP5-577
> Project: Tapestry 5
>  Issue Type: Bug
>  Components: tapestry-core
>Affects Versions: 5.1.0.0, 5.1.0.1
>Reporter: Andy Blower
>Priority: Critical
>
> I think that the changes made in T5.1 for TAP5-422 break backwards 
> compatibility with T5.0's locale persistence. In T5.0 it was a simple matter 
> to override the default cookie persistence by creating a custom 
> implementation of the PersistentLocale service and contributing it to be used 
> instead of the standard internal T5 implementation.
> The TAP5-422 changes broke backwards compatibility because anyone who's 
> created their own implementation of PersistentLocale, or just wants the 5.0 
> cookie persistence behaviour, would have found that it's a lot more work and 
> involves some heavy changes to Tapestry internals. Now with the recent 
> changes for TAP5-418 (committed yesterday), the situation had been alleviated 
> somewhat by allowing the the hard-wired URl locale persistence to be switched 
> off using a new symbol.
> However, I still think that this breaks backwards compatibility in two ways:
> 1) By changing the default behaviour of locale persistence so that anyone 
> relying on the locale persistence behaviour of 5.0 will have to make 
> non-trivial changes when they upgrade to 5.1 to keep the same operation.
> 2) By requiring so much work for anyone wanting to keep the 5.0 cookie 
> persistence behaviour or define their own custom locale persistence. (In 5.0 
> it was easy to figure out and implement a custom locale persistence method)
> From my analysis of the changes made by TAP5-422 & TAP5-418, I think anyone 
> wanting non-URL based locale persistence will need to do the following when 
> upgrading from 5.0 to 5.1:
> 1) Set the ENCODE_LOCALE_INTO_PATH symbol to false.
> 2) Create an implementation of PersistentLocale and contribute it to the IOC. 
> (copied from the standard 5.0 code if the old default cookie persistence is 
> desired)
> 3) Create a custom filter written and created to do the same job as the 5.0 
> LocalizationFilter and contribute it to the IOC RequestHandler. This filter 
> will need to call the LocalizationSetter setLocaleFromLocaleName() method 
> instead of the old setThreadLocale() method. 
> My suggested resolution would be to re-instate the 5.0 cookie persistence 
> (LocalizationFilter & PersistentLocaleImpl) and have the new 
> ENCODE_LOCALE_INTO_PATH symbol default to false allowing 5.1 to work the same 
> way as 5.0 out of the box. If the symbol is set to true, then the 
> LocalizationFilter is disabled (not contributed to RequestHandler) and the 
> PersistentLocale service will need to just store the locale (not set it in a 
> cookie) for later use by LinkSourceImpl. 
> LocalizationSetterImpl.setLocaleFromLocaleName(String localeName) would also 
> need changing back to overriding the passed localeName if a persistent one 
> had been set into the PersistentLocale service. There may by a much better 
> solution than this as I've not spent much time on it, but I though I should 
> try to be helpful as possible.
> (It should be noted that this is purely a product of my analysis of the 5.1 
> code, I have not found the time to actually run T5.1 and test this out - I 
> should be able to do this in about a week and a half, but I'm currently 
> approaching a major milestone deadline. Hopefully someone else will find the 
> time to prove or disprove my hypothesis.)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

[Robert Zeigler (JIRA)]

[ 
https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681490#action_12681490
 ] 

Robert Zeigler commented on TAP5-577:
-

1) previously generated urls => should be gracefully handled as simply a url 
with no locale info? So the user loses his/her locale for that request...
2) manually generated urls/javascript: Hm... shouldn't those be generated via 
componentResources.create{Action,Event,Form,Page}Link, in which case: no change 
required? :)


> TAP5-422 changes break persistent locale backwards compatibility.
> -
>
> Key: TAP5-577
> URL: https://issues.apache.org/jira/browse/TAP5-577
> Project: Tapestry 5
>  Issue Type: Bug
>  Components: tapestry-core
>Affects Versions: 5.1.0.0, 5.1.0.1
>Reporter: Andy Blower
>Priority: Critical
>
> I think that the changes made in T5.1 for TAP5-422 break backwards 
> compatibility with T5.0's locale persistence. In T5.0 it was a simple matter 
> to override the default cookie persistence by creating a custom 
> implementation of the PersistentLocale service and contributing it to be used 
> instead of the standard internal T5 implementation.
> The TAP5-422 changes broke backwards compatibility because anyone who's 
> created their own implementation of PersistentLocale, or just wants the 5.0 
> cookie persistence behaviour, would have found that it's a lot more work and 
> involves some heavy changes to Tapestry internals. Now with the recent 
> changes for TAP5-418 (committed yesterday), the situation had been alleviated 
> somewhat by allowing the the hard-wired URl locale persistence to be switched 
> off using a new symbol.
> However, I still think that this breaks backwards compatibility in two ways:
> 1) By changing the default behaviour of locale persistence so that anyone 
> relying on the locale persistence behaviour of 5.0 will have to make 
> non-trivial changes when they upgrade to 5.1 to keep the same operation.
> 2) By requiring so much work for anyone wanting to keep the 5.0 cookie 
> persistence behaviour or define their own custom locale persistence. (In 5.0 
> it was easy to figure out and implement a custom locale persistence method)
> From my analysis of the changes made by TAP5-422 & TAP5-418, I think anyone 
> wanting non-URL based locale persistence will need to do the following when 
> upgrading from 5.0 to 5.1:
> 1) Set the ENCODE_LOCALE_INTO_PATH symbol to false.
> 2) Create an implementation of PersistentLocale and contribute it to the IOC. 
> (copied from the standard 5.0 code if the old default cookie persistence is 
> desired)
> 3) Create a custom filter written and created to do the same job as the 5.0 
> LocalizationFilter and contribute it to the IOC RequestHandler. This filter 
> will need to call the LocalizationSetter setLocaleFromLocaleName() method 
> instead of the old setThreadLocale() method. 
> My suggested resolution would be to re-instate the 5.0 cookie persistence 
> (LocalizationFilter & PersistentLocaleImpl) and have the new 
> ENCODE_LOCALE_INTO_PATH symbol default to false allowing 5.1 to work the same 
> way as 5.0 out of the box. If the symbol is set to true, then the 
> LocalizationFilter is disabled (not contributed to RequestHandler) and the 
> PersistentLocale service will need to just store the locale (not set it in a 
> cookie) for later use by LinkSourceImpl. 
> LocalizationSetterImpl.setLocaleFromLocaleName(String localeName) would also 
> need changing back to overriding the passed localeName if a persistent one 
> had been set into the PersistentLocale service. There may by a much better 
> solution than this as I've not spent much time on it, but I though I should 
> try to be helpful as possible.
> (It should be noted that this is purely a product of my analysis of the 5.1 
> code, I have not found the time to actually run T5.1 and test this out - I 
> should be able to do this in about a week and a half, but I'm currently 
> approaching a major milestone deadline. Hopefully someone else will find the 
> time to prove or disprove my hypothesis.)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

[Fernando (JIRA)]

[ 
https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681491#action_12681491
 ] 

Fernando commented on TAP5-577:
---

1) you mean every time a user comes to the site?
1a) we are a facebook/opensocial app, so they proxy back to us, and the base 
url can't really change.. else we will be redirecting on every single request.
 (actually, because of this I just realized that we can never use the locale in 
url :( :( :( )
2) yes all urls should be generated by tapestry, but we have a handful of 
apps/sites, handful of developers, tonnes of features we need to implements and 
just
   a slight lack of time to do it all in.. I can't guarantee that all of our 
developers and efforts are on the up and up... so I can't roll this out without 
copious amounts
   of testing. (hence the slow/controlled rollout ability, in our own terms)


> TAP5-422 changes break persistent locale backwards compatibility.
> -
>
> Key: TAP5-577
> URL: https://issues.apache.org/jira/browse/TAP5-577
> Project: Tapestry 5
>  Issue Type: Bug
>  Components: tapestry-core
>Affects Versions: 5.1.0.0, 5.1.0.1
>Reporter: Andy Blower
>Priority: Critical
>
> I think that the changes made in T5.1 for TAP5-422 break backwards 
> compatibility with T5.0's locale persistence. In T5.0 it was a simple matter 
> to override the default cookie persistence by creating a custom 
> implementation of the PersistentLocale service and contributing it to be used 
> instead of the standard internal T5 implementation.
> The TAP5-422 changes broke backwards compatibility because anyone who's 
> created their own implementation of PersistentLocale, or just wants the 5.0 
> cookie persistence behaviour, would have found that it's a lot more work and 
> involves some heavy changes to Tapestry internals. Now with the recent 
> changes for TAP5-418 (committed yesterday), the situation had been alleviated 
> somewhat by allowing the the hard-wired URl locale persistence to be switched 
> off using a new symbol.
> However, I still think that this breaks backwards compatibility in two ways:
> 1) By changing the default behaviour of locale persistence so that anyone 
> relying on the locale persistence behaviour of 5.0 will have to make 
> non-trivial changes when they upgrade to 5.1 to keep the same operation.
> 2) By requiring so much work for anyone wanting to keep the 5.0 cookie 
> persistence behaviour or define their own custom locale persistence. (In 5.0 
> it was easy to figure out and implement a custom locale persistence method)
> From my analysis of the changes made by TAP5-422 & TAP5-418, I think anyone 
> wanting non-URL based locale persistence will need to do the following when 
> upgrading from 5.0 to 5.1:
> 1) Set the ENCODE_LOCALE_INTO_PATH symbol to false.
> 2) Create an implementation of PersistentLocale and contribute it to the IOC. 
> (copied from the standard 5.0 code if the old default cookie persistence is 
> desired)
> 3) Create a custom filter written and created to do the same job as the 5.0 
> LocalizationFilter and contribute it to the IOC RequestHandler. This filter 
> will need to call the LocalizationSetter setLocaleFromLocaleName() method 
> instead of the old setThreadLocale() method. 
> My suggested resolution would be to re-instate the 5.0 cookie persistence 
> (LocalizationFilter & PersistentLocaleImpl) and have the new 
> ENCODE_LOCALE_INTO_PATH symbol default to false allowing 5.1 to work the same 
> way as 5.0 out of the box. If the symbol is set to true, then the 
> LocalizationFilter is disabled (not contributed to RequestHandler) and the 
> PersistentLocale service will need to just store the locale (not set it in a 
> cookie) for later use by LinkSourceImpl. 
> LocalizationSetterImpl.setLocaleFromLocaleName(String localeName) would also 
> need changing back to overriding the passed localeName if a persistent one 
> had been set into the PersistentLocale service. There may by a much better 
> solution than this as I've not spent much time on it, but I though I should 
> try to be helpful as possible.
> (It should be noted that this is purely a product of my analysis of the 5.1 
> code, I have not found the time to actually run T5.1 and test this out - I 
> should be able to do this in about a week and a half, but I'm currently 
> approaching a major milestone deadline. Hopefully someone else will find the 
> time to prove or disprove my hypothesis.)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

[Andy Blower (JIRA)]

[ 
https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681499#action_12681499
 ] 

Andy Blower commented on TAP5-577:
--

Howard, 

I agree that using cookies to track user's persistent locale is not industry 
standard, but it happens to be the method used by the final public release of 
T5.0 which is the important point here. The fact that the default method for 
persisting locales is different is really only a backwards compatibility issue 
for anyone who's relying on the generated URLs for some reason or has issues 
with locale being in the URLs. (like our site which needs durable and shareable 
URLs regardless of a users' locale preference)

To my mind this is a judgement call for you to take as it's quite a subjective 
aspect of backwards compatibility, assuming of course that it's easy to 
override the default behaviour like it was in T5.0 where you provided an easy 
way with a public interface. This is where I have the main (less subjective, 
more objective) issue...

You are right that the functionality of persisting locale once set is still 
retained and is just done using a different method in T5.1, but only for people 
who haven't implemented their own PersistentLocale service. If they have then 
it will break on upgrading to T5.1 because the service interface (which is 
public and not internal) is now used in a different way by the internals - in 
other words the contract you made for this public interface is broken. If it 
were as simple as changing the symbol to false to re-enable the old contract 
with the public PersistentLocale service then it wouldn't be so bad, but 
currently it's non-trivial for someone to get their custom PersistentLocale 
services working in T5.1 which is why I consider backwards compatibility to be 
broken in this instance.

Regarding your statement "vast majority of users (who were not mucking about in 
the internals of Tapestry to customize persistent locales)" - I really don't 
think that creating and contributing a custom implementation of a public 
service interface can be characterized in this way. Especially by you. I 
thought that this was what T5 was all about; functional out of the box, but 
allowing powerful customization where it just "gets out of the way" to let you 
do what you need. Have I got this wrong?

I definitely have mucked about with Tapestry internals and I will have some 
upgrade pains due to these modification, but I knew that when I did it and you 
will not hear a thing from me about backwards compatibility when I've messed 
with internals (even if there was no other way in T5.0 to achieve what I needed 
to do) but that is not the case here.

I think I can safely assume that you do not consider reverting the default 
behaviour of T5.1 to cookie locale persistence an option which is fair enough. 
I don't think much of cookie persistence myself so I'm certainly not bothered 
by it, I just hope it doesn't trip up too many others. That's my only concern, 
although you seem to have confidence that it wont so my fears are most likely 
unfounded. As I've said before it will only really be a minor inconvenience to 
me personally if you don't resolve this issue - I think I know what I'm doing 
and have the ability to cope with this for my own work with Tapestry.

However, I do think the other issue of overriding the default method of locale 
persistence becoming so much harder in T5.1 is a major issue and does break 
backwards compatibility. The aim should be that if the ENCODE_LOCALE_INTO_PATH 
symbol is set to false, then a custom PersistentLocale service that worked with 
T5.0 should work the same with T5.1 so I guess the only sticking point is the 
lack of a LocalizationFilter in T5.1 which is enabled when the symbol is false 
and allows a custom PersistentLocale service written for T5.0 to work with T5.1 
without a lot of hassle and migration work. This should fix the main 
(objective) part of this issue.

> TAP5-422 changes break persistent locale backwards compatibility.
> -
>
> Key: TAP5-577
> URL: https://issues.apache.org/jira/browse/TAP5-577
> Project: Tapestry 5
>  Issue Type: Bug
>  Components: tapestry-core
>Affects Versions: 5.1.0.0, 5.1.0.1
>Reporter: Andy Blower
>Priority: Critical
>
> I think that the changes made in T5.1 for TAP5-422 break backwards 
> compatibility with T5.0's locale persistence. In T5.0 it was a simple matter 
> to override the default cookie persistence by creating a custom 
> implementation of the PersistentLocale service and contributing it to be used 
> instead of the standard internal T5 implementation.
> The TAP5-422 changes broke backwards compatibility because anyone who's 
> created their own implementation of PersistentLocale, or just

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

[Ferdy Nagy (JIRA)]

[ 
https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12703460#action_12703460
 ] 

Ferdy Nagy commented on TAP5-577:
-

You're not the only one bothered by this, and I think there are reasonable 
expectations that locale could passed around by cookie.  We use a central SSO 
that allows people to change their preferred language and they can move from 
application to application within the domain with each application displaying 
in their language choice.

The process outlined by Andy doesn't really work because the 
ComponentEventLinkEncoder always calls the LocalizationSetter which in turn 
always sets the ThreadLocale; thus overriding the set made by the filter.  To 
make it work I had to override the LocalizationSetter service to always return 
false, and change the filter to set the ThreadLocale directly.

This just doesn't "feel" right.  It seems like there should be a simple way for 
a single concern to deal with the localization setting that can handle a choice 
of either the URL containing the locale, or a cookie, or from the request.  I 
shouldn't have to deal with a filter and a localization setter and a 
persistence layer just to say "here is the locale" - and I certainly shouldn't 
have to look at the internals of LocalizationSetter and CELE to understand how 
this works (but I'm very happy I can, and also grok the process easily).

> TAP5-422 changes break persistent locale backwards compatibility.
> -
>
> Key: TAP5-577
> URL: https://issues.apache.org/jira/browse/TAP5-577
> Project: Tapestry 5
>  Issue Type: Bug
>  Components: tapestry-core
>Affects Versions: 5.1.0.0, 5.1.0.1
>Reporter: Andy Blower
>Priority: Critical
> Attachments: T5.1persistentLocale.txt
>
>
> I think that the changes made in T5.1 for TAP5-422 break backwards 
> compatibility with T5.0's locale persistence. In T5.0 it was a simple matter 
> to override the default cookie persistence by creating a custom 
> implementation of the PersistentLocale service and contributing it to be used 
> instead of the standard internal T5 implementation.
> The TAP5-422 changes broke backwards compatibility because anyone who's 
> created their own implementation of PersistentLocale, or just wants the 5.0 
> cookie persistence behaviour, would have found that it's a lot more work and 
> involves some heavy changes to Tapestry internals. Now with the recent 
> changes for TAP5-418 (committed yesterday), the situation had been alleviated 
> somewhat by allowing the the hard-wired URl locale persistence to be switched 
> off using a new symbol.
> However, I still think that this breaks backwards compatibility in two ways:
> 1) By changing the default behaviour of locale persistence so that anyone 
> relying on the locale persistence behaviour of 5.0 will have to make 
> non-trivial changes when they upgrade to 5.1 to keep the same operation.
> 2) By requiring so much work for anyone wanting to keep the 5.0 cookie 
> persistence behaviour or define their own custom locale persistence. (In 5.0 
> it was easy to figure out and implement a custom locale persistence method)
> From my analysis of the changes made by TAP5-422 & TAP5-418, I think anyone 
> wanting non-URL based locale persistence will need to do the following when 
> upgrading from 5.0 to 5.1:
> 1) Set the ENCODE_LOCALE_INTO_PATH symbol to false.
> 2) Create an implementation of PersistentLocale and contribute it to the IOC. 
> (copied from the standard 5.0 code if the old default cookie persistence is 
> desired)
> 3) Create a custom filter written and created to do the same job as the 5.0 
> LocalizationFilter and contribute it to the IOC RequestHandler. This filter 
> will need to call the LocalizationSetter setLocaleFromLocaleName() method 
> instead of the old setThreadLocale() method. 
> My suggested resolution would be to re-instate the 5.0 cookie persistence 
> (LocalizationFilter & PersistentLocaleImpl) and have the new 
> ENCODE_LOCALE_INTO_PATH symbol default to false allowing 5.1 to work the same 
> way as 5.0 out of the box. If the symbol is set to true, then the 
> LocalizationFilter is disabled (not contributed to RequestHandler) and the 
> PersistentLocale service will need to just store the locale (not set it in a 
> cookie) for later use by LinkSourceImpl. 
> LocalizationSetterImpl.setLocaleFromLocaleName(String localeName) would also 
> need changing back to overriding the passed localeName if a persistent one 
> had been set into the PersistentLocale service. There may by a much better 
> solution than this as I've not spent much time on it, but I though I should 
> try to be helpful as possible.
> (It should be noted that this is purely a product of my analysis of the 5.1 
> code, I have no

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

[Andy Blower (JIRA)]

[ 
https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12703567#action_12703567
 ] 

Andy Blower commented on TAP5-577:
--

"The process outlined by Andy doesn't really work because the 
ComponentEventLinkEncoder always calls the LocalizationSetter which in turn 
always sets the ThreadLocale; thus overriding the set made by the filter. To 
make it work I had to override the LocalizationSetter service to always return 
false, and change the filter to set the ThreadLocale directly. "

Nice to hear I'm not the only one bothered by this Ferdy, and you're quite 
correct on this - I just traced though to double check! The weird thing is what 
I've done at least half works... when the filter sets the locale, the messages 
displayed on my page are correctly localised even though the thread locale is 
subsequently set back to the browser default rather than our persistent locale. 
I have no idea why this is and I honestly haven't the time or inclination any 
more to look into this problem more so I'm not going to be finding out. I must 
have spent 20+ hours on this one issue, looking at code & writing 
emails/issues,  with absolutely no result. So I have better things to do with 
my time, but I definitely think that localisation has become a mess in T5.1 
which is a shame since it was so nice to simply contribute a service 
implementation of a public interface in 5.0 and the job was done.

I will be following the method you outlined, which will also mean I'll be 
following Howards advice from TAP5-658. (you might find that issue interesting 
to read also) My filter will also need the fall back to browser request locale 
copied from the setter, but that's no biggie. Thanks for the catch as this 
probably would have bitten me in the ass in a few months.



> TAP5-422 changes break persistent locale backwards compatibility.
> -
>
> Key: TAP5-577
> URL: https://issues.apache.org/jira/browse/TAP5-577
> Project: Tapestry 5
>  Issue Type: Bug
>  Components: tapestry-core
>Affects Versions: 5.1.0.0, 5.1.0.1
>Reporter: Andy Blower
>Priority: Critical
> Attachments: T5.1persistentLocale.txt
>
>
> I think that the changes made in T5.1 for TAP5-422 break backwards 
> compatibility with T5.0's locale persistence. In T5.0 it was a simple matter 
> to override the default cookie persistence by creating a custom 
> implementation of the PersistentLocale service and contributing it to be used 
> instead of the standard internal T5 implementation.
> The TAP5-422 changes broke backwards compatibility because anyone who's 
> created their own implementation of PersistentLocale, or just wants the 5.0 
> cookie persistence behaviour, would have found that it's a lot more work and 
> involves some heavy changes to Tapestry internals. Now with the recent 
> changes for TAP5-418 (committed yesterday), the situation had been alleviated 
> somewhat by allowing the the hard-wired URl locale persistence to be switched 
> off using a new symbol.
> However, I still think that this breaks backwards compatibility in two ways:
> 1) By changing the default behaviour of locale persistence so that anyone 
> relying on the locale persistence behaviour of 5.0 will have to make 
> non-trivial changes when they upgrade to 5.1 to keep the same operation.
> 2) By requiring so much work for anyone wanting to keep the 5.0 cookie 
> persistence behaviour or define their own custom locale persistence. (In 5.0 
> it was easy to figure out and implement a custom locale persistence method)
> From my analysis of the changes made by TAP5-422 & TAP5-418, I think anyone 
> wanting non-URL based locale persistence will need to do the following when 
> upgrading from 5.0 to 5.1:
> 1) Set the ENCODE_LOCALE_INTO_PATH symbol to false.
> 2) Create an implementation of PersistentLocale and contribute it to the IOC. 
> (copied from the standard 5.0 code if the old default cookie persistence is 
> desired)
> 3) Create a custom filter written and created to do the same job as the 5.0 
> LocalizationFilter and contribute it to the IOC RequestHandler. This filter 
> will need to call the LocalizationSetter setLocaleFromLocaleName() method 
> instead of the old setThreadLocale() method. 
> My suggested resolution would be to re-instate the 5.0 cookie persistence 
> (LocalizationFilter & PersistentLocaleImpl) and have the new 
> ENCODE_LOCALE_INTO_PATH symbol default to false allowing 5.1 to work the same 
> way as 5.0 out of the box. If the symbol is set to true, then the 
> LocalizationFilter is disabled (not contributed to RequestHandler) and the 
> PersistentLocale service will need to just store the locale (not set it in a 
> cookie) for later use by LinkSourceImpl. 
> LocalizationSetterImpl.setLocaleFr

[jira] Commented: (TAP5-577) TAP5-422 changes break persistent locale backwards compatibility.

[Ilya Obshadko (JIRA)]

[ 
https://issues.apache.org/jira/browse/TAP5-577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12747829#action_12747829
 ] 

Ilya Obshadko commented on TAP5-577:


The weird thing is what I've done at least half works... when the filter 
sets the locale, the messages displayed on my page are correctly localised even 
though the thread locale is subsequently set back to the browser default rather 
than our persistent locale. I have no idea why this is and I honestly haven't 
the time or inclination any more to look into this problem more so I'm not 
going to be finding out.

This is because of the following code in LocalizationSetterImpl

public boolean setLocaleFromLocaleName(String localeName)
{
boolean supported = acceptedLocaleNames.contains(localeName);

if (supported)
{
Locale locale = findClosestSupportedLocale(toLocale(localeName));
persistentLocale.set(locale);
threadLocale.setLocale(locale);
}
else
{
Locale requestLocale = request.getLocale();
//  
Locale supportedLocale = findClosestSupportedLocale(requestLocale);
threadLocale.setLocale(supportedLocale);
}

return supported;
}

I found out that during normal request processing this method is being called 
even if there is no persistent locale in the URL. So, when URL is, for example, 
http://localhost:8080/app/index, the methods takes string "index" as an 
argument; then of course it sets supported to false, and eventually falls back 
to the request locale.

I believe the whole implementation is flawed; LocalizationSetter implementation 
should not guess if it's supplied with locale name or not, the argument must be 
valid locale code (already defined in the list of possible locales in 
configuration) from the very start. So locale code should be checked for 
validity before it's passed to LocalizationSetter.

My solution was just copy-pasting LocalizationSetterImpl; then I've modified 
the code so LocalizationSetter would fall back to 'default' locale (first in 
supported locales list) rather than request locale. That solved my particular 
problem (I need Russian language version of the website by default).


> TAP5-422 changes break persistent locale backwards compatibility.
> -
>
> Key: TAP5-577
> URL: https://issues.apache.org/jira/browse/TAP5-577
> Project: Tapestry 5
>  Issue Type: Bug
>  Components: tapestry-core
>Affects Versions: 5.1.0.0, 5.1.0.1
>Reporter: Andy Blower
>Priority: Critical
> Attachments: T5.1persistentLocale.txt
>
>
> I think that the changes made in T5.1 for TAP5-422 break backwards 
> compatibility with T5.0's locale persistence. In T5.0 it was a simple matter 
> to override the default cookie persistence by creating a custom 
> implementation of the PersistentLocale service and contributing it to be used 
> instead of the standard internal T5 implementation.
> The TAP5-422 changes broke backwards compatibility because anyone who's 
> created their own implementation of PersistentLocale, or just wants the 5.0 
> cookie persistence behaviour, would have found that it's a lot more work and 
> involves some heavy changes to Tapestry internals. Now with the recent 
> changes for TAP5-418 (committed yesterday), the situation had been alleviated 
> somewhat by allowing the the hard-wired URl locale persistence to be switched 
> off using a new symbol.
> However, I still think that this breaks backwards compatibility in two ways:
> 1) By changing the default behaviour of locale persistence so that anyone 
> relying on the locale persistence behaviour of 5.0 will have to make 
> non-trivial changes when they upgrade to 5.1 to keep the same operation.
> 2) By requiring so much work for anyone wanting to keep the 5.0 cookie 
> persistence behaviour or define their own custom locale persistence. (In 5.0 
> it was easy to figure out and implement a custom locale persistence method)
> From my analysis of the changes made by TAP5-422 & TAP5-418, I think anyone 
> wanting non-URL based locale persistence will need to do the following when 
> upgrading from 5.0 to 5.1:
> 1) Set the ENCODE_LOCALE_INTO_PATH symbol to false.
> 2) Create an implementation of PersistentLocale and contribute it to the IOC. 
> (copied from the standard 5.0 code if the old default cookie persistence is 
> desired)
> 3) Create a custom filter written and created to do the same job as the 5.0 
> LocalizationFilter and contribute it to the IOC RequestHandler. This filter 
> will need to call the LocalizationSetter setLocaleFromLocaleName() method 
> instead of the old setThreadLocale() method. 
> My suggested resolution would be to