Merge "Setup: Move wgSkipSkins appendix to Skin::getAllowedSkins"

includes/DefaultSettings.php

@@ -3350,8 +3350,13 @@ $wgFallbackSkin = 'fallback';
 
 /**
  * Specify the names of skins that should not be presented in the list of
- * available skins in user preferences. If you want to remove a skin entirely,
- * remove it from the skins/ directory and its entry from LocalSettings.php.
+ * available skins in user preferences.
+ *
+ * NOTE: This does not uninstall the skin, and it will still be accessible
+ * via the `useskin` query parameter. To uninstall a skin, remove its inclusion
+ * from LocalSettings.php.
+ *
+ * @see Skin::getAllowedSkins
  */
 $wgSkipSkins = [];
 

includes/Setup.php

@@ -379,9 +379,6 @@ $wgDefaultUserOptions['watchlistdays'] = min(
 );
 unset( $rcMaxAgeDays );
 
-$wgSkipSkins[] = 'fallback';
-$wgSkipSkins[] = 'apioutput';
-
 // Set default shared prefix
 if ( $wgSharedPrefix === false ) {
 	$wgSharedPrefix = $wgDBprefix;

includes/preferences/DefaultPreferencesFactory.php

@@ -1343,7 +1343,7 @@ class DefaultPreferencesFactory implements PreferencesFactory {
 		$mptitle = Title::newMainPage();
 		$previewtext = $context->msg( 'skin-preview' )->escaped();
 
-		# Only show skins that aren't disabled in $wgSkipSkins
+		// Only show skins that aren't disabled
 		$validSkinNames = Skin::getAllowedSkins();
 		$allInstalledSkins = Skin::getSkinNames();
 

includes/skins/Skin.php

@@ -71,6 +71,10 @@ abstract class Skin extends ContextSource {
 
 		$allowedSkins = self::getSkinNames();
 
+		// Internal skins not intended for general use
+		unset( $allowedSkins['fallback'] );
+		unset( $allowedSkins['apioutput'] );
+
 		foreach ( $wgSkipSkins as $skip ) {
 			unset( $allowedSkins[$skip] );
 		}

tests/phpunit/includes/skins/SkinTest.php

@@ -14,4 +14,44 @@ class SkinTest extends MediaWikiTestCase {
 		$this->assertTrue( isset( $modules['core'] ), 'core key is set by default' );
 		$this->assertTrue( isset( $modules['styles'] ), 'style key is set by default' );
 	}
+
+	/**
+	 * @covers Skin::getAllowedSkins
+	 */
+	public function testGetAllowedSkinsEmpty() {
+		$skin = $this->getMockBuilder( Skin::class )
+			->setMethods( [ 'outputPage' ] )
+			->getMock();
+
+		$this->setService( 'SkinFactory', new SkinFactory() );
+		$this->setMwGlobals( 'wgSkipSkins', [] );
+
+		$this->assertEquals( [], $skin->getAllowedSkins() );
+	}
+
+	/**
+	 * @covers Skin::getAllowedSkins
+	 */
+	public function testGetAllowedSkins() {
+		$skin = $this->getMockBuilder( Skin::class )
+			->setMethods( [ 'outputPage' ] )
+			->getMock();
+		$noop = function () {
+		};
+
+		$sf = new SkinFactory();
+		$sf->register( 'foo', 'Foo', $noop );
+		$sf->register( 'apioutput', 'ApiOutput', $noop );
+		$sf->register( 'quux', 'Quux', $noop );
+		$sf->register( 'fallback', 'Fallback', $noop );
+		$sf->register( 'bar', 'Barbar', $noop );
+
+		$this->setService( 'SkinFactory', $sf );
+		$this->setMwGlobals( 'wgSkipSkins', [ 'quux' ] );
+
+		$this->assertEquals(
+			[ 'foo' => 'Foo', 'bar' => 'Barbar' ],
+			$skin->getAllowedSkins()
+		);
+	}
 }