Page MenuHomePhabricator

ChangeTags::formatSummaryRow needs a context
Closed, ResolvedPublic

Description

ChangeTags::formatSummaryRow is using $wgLang (user language) and wfMessage which results in use of global $wgTitle.

A IContextSource should be added as parameter to this static function, which can be used to avoid the globals

Event Timeline

Umherirrender raised the priority of this task from to Needs Triage.
Umherirrender updated the task description. (Show Details)
Umherirrender subscribed.

I would like to work on this bug. Since I am new to MediaWiki, it would be helpful if someone could point me in the right direction.

According to me the changes should be adding a parameter IContextSource $context to the function, and change

wfMessage('tag-list-wrapper') to $context->msg( 'tag-list-wrapper' )

and

$wgLang->commaList( $displayTags ) to $context->getLanguage()->commaList( $displayTags )

The next thing to do would be to define an IContextSource instance in all functions which reference this function and pass it in formatSummaryRow function call. If what I assumed is correct, I would be grateful for help in doing the same, and also in understanding how to test my changes.

Hi Aashaka, thanks for your interest in working on this! Have you checked https://www.mediawiki.org/wiki/How_to_become_a_MediaWiki_hacker already? It covers ways to test your code, and how to publish your patch. Thanks :)

Praveenraj unsubscribed.
Praveenraj subscribed.

Hi I am new to MediaWiki, Where can I find the source code, where bug is reported

Thanks for your reply Aklapper. I was travelling for a while. I have gone through it. I couldn't find a unit test for the ChangeTags class.

There are no unit tests for the ChangeTags class at the moment. That's a technical debt issue which I don't think it is germane to address as part of this task.

I see. I was going through the https://www.mediawiki.org/wiki/Manual:Pre-commit_checklist, which included performing unit tests. I suppose I will just test it by running it on Vagrant.

Change 267250 had a related patch set uploaded (by Aashaka):
Add IContextSource as parameter to ChangeTags::formatSummaryRow

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

Florian claimed this task.

Sorry @Aashaka that I haven't respond to your question :( In this case, it's ok to use the singleton function as it's easy to bypass it's usage by the caller of the function and you use it for backwards compatibility, only (so any caller should update the code in the near future :P).

Change 267250 merged by jenkins-bot:
Add IContextSource as parameter to ChangeTags::formatSummaryRow

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