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 added a subscriber: Umherirrender.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 12 2015, 7:44 PM
Aashaka added a subscriber: Aashaka.EditedJan 20 2016, 6:57 PM

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.

Restricted Application added a subscriber: StudiesWorld. · View Herald TranscriptJan 20 2016, 6:57 PM

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 removed a subscriber: Praveenraj.
Praveenraj added a subscriber: Praveenraj.

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.

TTO added a subscriber: TTO.Jan 27 2016, 8:49 AM

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.

@Praveenraj: Hi and welcome! Please see https://www.mediawiki.org/wiki/How_to_become_a_MediaWiki_hacker and use that for general questions. Thanks!

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

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

Florian closed this task as Resolved.Feb 12 2016, 7:33 PM
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).

Florian reassigned this task from Florian to Aashaka.Feb 12 2016, 7:33 PM
Florian removed a project: Patch-For-Review.

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

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