ILIAS-eLearning
UI: Remove StripTags from Text-Inputs
Hi all
While researching for Inconsistent Handling of Titles and Description I came across this transformation in text- and textarea-inputs. I don't think this is the right behavior and it makes implementing the decision above not possible for the new settings form. I'm not adding this as an Issue right now, but as an Improvement, but I can a Mantis ticket if you prefer.
Thanks and best, @kergomard
Again: It woul be good to somehow ensure that the "to display titles and descriptions in plain text (incl. HTML strings)" part is in place before removing the stripping. You might get security issues otherwise.
More important: The JF decision is about titles and descriptions. This PR removes the transformation from all text/textarea inputs. Imo you need full output escaping in place before doing so.
Hi @kergomard,
I pass this back to you for the time being. Please assign back once you are finished.
Thanks!
Hi all
Thanks for the feedback! I applied the changes and hope this is more or less, what you expected. I decided to not provide a getter for $strip_tags_from_input as I think this really should be completely internal.
I'm adding the strip_tags transformation in withInput(), from where I stand this is a hell of a side-effects, but I don't see another way of doing this right now. Worst case, if withInput() is called multiple times, we end up with multiple trafos. I'm glad, if you have a better idea! Just had an idea: We could also replace bool $strip_tags_from_input with ?Transformation $strip_tags_trafo and set the trafo there in the constructor. We then could null it if withoutStripTags() is called. This way we can guarantee only one trafo is set. I think this is better and I just added a commit changing this.
I already added a Jour Fixe tag as that is what I understood from your comment what I should do. Sorry, if I got that wrong!
Looking forward to your feedback and best, @kergomard
Hi @thibsy
Thank you for the hint! ...this makes things a lot easier! Done.
Best, @kergomard
Thx a lot. This needs JF approval before merging. However, from our perspective, this can be merged directly after JF.
Jour Fixe, 17 MAR 2025: We highly appreciate this suggestion and accept the PR for trunk.
