From d3eda44624e8b27ec8ad5b3979aa2e8dcafe0de1 Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Fri, 13 Aug 2021 12:39:48 -0700 Subject: [PATCH] registration: Make it easier to set a skin's templateDirectory When originally introduced, a skin's templateDirectory was relative to core, rather than the skin itself. Most skins needed to set something like "skins/Example/templates", hardcoding the default value of $wgStyleDirectory. skin.json will now assume that the value of templateDirectory is relative to the skin itself. If that directory doesn't exist, we'll fallback and assume it's using the previous behavior of being relative to core. In practice this should not be a breaking change since no skin is expected to have a directory like "skins/Example/templates" inside the skin itself. Finally, ExtensionProcessor will set a default templateDirectory if one isn't already set, so in most cases this setting should be able to just be removed entirely. Bug: T262067 Change-Id: I434b8a4319f3c8b65bcaab3465224058f31e7ae8 --- includes/registration/ExtensionProcessor.php | 36 ++++++++++- .../registration/ExtensionProcessorTest.php | 59 +++++++++++++++++++ .../registration/FooBar/templates/README.md | 1 + 3 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 tests/phpunit/includes/registration/FooBar/templates/README.md diff --git a/includes/registration/ExtensionProcessor.php b/includes/registration/ExtensionProcessor.php index 0c19e0907df..4dbb3698a70 100644 --- a/includes/registration/ExtensionProcessor.php +++ b/includes/registration/ExtensionProcessor.php @@ -53,8 +53,7 @@ class ExtensionProcessor implements Processor { 'ResourceLoaderSources', 'RevokePermissions', 'SessionProviders', - 'SpecialPages', - 'ValidSkinNames', + 'SpecialPages' ]; /** @@ -195,6 +194,7 @@ class ExtensionProcessor implements Processor { $this->extractHooks( $info, $path ); $this->extractExtensionMessagesFiles( $dir, $info ); $this->extractMessagesDirs( $dir, $info ); + $this->extractSkins( $dir, $info ); $this->extractSkinImportPaths( $dir, $info ); $this->extractNamespaces( $info ); $this->extractResourceLoaderModules( $dir, $info ); @@ -618,6 +618,38 @@ class ExtensionProcessor implements Processor { } } + /** + * Extract skins and handle path correction for templateDirectory. + * + * @param string $dir + * @param array $info + */ + protected function extractSkins( $dir, array $info ) { + if ( isset( $info['ValidSkinNames'] ) ) { + foreach ( $info['ValidSkinNames'] as $skinKey => $data ) { + if ( isset( $data['args'][0]['templateDirectory'] ) ) { + $templateDirectory = $data['args'][0]['templateDirectory']; + $correctedPath = $dir . '/' . $templateDirectory; + // Historically the template directory was relative to core + // but it really should've been relative to the skin directory. + // If the path exists relative to the skin directory, assume that + // is what was intended. Otherwise fall back on the previous behavior + // of having it relative to core. + if ( is_dir( $correctedPath ) ) { + $data['args'][0]['templateDirectory'] = $correctedPath; + } else { + // TODO: deprecate directories relative to core. + $data['args'][0]['templateDirectory'] = $templateDirectory; + } + } elseif ( isset( $data['args'][0] ) ) { + // If not set, we set a sensible default. + $data['args'][0]['templateDirectory'] = $dir . '/templates'; + } + $this->globals['wgValidSkinNames'][$skinKey] = $data; + } + } + } + /** * @param string $dir * @param array $info diff --git a/tests/phpunit/includes/registration/ExtensionProcessorTest.php b/tests/phpunit/includes/registration/ExtensionProcessorTest.php index 1cd8d3bb506..9fe702f2343 100644 --- a/tests/phpunit/includes/registration/ExtensionProcessorTest.php +++ b/tests/phpunit/includes/registration/ExtensionProcessorTest.php @@ -62,6 +62,65 @@ class ExtensionProcessorTest extends MediaWikiIntegrationTestCase { ); } + public function testExtractSkins() { + $processor = new ExtensionProcessor(); + $processor->extractInfo( $this->dir, self::$default + [ + 'ValidSkinNames' => [ + 'test-vector' => [ + 'class' => 'SkinTestVector', + ], + 'test-vector-empty-args' => [ + 'class' => 'SkinTestVector', + 'args' => [] + ], + 'test-vector-empty-options' => [ + 'class' => 'SkinTestVector', + 'args' => [ + [] + ] + ], + 'test-vector-core-relative' => [ + 'class' => 'SkinTestVector', + 'args' => [ + [ + 'templateDirectory' => 'skins/Vector/templates', + ] + ] + ], + 'test-vector-skin-relative' => [ + 'class' => 'SkinTestVector', + 'args' => [ + [ + 'templateDirectory' => 'templates', + ] + ] + ], + ] + ], 1 ); + $extracted = $processor->getExtractedInfo(); + $validSkins = $extracted['globals']['wgValidSkinNames']; + + $this->assertArrayHasKey( 'test-vector', $validSkins ); + $this->assertArrayHasKey( 'test-vector-core-relative', $validSkins ); + $this->assertArrayHasKey( 'test-vector-empty-args', $validSkins ); + $this->assertArrayHasKey( 'test-vector-skin-relative', $validSkins ); + $this->assertSame( + $this->dirname . '/templates', + $validSkins['test-vector-empty-options']['args'][0]['templateDirectory'], + 'A sensible default is provided.' + ); + $this->assertSame( + 'skins/Vector/templates', + $validSkins['test-vector-core-relative']['args'][0]['templateDirectory'], + 'unmodified' + ); + $this->assertSame( + $this->dirname . '/templates', + $validSkins['test-vector-skin-relative']['args'][0]['templateDirectory'], + 'modified' + ); + } + public function testExtractNamespaces() { // Test that namespace IDs defined in extension.json can be overwritten locally if ( !defined( 'MW_EXTENSION_PROCESSOR_TEST_EXTRACT_INFO_X' ) ) { diff --git a/tests/phpunit/includes/registration/FooBar/templates/README.md b/tests/phpunit/includes/registration/FooBar/templates/README.md new file mode 100644 index 00000000000..5905bfa195c --- /dev/null +++ b/tests/phpunit/includes/registration/FooBar/templates/README.md @@ -0,0 +1 @@ +This file exists to support ExtensionProcessorTest which checks for directory existence.