Page MenuHomePhabricator

ReverseChronologicalPager produces unbalanced HTML
Closed, ResolvedPublicBUG REPORT

Description

Visit Special:Log or Special:AbuseLog on a wiki of your choice and inspect its HTML. Where the core <ul> list is supposed to end, you will see two closing </ul></ul>, one of them superfluous (Firefox can highlight this).

By default, ReverseChronologicalPager::getFooter dispatches to ::getEndGroup and returns </ul>.
This is included in the output from (the parent) IndexPager::getBody.
It is clear that ::getEndGroup, producing </ul>, should be called exactly as many times as ::getStartGroup, producing the opening <ul>.
However, for pagers not using grouping by date (LogPager, AbuseLogPager, etc.), ::getStartGroup will never be called because $mGroupByDate is false (guarded by ::isHeaderRowNeeded).
Ultimately, ::getEndGroup is called once, whereas ::getStartGroup never, causing ::getBody to output unbalanced HTML.

This was introduced probably during T298638: Make modifications to Pager HTML to add heading separators to support Minerva skinning.

Event Timeline

Change #1228291 had a related patch set uploaded (by Akaza24; author: Akaza24):

[mediawiki/core@master] Fix ReverseChronologicalPager producing unbalanced HTML

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

Change #1228291 merged by jenkins-bot:

[mediawiki/core@master] Fix ReverseChronologicalPager producing unbalanced HTML

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

matmarex assigned this task to Akaza24.
matmarex added subscribers: Akaza24, matmarex.

Thanks for the bug report @matej_suchanek, and thanks for the patch @Akaza24!

Is this a good candidate for backport?