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

FIX: Fix to allow use of foreign text as tags and categories. Fixes … #245

Merged
merged 1 commit into from
Jun 3, 2015

Conversation

gordonbanderson
Copy link
Contributor

Instead of using the URL parameter directly for a tag or category value, generate a URL segment initially and search using that. This fixes the issue of likes of Thai text not being found.

@assertchris
Copy link
Contributor

Thanks for this. Just want to test it out locally, but it looks good...

@assertchris
Copy link
Contributor

I've tested this and it works. If you wouldn't mind rebasing it, I'll merge it! :)

@tractorcow
Copy link

Hi @gordonbanderson I have had a look at the sourced issue and your fix, and I'm not sure that passing the argument through the URLSegmentFilter twice is going to be ideal. we run the risk of "double encoding" in edge cases.

The root issue is that any extended characters are automatically decoded by the time it gets to this method. All that's necessary is to ensure that we cope with any decoded value by re-encoding it as it's stored in the database.

This is the fix I've added to include the encoded value as a fallback.

        if($tag) {
            return $dataRecord->Tags()
                ->filter('URLSegment', array($tag, rawurlencode($tag)))
                ->first();
        }

My main concern is that we are running the queried string past all the other logic which really has nothing to do with url decoding (such as checking for duplicate tags, plus other transformations on the string).

@tractorcow
Copy link

@gordonbanderson can you please see if #263 fixes your issues?

assertchris added a commit that referenced this pull request Jun 3, 2015
FIX: Fix to allow use of foreign text as tags and categories.  Fixes …
@assertchris assertchris merged commit 1b7c312 into silverstripe:master Jun 3, 2015
@gordonbanderson
Copy link
Contributor Author

Looks like I was too late to test :) I did feel regenerating the URLSegment was a bit icky but I needed a quick fix, your solution appears cleaner. Will test tomorrow.

@gordonbanderson gordonbanderson deleted the ISSUE_243 branch June 4, 2015 03:06
@gordonbanderson
Copy link
Contributor Author

can confirm above fix works

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.

3 participants