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

APIv4 - Fix html encoding of rich-text fields #26251

Merged
merged 1 commit into from
May 18, 2023

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented May 17, 2023

Overview

Fixes the same bug in APIv4 that was fixed for v3 in #12841
See https://chat.civicrm.org/civicrm/pl/oc55t77c3iyetehua71zwbqi5y

Before

Rich text custom fields do not display correctly if saved via APIv4.

After

Fixed.

Technical Details

In ece8de2 this was fixed but only for APIv3, and with no unit test. The previous fix also did not cover fields using "TextArea" as their input type, even though they are allowed to store HTML.
This fixes APIv4 and v3 and adds a test for both.

@civibot
Copy link

civibot bot commented May 17, 2023

(Standard links)

@civibot civibot bot added the master label May 17, 2023
In ece8de2 this was fixed but only for APIv3, and with no unit test.
The previous fix also did not cover fields using "TextArea" as their input type,
even though they are allowed to store HTML.
This fixes for APIv4 and v3 and adds a test. and adds a test.
@seamuslee001 seamuslee001 merged commit 5701f19 into civicrm:master May 18, 2023
@seamuslee001 seamuslee001 deleted the fixHtmlCoding branch May 18, 2023 21:50
@colemanw
Copy link
Member Author

@demeritcowboy funny that the unit test added in d80e8fa#diff-6a4e0910b489565b79b2cc8b5786e9345b9b955c4557981dc7614b04e7e5451eR19 includes a case for Windows... but intentionally fails that case.
Maybe you could add a Win case that's supposed to pass?

@demeritcowboy
Copy link
Contributor

I think it's that absolute paths are supposed to fail, so that is probably correct. But yeah I can try adding one that should pass.

@demeritcowboy
Copy link
Contributor

Test added at #26285

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.

3 participants