Page MenuHomePhabricator

Replace fragile addWikiTextAsInterface("<div class=....>$something</div>") pattern
Open, Needs TriagePublic

Description

A number of places (cf https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/462753/10/includes/FileDeleteForm.php) contain a pattern like:

$wgOut->addWikiTextAsInterface('<div class="error">' . $something . '</div>');

Of course, (a) this breaks if $something has an extra </div>, and (b) the $something gives up its start-of-line context in an unpredictable way (something the pattern has a newline after the opening <div>, sometimes it doesn't).

It would be better to introduce a new interface which uses the (robust) wrapping functionality of ParserOutput instead:

$wgOut->wrapWikiTextAsInterface('error', $something);

Since the output of parsing $something is tidied by the addWikiTextAsInterface method, this would ensure that the wrapper is robust.

A related problem is placed which do addWikiTextAsInterface("<p class=....>$something</p>"). This is even more fragile, since a double-newline or a <div> will break the wrapper.

A few instances wrap calls to Html::errorBox, like:
https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/462753/10/includes/page/Article.php
but as far as I can tell no one uses the $heading option to errorBox, so these should be straightforward to replace with $wgOut->addWikiTextAsInterfaceWrapped( 'errorbox', ...)

There's also an OutputPage::wrapWikiMsg() function with similar functionality; it would be nice if it could be made safe, for example by parsing the content in a tidy safe and *then* composing it with the wrapper. We could deprecate the case where the first argument contains a "$1", and then treat other strings as wrapper classes (like wrapWikiTextAsInterface) and perhaps accept an array of attribute->value mappings for more advanced wrapping -- the common case seems to be to add dir and lang attributes in addition to the class.

Event Timeline

cscott created this task.Sep 27 2018, 1:58 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 27 2018, 1:58 PM
cscott updated the task description. (Show Details)Sep 27 2018, 2:11 PM
cscott updated the task description. (Show Details)
cscott updated the task description. (Show Details)Sep 27 2018, 2:14 PM
cscott updated the task description. (Show Details)Sep 27 2018, 2:22 PM

Change 463260 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] Special:NewFiles - ensure top text is entirely wrapped

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

Change 463272 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] Special:Import - wrap error messages with <div> not <p>

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

Change 463287 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] WIP: Add safer methods to wrap wikitext on OutputPage

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

Change 463295 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] Use <div> wrappers instead of <p> in ProtectionForm

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

Change 463272 merged by jenkins-bot:
[mediawiki/core@master] Special:Import - wrap error messages with <div> not <p>

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

Change 463260 merged by jenkins-bot:
[mediawiki/core@master] Special:NewFiles - ensure top text is entirely wrapped

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

Change 463295 merged by jenkins-bot:
[mediawiki/core@master] Use <div> wrappers instead of <p> in ProtectionForm

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

Change 467873 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] Simplify arguments of OutputPage::wrapWikiMsg()

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

Change 463287 merged by jenkins-bot:
[mediawiki/core@master] Add OutputPage::wrapWikiTextAsInterface() to safely wrap wikitext

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

Change 467876 had a related patch set uploaded (by Jforrester; owner: C. Scott Ananian):
[mediawiki/core@REL1_32] Add OutputPage::wrapWikiTextAsInterface() to safely wrap wikitext

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

Change 467876 merged by jenkins-bot:
[mediawiki/core@REL1_32] Add OutputPage::wrapWikiTextAsInterface() to safely wrap wikitext

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

Change 467993 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] Use OutputPage::wrapWikiTextAsInterface() to add safe <div> wrappers

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

Change 467993 merged by jenkins-bot:
[mediawiki/core@master] Use OutputPage::wrapWikiTextAsInterface() to add safe <div> wrappers

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

Change 467995 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] Remove untidy wrapper from ImagePage::makeMetadataTable()

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

Change 467995 merged by jenkins-bot:
[mediawiki/core@master] Remove untidy wrapper from ImagePage::makeMetadataTable()

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