-
-
Notifications
You must be signed in to change notification settings - Fork 824
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
CRM-9362: PCP 'Your Message' should use WYSIWYG #18411
Conversation
(Standard links)
|
@MegaphoneJon was the with-html version in your db? We've hit problems with things like this before because it changes it from being saved as text to saved as html & other code not expecting html |
The original is also at least consistent with what is in the schema xml https://github.com/civicrm/civicrm-core/blob/master/xml/schema/PCP/PCP.xml#L80 if we wanted to change it (not the worst thing in the world) then the schema xml and also this line
|
The screenshots are from a civibuild, so presumably it's what's in the sample data. |
@KarinG you use pcps... |
That looks 👍 👍 |
Ok - well once @seamuslee001's comments are addressed I think we should merge it then. This isn't broadly used so agreement from @KarinG & @MegaphoneJon is OK from a concept-approved POV |
@MegaphoneJon |
@yashodha That's the field I'm changing. Am I misunderstanding the question? |
I'm still nervous about existing saved text in this field - maybe on upgrade we check for presence of html tags & if none run through nl2br |
@MegaphoneJon |
@MegaphoneJon can you make that change to the schema xml? |
Discussed with @seamuslee001 & @kcristiano - merging this & @seamuslee001 will do a follow up with xml fix & a check if purify is needed in the tpl |
Overview
The page where you edit your PCP settings has a
textarea
field where awysiwyg
field should be.Before
After
Technical Details
It's literally a one-word change.
Comments
I don't know when this regressed - but it must have been in SVN times.