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

[dev/core#412] Avoid truncated UTF-8 strings when using substr() #12935

Merged
merged 1 commit into from
Oct 18, 2018

Conversation

jensschuppe
Copy link
Contributor

@jensschuppe jensschuppe commented Oct 15, 2018

Overview

This fixes broken export forms when a custom group name contains a UTF-8 character at the 10th character position.

See dev/core#412.

Before

The export form can not be used due to a JavaScript error "Uncaught syntax error: Unexpected token ,".

The next column of options does not pop up.

screenshot 2018-10-18 20 53 22

After

The export form works correctly.

screenshot 2018-10-18 20 54 05

Technical Details

CRM_Core_BAO_Mapping::getCustomGroupName() truncates custom group names when longer than 10 characters using substr(), which causes multibyte characters being cut in half when at the truncating position.

This should use CRM_Utils_String::ellipsify() instead, which utilises mb_substr().

Comments

Maybe someone with more core code insight should inspect a grep substr( result for more places where that happens.

@civicrm-builder
Copy link

Can one of the admins verify this patch?

@civibot
Copy link

civibot bot commented Oct 15, 2018

(Standard links)

@civibot civibot bot added the master label Oct 15, 2018
@mattwire
Copy link
Contributor

Nice catch! @eileenmcnaughton @seamuslee001 Apparently this is an export form issue. Are you able to confirm the bug and proposed fix?

@seamuslee001
Copy link
Contributor

Jenkins add to whitelist

@eileenmcnaughton
Copy link
Contributor

I was able to replicate this & confirm the fix. Merging

@eileenmcnaughton eileenmcnaughton merged commit 51ffa35 into civicrm:master Oct 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants