Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Outbound rule for Internationalize your URLs #210

Open
brnovais opened this issue May 23, 2015 · 4 comments
Open

Outbound rule for Internationalize your URLs #210

brnovais opened this issue May 23, 2015 · 4 comments

Comments

@brnovais
Copy link

Implementing the tutorial Internationalize your URLs, everything works just fine for Inbound rules. When I try to use tags such as h:link or h:outputLink in my XHTML page, I got a few exceptions regarding Outbound rewriting.

A pull request testing the issue can be viewed here: #209.

You can see more information and discussion here.

To speedup my test, I used the following command: mvn test -PWILDFLY_MANAGED_8 -Dtest=LocaleTranspositionTest

chkal added a commit that referenced this issue May 26, 2015
@chkal
Copy link
Member

chkal commented May 26, 2015

Hey! Thanks a lot.

I'll try to find some time in the next days to have a look at this. But I'm very busy. So it may take some time.

If you would like to dig deeper into this, I've a first idea what is causing this. IMO the error is caused by this code in Join.

https://github.com/ocpsoft/rewrite/blob/a39a2d18f94e9843893d0fedb2ac9536cb9952ed/config-servlet/src/main/java/org/ocpsoft/rewrite/servlet/config/rule/Join.java#L164-168

If the evaluation order is inverted somehow, the query parameter would be processed BEFORE the path parameter. So the path parameter transposition should be able to find language from the query parameter in this case. It is worth a try. But I'm not sure about the side effect. But it would be interesting to see if inverting this code fixes the issue.

chkal added a commit to chkal/rewrite that referenced this issue May 28, 2015
chkal added a commit that referenced this issue May 28, 2015
@chkal
Copy link
Member

chkal commented May 28, 2015

I created a branch with a possible fix.

https://github.com/ocpsoft/rewrite/tree/issue-210

chkal added a commit that referenced this issue May 28, 2015
@cgendreau
Copy link
Contributor

Thanks for the outbound rule test @brnovais and thanks for the possible fix @chkal.

Maybe the translate(String lang, String value) method should handle lang == null more gracefully but in theory (AFAIK at least), the transposition is not called unless it matches the Rule. So I'm wondering if under normal circumstance, Parameters.retrieve(context, this.languageParam) can return null and if yes, do we notify the user(developer) or we just ignore the transposition and keep the original value?

@chkal
Copy link
Member

chkal commented May 28, 2015

I agree that there should be some exception telling the developer that the language query parameter is missing. It's a very common mistake to forget about the <f:param> in <h:link>.

The main problem here is that the transposition is done while writing the parameter value to the parameter value store. Now if a transpositions depends on some other parameter value, the order of processing the parameter values becomes important.

Basically my fix is just a hack. It just processes all query parameters before processing the path itself. As usually a path parameter is transposed using LocaleTransposition, this works around the issue here.

I wonder what @lincolnthree things about this. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants