Page MenuHomePhabricator

Simplify MobileFormatter::filterContent (Drop $removeDefaults and $unused) arguments
Closed, ResolvedPublic

Description

Story

At the moment MobileFormatter::filterContent is declared like this:

public function filterContent(
	$removeDefaults = true, $unused = false, $removeImages = false,
	$showFirstParagraphBeforeInfobox = false
) {
...
	$isSpecialPage = $this->title->isSpecialPage();

		// Don't remove elements in special pages
	if ( !$isSpecialPage && $removeDefaults ) {
		$this->remove( $removableClasses );
	}
...
  1. This is wrong and violates the OOP overriding: HtmlFormatter::filterContent has no arguments in signature https://gerrit.wikimedia.org/g/HtmlFormatter/+/aef4a7e6186cc03fedd2570386cb047537fd6180/src/HtmlFormatter.php#150:
public function filterContent() {
  1. The only place where we use none-default arguments is in https://gerrit.wikimedia.org/g/mediawiki/extensions/MobileFrontend/+/6155573221f93d2fa14420b03e54fefa08a997f5/includes/ExtMobileFrontend.php#104

and the first argument intersects with callee if ( !$isSpecialPage && $removeDefaults ) and in real life can be omitted.

if ( $context->getContentTransformations() ) {
	$isSpecialPage = $title->isSpecialPage();

	// Remove images if they're disabled from special pages, but don't transform otherwise
	$formatter->filterContent( !$isSpecialPage,
		null,
		$removeImages, $showFirstParagraphBeforeInfobox );
}
Note:

MobileFrontend is the only client that uses those arguments so they can be amended eligibly.

Event Timeline

@Peter.ovchyn: Assuming this task is about the MobileFrontend code project, hence adding that project tag so other people who don't know or don't care about team tags can also find this task when searching via projects. Feel free to correct. (Please set appropriate project tags when possible.) Thanks!

Change 623812 had a related patch set uploaded (by Peter.ovchyn; owner: Peter.ovchyn):
[mediawiki/extensions/MobileFrontend@master] [Hygiene] Remove $removeDefaults and $unused from filterContent signature.

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

The patch is merged but 2 arguments remain so moving to "needs more work"

Change 623812 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] [Hygiene] Remove $removeDefaults and $unused from filterContent signature.

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

The patch is merged but 2 arguments remain so moving to "needs more work"

@Jdlrobson I moved this task to T261769: MobileFormatter follow up work, It will be done there. We could close this one.