Page MenuHomePhabricator

Remove linktarget when calling PrevNextNavigationRenderer::buildPrevNextNavigation
Open, Needs TriagePublic

Description

PrevNextNavigationRenderer::buildPrevNextNavigation is getting a LinkTarget and passed that to Message::title

The problem is that Message::title takes an Title object and cannot take a LinkTarget (it is passed later to Parser object)

On the other side PrevNextNavigationRenderer takes a MessageLocalizer which normally has a context and should provide a Message object with the correct title set up, so the extra title() call seems useless to me.

In my opinion the title should be removed from the function. It was added in 1.34, so it could be changed without deprecation.

So my question is: Have I miss something and this title was added for a reason or is there a way to improve this?

See T207977 and https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/513240/

This was found by phan

<file name="includes\Navigation\PrevNextNavigationRenderer.php">
  <error line="57" severity="warning" message="Argument 1 (title) is \MediaWiki\Linker\LinkTarget|string but \Message::title() takes \Title defined at includes\Message.php:827" source="PhanTypeMismatchArgument"/>
  <error line="68" severity="warning" message="Argument 1 (title) is \MediaWiki\Linker\LinkTarget|string but \Message::title() takes \Title defined at includes\Message.php:827" source="PhanTypeMismatchArgument"/>
  <error line="85" severity="warning" message="Argument 1 (title) is \MediaWiki\Linker\LinkTarget|string but \Message::title() takes \Title defined at includes\Message.php:827" source="PhanTypeMismatchArgument"/>
  <error line="105" severity="warning" message="Argument 1 (title) is \MediaWiki\Linker\LinkTarget|string but \Message::title() takes \Title defined at includes\Message.php:827" source="PhanTypeMismatchArgument"/>
</file>

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 19 2019, 7:29 PM
Daimona added a subscriber: Daimona.Mon, Sep 2, 8:34 AM

Note, in https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/533703/7/includes/Navigation/PrevNextNavigationRenderer.php I changed LinkTarget back to Title. And it doesn't end here: there's a call to $this->messageLocalizer->getLanguage();, but MessageLocalizer doesn't have a getLanguage() method.