From 2058d79330a396c5d79afff5b9ce87dd3db32ee2 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Mon, 29 Jul 2019 23:42:57 +0100 Subject: [PATCH] context: Make the getSkin() fallback logic more explicit Previously the first two post-hook checks where mutually exclusive and the fallback was generic (not conditional) which means we would also allow the first two conditions to succeed in a way that leaves $this->skin as null. Aside from that not being possible right now, it also duplicates a null check in two places, which isn't ideal. Codify that the first two blocks always result in $this->skin being assigned non-null by using an 'else' branch instead of another generic check. Change-Id: I7986684bf1bb2daad3aaddfbcd2ca02e652f78c2 --- includes/context/RequestContext.php | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/includes/context/RequestContext.php b/includes/context/RequestContext.php index 4393abb23d..afc7045e49 100644 --- a/includes/context/RequestContext.php +++ b/includes/context/RequestContext.php @@ -371,33 +371,27 @@ class RequestContext implements IContextSource, MutableContext { Hooks::run( 'RequestContextCreateSkin', [ $this, &$skin ] ); $factory = MediaWikiServices::getInstance()->getSkinFactory(); - // If the hook worked try to set a skin from it if ( $skin instanceof Skin ) { + // The hook provided a skin object $this->skin = $skin; } elseif ( is_string( $skin ) ) { + // The hook provided a skin name // Normalize the key, just in case the hook did something weird. $normalized = Skin::normalizeKey( $skin ); $this->skin = $factory->makeSkin( $normalized ); - } - - // If this is still null (the hook didn't run or didn't work) - // then go through the normal processing to load a skin - if ( $this->skin === null ) { + } else { + // No hook override, go through normal processing if ( !in_array( 'skin', $this->getConfig()->get( 'HiddenPrefs' ) ) ) { - # get the user skin $userSkin = $this->getUser()->getOption( 'skin' ); $userSkin = $this->getRequest()->getVal( 'useskin', $userSkin ); } else { - # if we're not allowing users to override, then use the default $userSkin = $this->getConfig()->get( 'DefaultSkin' ); } - // Normalize the key in case the user is passing gibberish - // or has old preferences (T71566). + // Normalize the key in case the user is passing gibberish query params + // or has old user preferences (T71566). + // Skin::normalizeKey will also validate it, so makeSkin() won't throw. $normalized = Skin::normalizeKey( $userSkin ); - - // Skin::normalizeKey will also validate it, so - // this won't throw an exception $this->skin = $factory->makeSkin( $normalized ); } -- 2.20.1