Page MenuHomePhabricator

Message::text() may return HTML
Open, Needs TriagePublic

Description

Problem
When using Message::text(), it is easy to assume that the method would return text rather than HTML. The documentation does say:

@return string Unescaped message text.

but that's not all that clear because the method name implies that there wouldn't be any HTML anyways.

Solution

  • Rename this method to raw()
  • Create a new text() method that is the same as the previous method, but with the output run through strip_tags() so that any HTML tags will be removed.

Alternatively, clearly document that text is not what is returned but rather html.

Event Timeline

Naming is hard. I would rather not go through another round of API changes.

I would recommend to create interface that accept message objects directly, e.g. Html::element( 'div', [], $messageObj ) to reduce the need for developers to make a choice which output format to use.

I also support updating the documentation to say Message text that can contain unescaped HTML.

[...]
I would recommend to create interface that accept message objects directly, e.g. Html::element( 'div', [], $messageObj ) to reduce the need for developers to make a choice which output format to use.

This is one of the largest annoyances when dealing with messages that should go in an Html element.

I also support updating the documentation to say Message text that can contain unescaped HTML.

Seems like a no-brainer. I dare say that this is an absolute must, considering that there have been issues with unescaped HTML causing all kinds of problems, let's not make things more difficult for the developers.

There is also a plain() function to make the confusion even worse, with the difference between text() and plain() is that text() transforms {{..}} constructs, according to Message.php.