Page MenuHomePhabricator

Linker::userToolLinks uses $wgUser
Closed, ResolvedPublic

Description

Currently:

Linker::userToolLinks
/**
 * Generate standard user tool links (talk, contributions, block link, etc.)
 *
 * @since 1.16.3
 * @param int $userId User identifier
 * @param string $userText User name or IP address
 * @param bool $redContribsWhenNoEdits Should the contributions link be
 *   red if the user has no edits?
 * @param int $flags Customisation flags (e.g. Linker::TOOL_LINKS_NOBLOCK
 *   and Linker::TOOL_LINKS_EMAIL).
 * @param int|null $edits User edit count (optional, for performance)
 * @param bool $useParentheses (optional) Wrap comments in parentheses where needed
 * @return string HTML fragment
 */
public static function userToolLinks(
	$userId, $userText, $redContribsWhenNoEdits = false, $flags = 0, $edits = null,
	$useParentheses = true
) {
[...]
	global $wgUser, $wgDisableAnonTalk, $wgLang;
[...]
	$userCanBlock = MediaWikiServices::getInstance()->getPermissionManager()
		->userHasRight( $wgUser, 'block' );
	if ( $blockable && $userCanBlock ) {
		$items[] = self::blockLink( $userId, $userText );
	}
	if ( $addEmailLink && $wgUser->canSendEmail() ) {
		$items[] = self::emailLink( $userId, $userText );
	}
[...]
}

This should requires a User $viewer to be passed, to avoid using the global state
However, this function has many callers outside of core, so a user cannot be easily added as a required variable before the optional variables, and adding it to the end means that callers will need to duplicate the defaults

Proposal:
Create a new function, Linker::userToolLinksForViewer, that accepts a User parameter.
Convert Linker::userToolLinks to a wrapper for the new function that accesses global $wgUser (or RequestContext::getMain()->getUser(), not much better but allows progress on removing $wgUser in the meanwhile)
Replace uses

The new function makes it clear that the links return depend on the context user viewing the tool links

Event Timeline

You could also bring it to the LinkRenderer service as it holds some other Linker class function.
But that maybe include more technical debt, because this function needs the permission manager, the current function of LinkRenderer does not

emailLink and blockLink calling deprecated link() isself, which must be replaced by a service
SpecialPage::getTitleFor internal is using a service
wfMessage needs a context source

So I am not sure if just replacing the global user by a user passed as argument reduced global state at all in this function.

You could also bring it to the LinkRenderer service as it holds some other Linker class function.
But that maybe include more technical debt, because this function needs the permission manager, the current function of LinkRenderer does not

emailLink and blockLink calling deprecated link() isself, which must be replaced by a service
SpecialPage::getTitleFor internal is using a service
wfMessage needs a context source

So I am not sure if just replacing the global user by a user passed as argument reduced global state at all in this function.

Though not the best solution, perhaps just using RequestContext::getMain()->getUser() is called for?

Change 573414 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Remove use of global $wgUser in Linker::userToolLinks

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

Change 573414 merged by jenkins-bot:
[mediawiki/core@master] Remove use of global $wgUser in Linker::userToolLinks

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

DannyS712 removed a project: Patch-For-Review.

Went with the easy way out: request context