Page MenuHomePhabricator

Flow: use text() or plain() instead of message parse() where possible
Closed, ResolvedPublic

Description

While Flow changed anonymous IP handling, blank IP addresses led to topic participants appearing in a white typewriter text block. This happened because wikitext parsing of a message starting with a blank results in pre-formatted text. In eval.php,

echo wfMessage( 'flow-topic-participants', 2, 2, '', 'second person' )->parse();

(note the empty string for first participant); this prints

<pre>and second person
</pre>

We fixed that bug, but it's wasted effort doing this level of wikitext parsing if we don't need to, and it's prone to embarrassing HTML output. Niklas on mediawiki-i18n list commented

Is there any other wikitext besides the magic words? You could just use ->text() instead of ->parse().

and sure enough if you call text() instead you avoid the additional parsing. I didn't test this message's complex GENDER handling in a language where it matters, but we should check all our messages using parse() (about 13 of them in PHP) and see if we can switch them to ->text(), or escaped() if they're directly output, or even plain(). See https://www.mediawiki.org/wiki/Manual:Messages_API . Use of parse() where we don't intend to be converting wikitext constructs into HTML seems about 25% slower on this string in my dopey testing, and could cause some display bugs.


Version: master
Severity: normal

Details

Reference
bz59644

Event Timeline

bzimport raised the priority of this task from to Needs Triage.
bzimport set Reference to bz59644.
bzimport added a subscriber: Unknown Object (MLST).

bingle-admin wrote:

The WMF core features team tracks this bug on Mingle card https://mingle.corp.wikimedia.org/projects/flow/cards/689, but people from the community are welcome to contribute here and in Gerrit.

Change 105435 had a related patch set uploaded by Spage:
Call msg ->text() or escaped() instead of parse()

https://gerrit.wikimedia.org/r/105435

Change 105435 merged by jenkins-bot:
Call msg ->text() or escaped() instead of parse()

https://gerrit.wikimedia.org/r/105435

bd808 moved this task from Tag to Archive on the Performance board.Mar 11 2015, 7:30 PM