Page MenuHomePhabricator

Flow: Raw markup showing up in weird places
Closed, ResolvedPublic

Description

Topic history

Visit recent changes on ee-flow (http://ee-flow.wmflabs.org/wiki/Special:RecentChanges) – next to the expected username/talk links, there's a strange new field with raw wikimarkup (e.g., "[[User:$1|$1]] added a [$2 comment]").

This also appears in the history of some topics.[1] See attachment for screenshot.

1.http://ee-flow.wmflabs.org/w/index.php?title=Special:Flow/Sandbox&workflow=0507ac1dd4010e132562fa163e68c4ac&action=topic-history


Version: master
Severity: normal

Attached:

Screen_Shot_2013-10-28_at_2.00.05_PM.png (479×722 px, 47 KB)

Details

Reference
bz56277

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 2:15 AM
bzimport set Reference to bz56277.

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

This seems to be basically because RecentChanges formatting was totally broken by the implementation of history.

There is a whole system designed for providing an i18n message along with callbacks to fill the parameters. The callback seems to assume it's being called within a block, with access to the PostRevision in question.

The change did not consider that RecentChanges also needs to format history lines, and so we are stuck trying to reimplement the functionality in HistoryRecord, or to instantiate our own HistoryRenderer (the best option, since HistoryRecords seem to expect particular parameters which are set in HistoryRenderer – something that, IMO, is a mistake).

I'll discuss this at standup tomorrow / with Matthias.

Change 92903 had a related patch set uploaded by Matthias Mullie:
(bug 56277) Update RecentChanges to use history i18n

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

Patch was already in Gerrit: https://gerrit.wikimedia.org/r/#/c/92903/

I had indeed completely broken this with the history patch, which was also already using these same messages (as does, I think, the display of a moderated message, which I'll be fixing next)

Andrew: I agree that I'd prefer to have the parameters in HistoryRecord too: I'd prefer to only have to call HistoryRecord::getMessage() without any parameters, and have that method take care of it by itself.

However, some of the messages are rather complex, and need info that is only available in separate objects. If we were to let HistoryRecord deal with those by itself, it would have to have all these objects injected already. I've for now decided to let HistoryRenderer inject these when needed, via the getMessage() call. If you can think of a neater solution, by all means, I'd love it :)

Change 92903 merged by jenkins-bot:
(bug 56277) Update RecentChanges to use history i18n

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

The Mingle card is marked fixed, so declaring this fixed.