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 EZP-26748: Wrong image URI when content name has extended UTF characters #150

Closed
wants to merge 2 commits into from

Conversation

glye
Copy link
Member

@glye glye commented Dec 20, 2016

Fixes https://jira.ez.no/browse/EZP-26748
Test in https://github.com/ezsystems/ezpublish-kernel-ee/pull/145
Taken from https://github.com/ezsystems/ezpublish-platform/pull/20 where the tests don't run
Status: Ready to merge

Fixes the problem by ensuring UTF-8 is used, though this probably isn't the ideal way to do it. Anyway, let's see how the full tests go.

@glye
Copy link
Member Author

glye commented Jan 19, 2017

Review ping @andrerom @bdunogier - this is a (perhaps too) simple solution. I have hardcoded it because AFAIK ezp is utf-8 only.

@bdunogier
Copy link
Member

I'm not a charset expert, but based on what I know it looks okay.

Do we need to add the same thing to our tests bootstrap files, to be consistent ?

@glye
Copy link
Member Author

glye commented Jan 23, 2017

Do we need to add the same thing to our tests bootstrap files, to be consistent?

@bdunogier Sounds good. Could you give some pointers? (Travis/Behat config?)

@bdunogier
Copy link
Member

For phpunit, it would probably be in bootstrap.php. Behat will be taken care of by the change you've done here, as it uses app.php.

@glye
Copy link
Member Author

glye commented Jan 23, 2017

Ok, that would be bootstrap.php in kernel, repoforms, pui and other repos, then?

@andrerom
Copy link
Contributor

Bootstrap is one thing, we also more importantly need to add this in app/console, right?

@glye glye force-pushed the ezp26748_wrong_image_uri_utf8 branch from 6ba5fe1 to f9e0a58 Compare January 25, 2017 09:29
@glye
Copy link
Member Author

glye commented Jan 25, 2017

Update: Added same in app/console, thanks.
Wondering to what degree console commands are tested here. Locally at least cache clear seems fine with the change. (In most cases it doesn't matter, anyway.)

@yannickroger
Copy link
Contributor

Don't forget to send it to QA 😄

@glye
Copy link
Member Author

glye commented Feb 10, 2017

Sent to QA.

@glye
Copy link
Member Author

glye commented Feb 13, 2017

Merged in 1.7: 45aa9a6

@glye glye closed this Feb 13, 2017
@glye glye deleted the ezp26748_wrong_image_uri_utf8 branch February 13, 2017 15:19
andrerom pushed a commit that referenced this pull request Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants