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

Check for required mbstring extension #9278

Merged
merged 2 commits into from
Oct 18, 2016
Merged

Conversation

colemanw
Copy link
Member

No description provided.

@colemanw
Copy link
Member Author

Grepping around our codebase, we have a habit of always using mbstring functions conditionally. This breaks with that pattern, mostly because of #9268. What do we think?

@colemanw
Copy link
Member Author

colemanw commented Oct 17, 2016

I'm having second thoughts about this because SOAP is rarely used. I'm now inclined to:

  • Keep this PR but downgrade it from "error" to "warning"
  • Wrap the news widget's use of mb_substr in a conditional

@nganivet
Copy link
Contributor

Without understanding much of the context ... our database is UTF-8, anyone can type anything in all text fields (ie. I could well enter Russian or Chinese names on any contribution form), and PHP notably breaks if we do not use the mb_* functions with such input.

So I would +1 mandating the mbstring extension as this PR suggests, but also wrap the use in a conditional to be consistent with the rest of the code base. Upvoted Coleman's 2nd suggestion.

@seamuslee001
Copy link
Contributor

I am a + on the need for mbstring, I think that it should be needed as it is useful and since php4

@seamuslee001
Copy link
Contributor

I am satisfied with Coleman's 2nd option.

@colemanw colemanw merged commit 759bb8d into civicrm:4.7.13-rc Oct 18, 2016
@colemanw colemanw deleted the mbstring branch October 18, 2016 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants