-
Notifications
You must be signed in to change notification settings - Fork 821
CRM: Fix special character display in task titles and descriptions for international users #44157
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
base: trunk
Are you sure you want to change the base?
Conversation
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the display of international special characters in task titles and descriptions by decoding HTML entities before rendering.
- Implements a helper (decodeHTMLEntities) to decode HTML entities.
- Updates task and list view JavaScript files to use the new decoding function with proper escaping.
- Includes minor updates in the changelog and .gitignore to support the changes.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
projects/plugins/crm/js/jpcrm-admin-tasks.js | Decodes task titles before escaping, ensuring correct tooltip content. |
projects/plugins/crm/js/ZeroBSCRM.admin.listview.js | Replaces direct escaping with a decode-then-escape function in titles and descriptions. |
projects/plugins/crm/js/ZeroBSCRM.admin.global.js | Adds the new helper functions for HTML entity decoding and combined escaping. |
projects/plugins/crm/changelog/fix-crm-task-encoding | Updates the changelog to reflect the fix for special character display issues. |
projects/plugins/crm/.gitignore | Adds new ignore rules for cursor rules files. |
Comments suppressed due to low confidence (3)
projects/plugins/crm/js/jpcrm-admin-tasks.js:98
- Confirm that using the decoded title (which omits the prepended timeText in non-'listMonth' views) for the tooltip is intentional and aligns with the desired UI behavior.
'<div class="event_html" title="' + jpcrm.esc_attr( decodedTitle ) + '">' + html + '</div>';
projects/plugins/crm/js/ZeroBSCRM.admin.listview.js:4887
- Ensure that the new escTextWithDecode function provides the same level of security and escaping as the original esc_html, especially in contexts where user input is rendered.
jpcrm.escTextWithDecode( dataLine.title ) +
projects/plugins/crm/js/ZeroBSCRM.admin.listview.js:4898
- Verify that applying escTextWithDecode for the description field maintains consistency with previous behavior and update tests if necessary to cover any edge cases introduced by the decoding logic.
const td = '<td>' + jpcrm.escTextWithDecode( dataLine.desc ) + '</td>';
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment about ignored files.
This solves the immediate problem. My bigger concern is that we're handling these differently than other objects by unnecessarily converting chars into entities for storage in the database for tasks; ideally we should be stripping tags, saving text as-is and then escaping on output.
.cursor-rules.json | ||
.cursorrules | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were these added intentionally? Ideally ignored files would be files that are expected to exist, but it'd be odd to have these exist at the project level.
Proposed changes:
decodeHTMLEntities()
helper function to properly decode all HTML entities before displayOther information:
Jetpack product discussion
Discussed here: p1749748624776009-slack-CL5UJ9ASE
Does this pull request change what data or activity we track or use?
No changes to data tracking or usage.
Testing instructions:
Before testing:
Test the fix:
Go to CRM → Tasks → List View
ì
,ò
,ù
,À
,È
,Ì
Go to CRM → Tasks → Calendar View
Verify that other views (Edit, View) continue to work correctly (they were already working)
Test with different languages: