diff --git a/i18n/en.json b/i18n/en.json index 6df38c5c..f2e1f408 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -56,6 +56,7 @@ "gadgets-validate-invalidnamespaces": "The following namespace {{PLURAL:$2|number does|numbers do}} not exist: $1", "gadgets-validate-invalidtargets": "Allowed values for targets are: mobile, desktop. Found invalid {{PLURAL:$2|value|values}}: $1", "gadgets-validate-invalidtitle": "Page title \"$1\" is invalid", + "gadgets-validate-unknownpages": "Contains one or more pages without .js, .css or .json suffix. They would not be used.", "gadgets-validate-nopage": "Page \"$1\" does not exist.", "right-gadgets-definition-edit": "Edit gadget definitions", "action-gadgets-definition-edit": "edit this gadget definition", diff --git a/i18n/qqq.json b/i18n/qqq.json index 190cb804..6cdf1103 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -70,6 +70,7 @@ "gadgets-validate-invalidnamespaces": "Warning message to indicate that the namespaces are not recognised. Parameters:\n* $1 - comma separated list of invalid namespaces, $2 - number of items in list $1", "gadgets-validate-invalidtargets": "Warning message to indicate that the targets are not recognised. Parameters:\n* $1 - comma separated list of invalid targets, $2 - number of items in list $1", "gadgets-validate-invalidtitle": "Warning message to indicate that the page title is invalid. Parameters:\n* $1 - page name", + "gadgets-validate-unknownpages": "Warning message to indicate that pages without .js, .css or .json suffix are not recognised.", "gadgets-validate-nopage": "Warning message to indicate the script/style/json page does not exist. Parameters:\n* $1 - page name", "right-gadgets-definition-edit": "{{doc-right|gadgets-definition-edit}}", "action-gadgets-definition-edit": "{{doc-action|gadgets-definition-edit}}", diff --git a/includes/Content/GadgetDefinitionContent.php b/includes/Content/GadgetDefinitionContent.php index 60ba5212..038d5adb 100644 --- a/includes/Content/GadgetDefinitionContent.php +++ b/includes/Content/GadgetDefinitionContent.php @@ -49,7 +49,7 @@ class GadgetDefinitionContent extends JsonContent { * @return string */ public function beautifyJSON() { - // @todo we should normalize entries in module.scripts and module.styles + // @todo we should normalize entries in module.pages return FormatJson::encode( $this->getAssocArray(), "\t", FormatJson::UTF8_OK ); } diff --git a/includes/Content/GadgetDefinitionContentHandler.php b/includes/Content/GadgetDefinitionContentHandler.php index 70cf5d86..38c40415 100644 --- a/includes/Content/GadgetDefinitionContentHandler.php +++ b/includes/Content/GadgetDefinitionContentHandler.php @@ -68,9 +68,7 @@ class GadgetDefinitionContentHandler extends JsonContentHandler { 'supportsUrlLoad' => false, ], 'module' => [ - 'scripts' => [], - 'styles' => [], - 'datas' => [], + 'pages' => [], 'peers' => [], 'dependencies' => [], 'messages' => [], @@ -91,12 +89,10 @@ class GadgetDefinitionContentHandler extends JsonContentHandler { // Create a deep clone. FIXME: unserialize(serialize()) is hacky. $data = unserialize( serialize( $content->getData()->getValue() ) ); if ( $data !== null ) { - foreach ( [ 'scripts', 'styles', 'datas' ] as $type ) { - if ( isset( $data->module->{$type} ) ) { - foreach ( $data->module->{$type} as &$page ) { - $title = Title::makeTitleSafe( NS_GADGET, $page ); - $this->makeLink( $parserOutput, $page, $title ); - } + if ( isset( $data->module->pages ) ) { + foreach ( $data->module->pages as &$page ) { + $title = Title::makeTitleSafe( NS_GADGET, $page ); + $this->makeLink( $parserOutput, $page, $title ); } } foreach ( $data->module->dependencies as &$dep ) { diff --git a/includes/Content/GadgetDefinitionValidator.php b/includes/Content/GadgetDefinitionValidator.php index 832d5e45..f158fd39 100644 --- a/includes/Content/GadgetDefinitionValidator.php +++ b/includes/Content/GadgetDefinitionValidator.php @@ -29,9 +29,7 @@ class GadgetDefinitionValidator { 'settings.supportsUrlLoad' => [ 'is_bool', 'boolean' ], 'settings.requiresES6' => [ 'is_bool', 'boolean' ], 'module' => [ 'is_array', 'array' ], - 'module.scripts' => [ 'is_array', 'array', 'is_string', 'string' ], - 'module.styles' => [ 'is_array', 'array', 'is_string', 'string' ], - 'module.datas' => [ 'is_array', 'array', 'is_string', 'string' ], + 'module.pages' => [ 'is_array', 'array', 'is_string', 'string' ], 'module.dependencies' => [ 'is_array', 'array', 'is_string', 'string' ], 'module.peers' => [ 'is_array', 'array', 'is_string', 'string' ], 'module.messages' => [ 'is_array', 'array', 'is_string', 'string' ], diff --git a/includes/Content/schema.json b/includes/Content/schema.json index 24d75d0f..252974ec 100644 --- a/includes/Content/schema.json +++ b/includes/Content/schema.json @@ -87,17 +87,9 @@ "type": "object", "additionalProperties": false, "properties": { - "scripts": { + "pages": { "type": "array", - "description": "List of JavaScript pages included in this gadget" - }, - "styles": { - "type": "array", - "description": "List of CSS pages included in this gadget" - }, - "datas": { - "type": "array", - "description": "List of JSON pages included in this gadget" + "description": "List of JS/CSS/JSON pages included in this gadget" }, "dependencies": { "type": "array", diff --git a/includes/Gadget.php b/includes/Gadget.php index 9589eb1a..5aae2bd9 100644 --- a/includes/Gadget.php +++ b/includes/Gadget.php @@ -36,19 +36,15 @@ class Gadget { /** * Increment this when changing class structure */ - public const GADGET_CLASS_VERSION = 15; + public const GADGET_CLASS_VERSION = 16; public const CACHE_TTL = 86400; - /** @var string[] */ - private $scripts = []; - /** @var string[] */ - private $styles = []; - /** @var string[] */ - private $datas = []; /** @var string[] */ private $dependencies = []; /** @var string[] */ + private array $pages = []; + /** @var string[] */ private $peers = []; /** @var string[] */ private $messages = []; @@ -88,10 +84,8 @@ class Gadget { public function __construct( array $options ) { foreach ( $options as $member => $option ) { switch ( $member ) { - case 'scripts': - case 'styles': - case 'datas': case 'dependencies': + case 'pages': case 'peers': case 'messages': case 'name': @@ -132,13 +126,13 @@ class Gadget { }; return [ 'category' => $data['settings']['category'], - 'datas' => array_map( $prefixGadgetNs, $data['module']['datas'] ), 'dependencies' => $data['module']['dependencies'], 'hidden' => $data['settings']['hidden'], 'messages' => $data['module']['messages'], 'name' => $id, 'onByDefault' => $data['settings']['default'], 'package' => $data['settings']['package'], + 'pages' => array_map( $prefixGadgetNs, $data['module']['pages'] ), 'peers' => $data['module']['peers'], 'requiredActions' => $data['settings']['actions'], 'requiredContentModels' => $data['settings']['contentModels'], @@ -147,8 +141,6 @@ class Gadget { 'requiredSkins' => $data['settings']['skins'], 'requiresES6' => $data['settings']['requiresES6'], 'resourceLoaded' => true, - 'scripts' => array_map( $prefixGadgetNs, $data['module']['scripts'] ), - 'styles' => array_map( $prefixGadgetNs, $data['module']['styles'] ), 'supportsUrlLoad' => $data['settings']['supportsUrlLoad'], 'targets' => $data['settings']['targets'], 'type' => $data['module']['type'], @@ -162,13 +154,13 @@ class Gadget { public function toArray(): array { return [ 'category' => $this->category, - 'datas' => $this->datas, 'dependencies' => $this->dependencies, 'hidden' => $this->hidden, 'messages' => $this->messages, 'name' => $this->name, 'onByDefault' => $this->onByDefault, 'package' => $this->package, + 'pages' => $this->pages, 'peers' => $this->peers, 'requiredActions' => $this->requiredActions, 'requiredContentModels' => $this->requiredContentModels, @@ -177,8 +169,6 @@ class Gadget { 'requiredSkins' => $this->requiredSkins, 'requiresES6' => $this->requiresES6, 'resourceLoaded' => $this->resourceLoaded, - 'scripts' => $this->scripts, - 'styles' => $this->styles, 'supportsUrlLoad' => $this->supportsUrlLoad, 'targets' => $this->targets, 'type' => $this->type, @@ -291,7 +281,7 @@ class Gadget { */ public function isPackaged(): bool { // A packaged gadget needs to have a main script, so there must be at least one script - return $this->package && $this->supportsResourceLoader() && $this->scripts !== []; + return $this->package && $this->supportsResourceLoader() && $this->getScripts() !== []; } /** @@ -375,7 +365,7 @@ class Gadget { * @return bool Whether this gadget has resources that can be loaded via ResourceLoader */ public function hasModule() { - return $this->styles || ( $this->supportsResourceLoader() && $this->scripts ); + return $this->getStyles() || ( $this->supportsResourceLoader() && $this->getScripts() ); } /** @@ -389,28 +379,34 @@ class Gadget { * @return array Array of pages with JS (including namespace) */ public function getScripts() { - return $this->scripts; + return array_values( array_filter( $this->pages, static function ( $page ) { + return str_ends_with( $page, '.js' ); + } ) ); } /** * @return array Array of pages with CSS (including namespace) */ public function getStyles() { - return $this->styles; + return array_values( array_filter( $this->pages, static function ( $page ) { + return str_ends_with( $page, '.css' ); + } ) ); } /** * @return array Array of pages with JSON (including namespace) */ public function getJSONs(): array { - return $this->isPackaged() ? $this->datas : []; + return array_values( array_filter( $this->pages, static function ( $page ) { + return str_ends_with( $page, '.json' ); + } ) ); } /** * @return array Array of all of this gadget's resources */ public function getScriptsAndStyles() { - return array_merge( $this->scripts, $this->styles, $this->getJSONs() ); + return array_merge( $this->getScripts(), $this->getStyles(), $this->getJSONs() ); } /** @@ -418,7 +414,7 @@ class Gadget { * @return string[] */ public function getLegacyScripts() { - return $this->supportsResourceLoader() ? [] : $this->scripts; + return $this->supportsResourceLoader() ? [] : $this->getScripts(); } /** @@ -498,7 +494,7 @@ class Gadget { return $this->type; } // Similar to ResourceLoaderWikiModule default - if ( $this->styles && !$this->scripts && !$this->dependencies ) { + if ( $this->getStyles() && !$this->getScripts() && !$this->dependencies ) { return 'styles'; } @@ -517,18 +513,23 @@ class Gadget { $warnings[] = "gadgets-validate-es6default"; } + // Gadget containing files with uncrecognised suffixes + if ( count( array_diff( $this->pages, $this->getScriptsAndStyles() ) ) !== 0 ) { + $warnings[] = "gadgets-validate-unknownpages"; + } + // Non-package gadget containing JSON files - if ( !$this->package && count( $this->datas ) > 0 ) { + if ( !$this->package && count( $this->getJSONs() ) > 0 ) { $warnings[] = "gadgets-validate-json"; } // Package gadget without a script file in it (to serve as entry point) - if ( $this->package && count( $this->scripts ) === 0 ) { + if ( $this->package && count( $this->getScripts() ) === 0 ) { $warnings[] = "gadgets-validate-noentrypoint"; } // Gadget with type=styles having non-CSS files - if ( $this->type === 'styles' && count( $this->scripts ) > 0 ) { + if ( $this->type === 'styles' && count( $this->getScripts() ) > 0 ) { $warnings[] = "gadgets-validate-scriptsnotallowed"; } diff --git a/includes/GadgetResourceLoaderModule.php b/includes/GadgetResourceLoaderModule.php index c836f564..79c53402 100644 --- a/includes/GadgetResourceLoaderModule.php +++ b/includes/GadgetResourceLoaderModule.php @@ -61,8 +61,10 @@ class GadgetResourceLoaderModule extends RL\WikiModule { foreach ( $gadget->getScripts() as $script ) { $pages[$script] = [ 'type' => 'script' ]; } - foreach ( $gadget->getJSONs() as $json ) { - $pages[$json] = [ 'type' => 'data' ]; + if ( $gadget->isPackaged() ) { + foreach ( $gadget->getJSONs() as $json ) { + $pages[$json] = [ 'type' => 'data' ]; + } } } diff --git a/includes/MediaWikiGadgetsDefinitionRepo.php b/includes/MediaWikiGadgetsDefinitionRepo.php index a355db8a..33288a1e 100644 --- a/includes/MediaWikiGadgetsDefinitionRepo.php +++ b/includes/MediaWikiGadgetsDefinitionRepo.php @@ -304,14 +304,7 @@ class MediaWikiGadgetsDefinitionRepo extends GadgetRepo { foreach ( preg_split( '/\s*\|\s*/', $pages, -1, PREG_SPLIT_NO_EMPTY ) as $page ) { $page = $this->titlePrefix . $page; - - if ( preg_match( '/\.json$/', $page ) ) { - $info['datas'][] = $page; - } elseif ( preg_match( '/\.js/', $page ) ) { - $info['scripts'][] = $page; - } elseif ( preg_match( '/\.css/', $page ) ) { - $info['styles'][] = $page; - } + $info['pages'][] = $page; } return new Gadget( $info ); diff --git a/tests/phpunit/integration/GadgetDefinitionNamespaceRepoTest.php b/tests/phpunit/integration/GadgetDefinitionNamespaceRepoTest.php index 45ab9184..3e253934 100644 --- a/tests/phpunit/integration/GadgetDefinitionNamespaceRepoTest.php +++ b/tests/phpunit/integration/GadgetDefinitionNamespaceRepoTest.php @@ -13,7 +13,7 @@ class GadgetDefinitionNamespaceRepoTest extends MediaWikiIntegrationTestCase { */ public function testGetGadget() { $this->editPage( 'Gadget definition:Test', - '{"module":{"scripts":["test.js"]}, "settings":{"default":true}}' ); + '{"module":{"pages":["test.js"]}, "settings":{"default":true}}' ); $services = $this->getServiceContainer(); $repo = new GadgetDefinitionNamespaceRepo( $services->getMainWANObjectCache(), $services->getRevisionLookup() ); @@ -27,9 +27,9 @@ class GadgetDefinitionNamespaceRepoTest extends MediaWikiIntegrationTestCase { */ public function testGetGadgetIds() { $this->editPage( 'Gadget definition:X1', - '{"module":{"scripts":["Gadget:test.js"]}, "settings":{"default":true}}' ); + '{"module":{"pages":["Gadget:test.js"]}, "settings":{"default":true}}' ); $this->editPage( 'Gadget definition:X2', - '{"module":{"scripts":["Gadget:test.js"]}, "settings":{"default":true}}' ); + '{"module":{"pages":["Gadget:test.js"]}, "settings":{"default":true}}' ); $services = $this->getServiceContainer(); $wanCache = $services->getMainWANObjectCache(); diff --git a/tests/phpunit/integration/GadgetHooksTest.php b/tests/phpunit/integration/GadgetHooksTest.php index 4b36ece4..784c9b9f 100644 --- a/tests/phpunit/integration/GadgetHooksTest.php +++ b/tests/phpunit/integration/GadgetHooksTest.php @@ -17,7 +17,7 @@ class GadgetHooksTest extends MediaWikiIntegrationTestCase { public function testDefaultGadget() { $services = $this->getServiceContainer(); $repo = new StaticGadgetRepo( [ - 'g1' => new Gadget( [ 'name' => 'g1', 'onByDefault' => true, 'styles' => 'test.css' ] ), + 'g1' => new Gadget( [ 'name' => 'g1', 'onByDefault' => true, 'pages' => [ 'test.css' ] ] ), ] ); $hooks = new GadgetHooks( $repo, $services->getUserOptionsLookup(), null ); $out = new OutputPage( RequestContext::getMain() ); @@ -34,7 +34,7 @@ class GadgetHooksTest extends MediaWikiIntegrationTestCase { public function testEnabledGadget() { $services = $this->getServiceContainer(); $repo = new StaticGadgetRepo( [ - 'g1' => new Gadget( [ 'name' => 'g1', 'scripts' => 'test.js', 'resourceLoaded' => true ] ), + 'g1' => new Gadget( [ 'name' => 'g1', 'pages' => [ 'test.js' ], 'resourceLoaded' => true ] ), ] ); $hooks = new GadgetHooks( $repo, $services->getUserOptionsLookup(), null ); $context = RequestContext::getMain(); @@ -58,9 +58,9 @@ class GadgetHooksTest extends MediaWikiIntegrationTestCase { */ public function testDefaultUserOptions() { $repo = new StaticGadgetRepo( [ - 'g1' => new Gadget( [ 'name' => 'g1', 'styles' => 'test.css', 'onByDefault' => true ] ), - 'g2' => new Gadget( [ 'name' => 'g2', 'styles' => 'test.css' ] ), - 'g3' => new Gadget( [ 'name' => 'g3', 'styles' => 'test.css', 'hidden' => true ] ), + 'g1' => new Gadget( [ 'name' => 'g1', 'pages' => [ 'test.css' ], 'onByDefault' => true ] ), + 'g2' => new Gadget( [ 'name' => 'g2', 'pages' => [ 'test.css' ] ] ), + 'g3' => new Gadget( [ 'name' => 'g3', 'pages' => [ 'test.css' ], 'hidden' => true ] ), ] ); $this->setService( 'GadgetsRepo', $repo ); $optionsLookup = $this->getServiceContainer()->getUserOptionsLookup(); @@ -76,12 +76,12 @@ class GadgetHooksTest extends MediaWikiIntegrationTestCase { public function testPreferences() { $services = $this->getServiceContainer(); $repo = new StaticGadgetRepo( [ - 'foo' => new Gadget( [ 'name' => 'foo', 'styles' => 'foo.css' ] ), - 'bar' => new Gadget( [ 'name' => 'bar', 'styles' => 'bar.css', + 'foo' => new Gadget( [ 'name' => 'foo', 'pages' => [ 'foo.css' ] ] ), + 'bar' => new Gadget( [ 'name' => 'bar', 'pages' => [ 'bar.css' ], 'category' => 'keep-section1' ] ), - 'baz' => new Gadget( [ 'name' => 'baz', 'styles' => 'baz.css', 'requiredRights' => [ 'delete' ], + 'baz' => new Gadget( [ 'name' => 'baz', 'pages' => [ 'baz.css' ], 'requiredRights' => [ 'delete' ], 'category' => 'remove-section' ] ), - 'quux' => new Gadget( [ 'name' => 'quux', 'styles' => 'quux.css', 'requiredRights' => [ 'read' ], + 'quux' => new Gadget( [ 'name' => 'quux', 'pages' => [ 'quux.css' ], 'requiredRights' => [ 'read' ], 'category' => 'keep-section2' ] ), ] ); $hooks = new GadgetHooks( $repo, $services->getUserOptionsLookup(), null ); diff --git a/tests/phpunit/unit/GadgetTest.php b/tests/phpunit/unit/GadgetTest.php index 47266e83..ec278d4f 100644 --- a/tests/phpunit/unit/GadgetTest.php +++ b/tests/phpunit/unit/GadgetTest.php @@ -38,8 +38,7 @@ class GadgetTest extends MediaWikiUnitTestCase { 'requiresES6' => false, ], 'module' => [ - 'scripts' => [ 'foo.js' ], - 'styles' => [ 'bar.css' ], + 'pages' => [ 'foo.js', 'bar.css' ], 'datas' => [], 'dependencies' => [ 'moment' ], 'peers' => [], @@ -81,12 +80,14 @@ class GadgetTest extends MediaWikiUnitTestCase { * @covers \MediaWiki\Extension\Gadgets\Gadget */ public function testSimpleCases() { - $g = $this->makeGadget( '* foo bar| foo.css|foo.js|foo.bar' ); + $g = $this->makeGadget( '* foo bar| foo.css|foo.js|foo.json|foo.bar' ); $this->assertEquals( 'foo_bar', $g->getName() ); $this->assertEquals( 'ext.gadget.foo_bar', Gadget::getModuleName( $g->getName() ) ); $this->assertEquals( [ 'MediaWiki:Gadget-foo.js' ], $g->getScripts() ); $this->assertEquals( [ 'MediaWiki:Gadget-foo.css' ], $g->getStyles() ); - $this->assertEquals( [ 'MediaWiki:Gadget-foo.js', 'MediaWiki:Gadget-foo.css' ], $g->getScriptsAndStyles() ); + $this->assertEquals( [ 'MediaWiki:Gadget-foo.json' ], $g->getJSONs() ); + $this->assertEquals( [ 'MediaWiki:Gadget-foo.js', 'MediaWiki:Gadget-foo.css', 'MediaWiki:Gadget-foo.json' ], + $g->getScriptsAndStyles() ); $this->assertEquals( [ 'MediaWiki:Gadget-foo.js' ], $g->getLegacyScripts() ); $this->assertFalse( $g->supportsResourceLoader() ); $this->assertTrue( $g->hasModule() );