Page MenuHomePhabricator

Echo notication for "new topics" shows raw HTML escape sequence in user interface
Closed, ResolvedPublic

Description

  1. User B watches a talk page of which the title contains an ampersand. For example "Talk:Mission & Values", or this sandbox test case on mediawiki.org/wiki/Talk:Structured Discussions/Sandbox & Box of sand. Simply view the board and ensure the Watchstar is on.
  2. User A creates a new topic on this board. E.g. new topic "Foo"
  3. Another topic is created, e.g. "Bar" either by a third user or the same user A, doesn't matter.

Observe User B browsing the wiki, any page is fine except the talk page in question, and it has a new notification, which when viewed is as follows.

Actual result

It says Stop watching new activity on "Sandbox & Box of sand"

capture.png (802×1 px, 70 KB)

Expected result

It should say Sandbox & Box of sand instead of Sandbox & Box of sand.

Other Info

I initially did not know how to reproduce it after seeing it once. I have confirmed that the notication for a reply to a watched topic is fine, and that the first notification for a single new topic on a watched talk page is also fine. It is only after the first one that the issue appears.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Tgr subscribed.

Probably an issue with DiscussionTools more than Echo itself?

matej_suchanek subscribed.

I think this is StructuredDiscussions (Flow).

ppelberg subscribed.

Meta: I'm going to remove DiscussionTools as Echo is not part of this extension.

The incorrectly displayed message is notification-dynamic-actions-flow-board-unwatch. It is displayed when several new topics were created, so it's a bundled notification

The issue is reproducible on enwiki betalabs:

  • create a Flow page with an ampersand in the title (e.g. Flow test talk:Sandbox & test)
  • make sure that another user watches it - and add couple of topics
  • a user would receive a notification with incorrectly displayed message.

e.g.

<span class="oo-ui-labelElement-label">(notification-dynamic-actions-flow-board-unwatch: Sandbox &amp;amp; test, &lt;a rel="nofollow" class="external free" href="https://en.wikipedia.beta.wmflabs.org/w/index.php?title=Flow_test_talk:Sandbox_%26_test&amp;amp;action=unwatch"&gt;https://en.wikipedia.beta.wmflabs.org/w/index.php?title=Flow_test_talk:Sandbox_%26_test&amp;amp;action=unwatch&lt;/a&gt;, ET1027)<label class="mw-echo-ui-menuItemWidget-description oo-ui-widget oo-ui-widget-enabled oo-ui-labelElement-label oo-ui-labelWidget oo-ui-element-hidden" aria-disabled="false"></label></span>
Screen Shot 2020-12-16 at 3.16.58 PM.png (424×511 px, 54 KB)
Screen Shot 2020-12-16 at 3.45.38 PM.png (441×644 px, 76 KB)
Screen Shot 2020-12-16 at 3.52.25 PM.png (397×521 px, 57 KB)

Re-checked some other notifications for possible regressions in displaying such titles - all seem to be ok.

Tgr added subscribers: Thibaut120094, kostajh, Pamputt and 2 others.

EchoEventPresentationModel::getSecondaryLinks() says 'label' => (string) link text (non-escaped), and indeed they do get escaped in the formatters. So yes it's double-escaping, although confusingly EchoEventPresentationModel::getWatchActionLink(), the method the Flow model replaces, also has the same bug.

Change 755929 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/Flow@master] Avoid double-escaping in notification watch links

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

Change 755930 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/Echo@master] Avoid double-escaping in notification watch links

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

Change 755929 merged by jenkins-bot:

[mediawiki/extensions/Flow@master] Avoid double-escaping in notification watch links

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

Change 755930 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] Avoid double-escaping in notification watch links

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

Umherirrender assigned this task to Tgr.