Remove code path in Sanitizer that depends on tidy
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	cscott
	Mar 16 2015, 8:18 PM

Project Tags

Referenced Files

None

Subscribers

Description

As uncovered by https://gerrit.wikimedia.org/r/181777 the _TOC_ magic word emits empty <p></p> tags around the TOC. When HTML tidy is turned *off*, the Sanitizer removes empty tags. When HTML tidy is enabled, the Sanitizer leaves the empty tags alone -- and so does tidy, so they show up in the output.

I think the Sanitizer shouldn't try to do tidy's job. It should leave empty tags alone, regardless.

From parserTests.txt:

# Note that the html output does not have the <p></p>, but the
# html+tidy output *does*.  This is because the empty <p></p> is
# removed by the sanitizer, but only when tidy is *not* enabled (!).
!! test
Empty <p> tag in TOC, removed by Sanitizer (T92892)
!! wikitext
__TOC__
== x ==
!! html
<div id="toc" class="toc"><div id="toctitle"><h2>Contents</h2></div>
<ul>
<li class="toclevel-1 tocsection-1"><a href="#x"><span class="tocnumber">1</span> <span class="toctext">x</span></a></li>
</ul>
</div>

<h2><a href="#x" class="mw-headline-anchor" aria-hidden="true" title="Link to this section">§</a><span class="mw-headline" id="x">x</span><span class="mw-editsection"><span class="mw-editsection-bracket">[</span><a href="/index.php?title=Parser_test&amp;action=edit&amp;section=1" title="Edit section: x">edit</a><span class="mw-editsection-bracket">]</span></span></h2>

!! html+tidy
<p></p>
<div id="toc" class="toc">
<div id="toctitle">
<h2>Contents</h2>
</div>
<ul>
<li class="toclevel-1 tocsection-1"><a href="#x"><span class="tocnumber">1</span> <span class="toctext">x</span></a></li>
</ul>
</div>
<p></p>
<h2><a href="#x" class="mw-headline-anchor" aria-hidden="true" title="Link to this section">§</a><span class="mw-headline" id="x">x</span><span class="mw-editsection"><span class="mw-editsection-bracket">[</span><a href="/index.php?title=Parser_test&amp;action=edit&amp;section=1" title="Edit section: x">edit</a><span class="mw-editsection-bracket">]</span></span></h2>
!! end

Related Objects

Mentioned In: T89331: Replace HTML4 Tidy in MW parser with an equivalent HTML5 based tool
rMW78c3f2f4b146: Tidy up tidy usage
T92999: Better handle `__TOC__` in parser tests

Event Timeline

cscott created this task.Mar 16 2015, 8:18 PM

cscott raised the priority of this task from to Needs Triage.

cscott updated the task description. (Show Details)

cscott added a project: MediaWiki-Parser.

cscott subscribed.

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 16 2015, 8:18 PM

cscott updated the task description. (Show Details)Mar 16 2015, 8:19 PM

cscott set Security to None.

cscott mentioned this in T92999: Better handle `__TOC__` in parser tests.Mar 17 2015, 7:40 PM

cscott mentioned this in rMW78c3f2f4b146: Tidy up tidy usage.Mar 23 2015, 3:51 PM

tstarling mentioned this in T89331: Replace HTML4 Tidy in MW parser with an equivalent HTML5 based tool.Sep 8 2015, 3:35 AM

ssastry triaged this task as Medium priority.Jan 4 2017, 6:33 PM

Sanitizer is in fact not involved. The Parser wraps the TOC with <mw:toc>...</mw:toc>. The input to Tidy is:

<mw:toc><div id="toc" class="toc"><div class="toctitle" lang="en" dir="ltr"><h2>Contents</h2></div>
<ul>
<li class="toclevel-1 tocsection-1"><a href="#x"><span class="tocnumber">1</span> <span class="toctext">x</span></a></li>
</ul>
</div>
</mw:toc>
<h2><span class="mw-headline" id="x">x</span><mw:editsection page="Main Page" section="1">x</mw:editsection></h2>

Tidy wraps both the start and end tags in <p></p>, generating unbalanced output:

<p><mw:toc></p>
<div id="toc" class="toc">
<div class="toctitle" lang="en" dir="ltr" xml:lang="en">
<h2>Contents</h2>
</div>
<ul>
<li class="toclevel-1 tocsection-1"><a href="#x"><span class="tocnumber">1</span> <span class="toctext">x</span></a></li>
</ul>
</div>
<p></mw:toc></p>
<h2><span class="mw-headline" id="x">x</span><mw:editsection page="Main Page" section="1">x</mw:editsection></h2>

ParserOutput::getText() then removes the start and end tags, leaving empty p elements. The solution is to use RemexHtml instead, which adds elements with unknown names to the document tree, as required by the HTML 5 spec. I suggest declining the task.

Interesting. That at least resolves the mystery of why Tidy and Remex seemed like were doing exactly the opposite of what we thought should be doing.

As we discussed, once Tidy is gone, that code path in the Sanitizer can be removed. So, that part of this task: "I think the Sanitizer shouldn't try to do tidy's job. It should leave empty tags alone, regardless." is still relevant.

Arlolra renamed this task from Sanitizer removes empty tags, only when tidy is *disabled* to Remove code path in Sanitizer that depends on tidy.Mar 7 2018, 12:22 AM

Arlolra updated the task description. (Show Details)

Izno added a project: Tidy.Aug 22 2018, 10:35 PM

Tidy is dead. Resolving.

Remove code path in Sanitizer that depends on tidyClosed, ResolvedPublicActions

Description

Related Objects

Event Timeline

Remove code path in Sanitizer that depends on tidy
Closed, ResolvedPublic
Actions