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

Multibyte PagePath (URL) support #366

Merged
merged 1 commit into from Sep 4, 2014

Conversation

tao-s
Copy link
Contributor

@tao-s tao-s commented Jul 3, 2014

concrete5 Japan team is taking a look at multibyte issues now.

Multibyte URL is very important to us
Therefore, we have made the pull request to meet the followings:

  1. Sanitization.

We can use Japanese (and other multibyte characters) for URL slug, but there is no sanitization on both way (enter and output). What's your plan in the future?

  1. Decode page path

We need to decode the page path URL at Request class in order to be able to use multibyte characters. Otherwise, it would return 404. Can we do it?

  1. When printing out the page path in , we need to encode the page path.
  2. Are you still planning to use PagePath object when handling Page object? Or getCollectionPath() would be obsolete?

@katzueno
Copy link
Contributor

katzueno commented Sep 4, 2014

Ping.

@aembler aembler merged commit 0d1b592 into concretecms:master Sep 4, 2014
@aembler
Copy link
Member

aembler commented Sep 4, 2014

I have merged this pull request with some tweaks:

  1. I have included the decoding of getPath() - this fixes the 404s.
  2. I have included the getEncodePath() method in the Page class3
  3. I have not wrapped getCollectionPath in this method, or the other method. I feel like wrapping getCollectionPath() in this method all the time is a bad idea – it should be wrapped only on those rare times when we actually display it. Right? Thoughts?

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

Successfully merging this pull request may close these issues.

None yet

3 participants