Page MenuHomePhabricator

Extra </div> output by CleanChanges if tidy off messes up monobook and creates invalid html
Closed, ResolvedPublic

Description

Makes monobook look ugly. See conversation with nemo in MediaWiki-General

https://translatewiki.net/w/i.php?title=Special:RecentChanges&limit=1&cleanrc=1&useskin=monobook

Presumably the extra </div> added in endRecentChangesList(). Perhaps that was a work around to a core bug that has since been fixed?


Version: unspecified
Severity: normal
Whiteboard: gci2013 https://www.google-melange.com/gci/task/view/google/gci2013/5825105095557120

Details

Reference
bz58439

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 2:19 AM
bzimport set Reference to bz58439.
bzimport added a subscriber: Unknown Object (MLST).

(In reply to comment #0)

Presumably the extra </div> added in endRecentChangesList(). Perhaps that
was a work around to a core bug that has since been fixed?

That div is supposed to close the div that is opened in beginRecentChangesList(), isn't it?

Change 103746 had a related patch set uploaded by M4tx:
Fix extra </div> output creating invalid HTML

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

After about 2 hours of investigating, I finally, I think, know everything about this bug.

Look at the EnhancedChangesList.php in old MW (dunno which version exactly):

public function endRecentChangesList() {
return $this->recentChangesBlock() . parent::endRecentChangesList();
}
(FYI: parent::endRecentChangesList() returns '</ul>' or '')

And new one:

public function endRecentChangesList() {
return $this->recentChangesBlock() . '</div>';
}

What was the problem?

CleanChanges_body.php:

public function endRecentChangesList() {
return parent::endRecentChangesList() . '</div>';
}

As you probably think already, the function above was really something like:
return $this->recentChangesBlock() . '</div>' . '</div>';

And that caused the problem.
There wasn't such problem in older version of MediaWiki, because ChangesList::endRecentChangesList() returned only '' or '</ul>', so adding '</div>' was necessary. That changed in newer version of MW, which caused an extension to output '</div>' 2 times.

The only matter that I'm thinking about now is that my patch caused the extension be unusable (causing the same problem) in older versions of MediaWiki... Is it a serious problem?

PS. "older version of MediaWiki" means here something between 1.23alpha and current git version. I don't know really; I have a copy of it on my personal server (I think I downloaded it as a zip from official MW website...)

Change 103746 merged by jenkins-bot:
Remove extra </div> creating invalid HTML

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