Page MenuHomePhabricator

JS-parser for reasons on block/delete/protect forms too simplistic
Closed, ResolvedPublic

Description

https://gerrit.wikimedia.org/r/330447 (T34950: Use jQuery.suggestions to add reason suggestions to block/delete/protect forms) added suggestions for block/delete/protect reasons, but the way the message with the reasons is parsed breaks in several cases.

See https://en.wikipedia.org/w/index.php?title=MediaWiki:Deletereason-dropdown&action=edit for how the message with the reasons looks in a real wiki, or at https://de.wikipedia.beta.wmflabs.org/w/index.php?title=MediaWiki:Deletereason-dropdown&action=edit (this is where I tested).

  • Headers starting with just one * are appended to the previous reason, e.g. I get the suggestion "Unerwünschte Wiederanlage einer gelöschten Seite, siehe dazu [[WP:LP|Löschprüfung]]* Aufräumarbeiten und Organisatorisches", where everything after the * shouldn't be there.
  • When the ** isn't followed by a space (which isn't mandatory), the reason isn't suggested. E.g. I don't get "[[WP:VAND|Seiteninhalt war Unsinn]]" as suggestion.
  • Context-specific reasons inside a {{switch}} are either not shown at all (e.g. if they are prepended by spaces like in de.wikipedia), or always shown, some of them with appended syntax from the next lines.

Event Timeline

Schnark created this task.Jan 11 2017, 8:55 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 11 2017, 8:55 AM
Filip claimed this task.Jan 11 2017, 1:19 PM

Hmm, you're right. Missed this!

Filip added a comment.EditedJan 11 2017, 1:27 PM

Code will be parsed in php files, by addJsConfig function. Like in PS1. It will be simplier to parse this in php.

Thanks for you report!

Filip added a comment.Jan 11 2017, 4:07 PM

Looks like "Xml::listDropdown" does exacly what we want. I'll just use modified version of this code to create new function which only parses this data.

Change 331628 had a related patch set uploaded (by Filip):
Improved parsing in reason suggests

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

@Schnark: Thanks for reporting this! I wasn't aware of such structures in this message. Thanks @FilipGCI for fixing this! :)

Florian closed this task as Resolved.Jan 11 2017, 6:41 PM

Change 331628 merged by jenkins-bot:
Improved parsing in reason suggests

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