Page MenuHomePhabricator

AbuseFilter abusefilter-warning rendered with <div class="mw-parser-output"> in text
Closed, ResolvedPublic

Description

Triggering an abuse filter on azb.wikipedia.org results in the following:

The abusefilter-warning message takes the name of the filter as its first parameter, but somehow it's being wrapped in mw-parser-output and then html-escaped.

User: Logged-in
User language: English
Interface message: Default (not overridden)

Event Timeline

Krinkle created this task.Aug 13 2017, 2:05 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 13 2017, 2:05 PM

but somehow it's being wrapped in mw-parser-output

It uses OutputPage::parseInline() for some reason to parse the filter description, and doesn't set the $interface parameter to true. (here)

and then html-escaped

Status::getErrorMessage() does that. (here)

Change 390074 had a related patch set uploaded (by Melos; owner: Melos):
[mediawiki/extensions/AbuseFilter@master] Disable WrapOutputClass when af_public_comments is parsed

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

Huji added a subscriber: Huji.Mar 5 2018, 3:00 AM

@Melos, @Krinkle: so I don't understand this. The picture shown above would only make sense if the public description of the rule contains HTML tags. Is that really the case? It doesn't make much sense because the list of filters on Special:AbuseFilter would also show the tags in plain text (i.e. escaped, like example below) and that is how it should be anyway. Am I misunderstanding something here? Is the azbwiki filter public or can you at least describe what it's public description is?

Krinkle added a comment.EditedMar 5 2018, 6:48 PM

I know how to reproduce the issue, and I know the current output is wrong. However, I don't know more than that, sorry.

What I do know is that the issue I reported is about the filter description, and I see it happen on the edit page as part of a triggered warning. Not on a special page.

Anomie added a comment.Mar 5 2018, 7:33 PM

@Huji: AbuseFilter is inconsistent, so it's not surprising that you don't understand it.

  • In some places, such as the screenshot you included from Special:AbuleFilter, the field is treated plain text.
  • In other places, such as Special:AbuseLog and the "preview selected message" feature when editing a filter, it's treated as wikitext and gets parsed to HTML.
  • And in some places, such as the warning produced on the edit page when the filter actually fires, it's treated as wikitext and parsed to HTML and then that HTML is treated as wikitext and parsed a second time, which breaks links in the original "wikitext" and leads to display as shown in this task's description.

What really needs to happen is that one interpretation, plain text or wikitext, needs to be chosen and used everywhere. IMO it should just be plain text. And then either way the edit page warning needs to be fixed to not double-parse.

Huji added a comment.Mar 6 2018, 12:26 AM

Thanks for explaining @Anomie! I agree that it should be just plain text everywhere.

Melos added a comment.Mar 7 2018, 4:38 PM
  • In some places, such as the screenshot you included from Special:AbuleFilter, the field is treated plain text.

Correct, it is only a description so plain text is correct IMHO.

  • In other places, such as Special:AbuseLog and the "preview selected message" feature when editing a filter, it's treated as wikitext and gets parsed to HTML.

Here there is the first problem, we need to change it in plain text (see for ex. T141670).

  • And in some places, such as the warning produced on the edit page when the filter actually fires, it's treated as wikitext and parsed to HTML and then that HTML is treated as wikitext and parsed a second time, which breaks links in the original "wikitext" and leads to display as shown in this task's description.

What really needs to happen is that one interpretation, plain text or wikitext, needs to be chosen and used everywhere. IMO it should just be plain text. And then either way the edit page warning needs to be fixed to not double-parse.

I agree, plain text is the better choose and so we can also remove parseinline.

Krinkle removed a subscriber: Krinkle.
Huji added a comment.Mar 9 2018, 2:51 AM
  • In other places, such as Special:AbuseLog and the "preview selected message" feature when editing a filter, it's treated as wikitext and gets parsed to HTML.

Here there is the first problem, we need to change it in plain text (see for ex. T141670).

As far as the preview feature is concerned, the culprit is this line in which mw.Api.parse() is used.

The issue is we can't not use mw.Api.parse() here because the $1 parameter has to be parsed. Do we have a way to parse only the parameters, but escape the HTML? If not, another alternative is to make sure what we send to the API is escaped. That'll fix the issue with HTML tags, but the parameter will still be parsed.

Anomie added a comment.Mar 9 2018, 8:39 PM

You might try wrapping the parameter in <nowiki> before passing the thing to be parsed, and running it through mw.html.escape() for good measure.

Change 418584 had a related patch set uploaded (by Melos; owner: Melos):
[mediawiki/extensions/AbuseFilter@master] Always show abuse filter public comments as plain text

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

Change 390074 abandoned by Melos:
Fix abusefilter-warning render

Reason:
abandon for Change-Id: I173ffab1a99c1536cca260b76be0d95a4966b139

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

Stryn added a subscriber: Stryn.Mar 28 2018, 2:07 PM

Abusefilter warning messages are broken also on the Finnish Wikipedia

Change 418584 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Always show abuse filter public comments as plain text

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

Melos closed this task as Resolved.Mar 31 2018, 10:42 AM
Melos claimed this task.
Melos removed a project: Patch-For-Review.