From 4675834a28c3a40317bd81c7f5155b4ebf9675a3 Mon Sep 17 00:00:00 2001 From: Morne Alberts Date: Thu, 21 May 2026 13:12:35 +0200 Subject: [PATCH 1/2] Restore sparse per-component module loading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The intended-but-broken sparse-loading scheme had two gaps: 1) Only ImageModal::177 called BootstrapComponentsService::registerComponentAsActive(), so HooksHandler::onParserAfterParse's foreach over getActiveComponents() never iterated tooltip / popover / carousel / modal / etc. — their per-component .fix modules were never queued, JS or CSS. 2) The foreach itself only called $parserOutput->addModuleStyles(), so even for ImageModal the JS half of any module with a 'scripts' entry was unreachable through that path. Close both gaps: - AbstractComponent::parseComponent() now calls registerComponentAsActive() right after opening the nesting controller. All 11 explicit-syntax components (Accordion, Alert, Badge, Button, Card, Carousel, Collapse, Jumbotron, Modal, Popover, Tooltip) inherit this via the single chokepoint. Fetched via MediaWikiServices, matching the service-locator pattern already used by LuaLibrary, CarouselGallery, and ImageModal in the codebase. - HooksHandler::onParserAfterParse foreach now calls both addModuleStyles() and addModules() per module. Style-only modules (modal.fix etc.) treat the addModules call as a no-op; modules with a 'scripts' entry (tooltip.fix, popover.fix, carousel.fix) get their init JS loaded. Net effect: a page with only a tooltip loads only tooltip.fix. A page with nothing loads no BC init modules at all. Sparse loading is restored as originally intended. Verified on a fresh MW 1.39.17 + Vector + BC stack and on a MW 1.43.8 + Chameleon BS4 + BC stack: tooltip and popover both fire; carousel.fix stays mw.loader state 'registered' on pages without a carousel; modal still opens (PR #75's inline-emission fix is orthogonal and unaffected). PHPUnit unit suite green relative to baseline (6 pre-existing failures unchanged). Co-Authored-By: Claude Opus 4.7 --- src/AbstractComponent.php | 4 ++++ src/HooksHandler.php | 1 + 2 files changed, 5 insertions(+) diff --git a/src/AbstractComponent.php b/src/AbstractComponent.php index e3ae555..8e9dd2e 100644 --- a/src/AbstractComponent.php +++ b/src/AbstractComponent.php @@ -26,6 +26,7 @@ namespace MediaWiki\Extension\BootstrapComponents; +use MediaWiki\MediaWikiServices; use \MWException; /** @@ -153,6 +154,9 @@ public function parseComponent( $parserRequest ) { throw new MWException( 'Invalid ParserRequest supplied to component ' . $this->getComponentName() . '!' ); } $this->getNestingController()->open( $this ); + MediaWikiServices::getInstance() + ->getService( 'BootstrapComponentsService' ) + ->registerComponentAsActive( $this->getComponentName() ); $this->initComponentData( $parserRequest ); $input = $this->prepareInput( $parserRequest ); diff --git a/src/HooksHandler.php b/src/HooksHandler.php index fa01e95..7ff5bac 100644 --- a/src/HooksHandler.php +++ b/src/HooksHandler.php @@ -228,6 +228,7 @@ public function onParserAfterParse( $parser, &$text, $stripState ): bool { } foreach ( $this->getComponentLibrary()->getModulesFor( $activeComponent ) as $module ) { $parser->getOutput()->addModuleStyles( [ $module ] ); + $parser->getOutput()->addModules( [ $module ] ); } } return true; From e7f2f3fadfc805e306e03869b414a1004d0f8c83 Mon Sep 17 00:00:00 2001 From: Morne Alberts Date: Thu, 21 May 2026 13:44:00 +0200 Subject: [PATCH 2/2] Add HooksHandlerTest case pinning the foreach addModules contract Asserts that HooksHandler::onParserAfterParse calls both addModuleStyles and addModules on the ParserOutput for each active component's modules. Previously only addModuleStyles was called, so components with a scripts entry (tooltip.fix, popover.fix, carousel.fix) had their init JS dropped. Co-Authored-By: Claude Opus 4.7 --- tests/phpunit/Unit/HooksHandlerTest.php | 48 +++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/phpunit/Unit/HooksHandlerTest.php b/tests/phpunit/Unit/HooksHandlerTest.php index 827d40d..3d9867e 100644 --- a/tests/phpunit/Unit/HooksHandlerTest.php +++ b/tests/phpunit/Unit/HooksHandlerTest.php @@ -6,6 +6,8 @@ use MediaWiki\Extension\BootstrapComponents\ComponentLibrary; use MediaWiki\Extension\BootstrapComponents\HooksHandler; use MediaWiki\Extension\BootstrapComponents\NestingController; +use Parser; +use ParserOutput; use PHPUnit\Framework\TestCase; /** @@ -72,4 +74,50 @@ public function testOnGalleryGetModes() { public function testOnOutputPageParserOutput() { $this->assertTrue( true ); } + + public function testOnParserAfterParseLoadsActiveComponentScriptsAndStyles() { + $loadedStyles = []; + $loadedModules = []; + + $parserOutput = $this->createMock( ParserOutput::class ); + $parserOutput->method( 'addModuleStyles' ) + ->willReturnCallback( function ( $m ) use ( &$loadedStyles ) { + $loadedStyles = array_merge( $loadedStyles, (array)$m ); + } ); + $parserOutput->method( 'addModules' ) + ->willReturnCallback( function ( $m ) use ( &$loadedModules ) { + $loadedModules = array_merge( $loadedModules, (array)$m ); + } ); + + $parser = $this->createMock( Parser::class ); + $parser->method( 'getOutput' )->willReturn( $parserOutput ); + + $service = $this->createMock( BootstrapComponentsService::class ); + $service->method( 'getNameOfActiveSkin' )->willReturn( 'vector' ); + $service->method( 'getActiveComponents' )->willReturn( [ 'tooltip', 'popover' ] ); + + $library = $this->createMock( ComponentLibrary::class ); + $library->method( 'isRegistered' )->willReturn( true ); + $library->method( 'getModulesFor' ) + ->willReturnCallback( function ( $name ) { + return [ 'ext.bootstrapComponents.' . $name . '.fix' ]; + } ); + + $handler = new HooksHandler( + $service, + $library, + $this->createMock( NestingController::class ) + ); + + $text = ''; + $handler->onParserAfterParse( $parser, $text, null ); + + // Per-component fix modules must reach BOTH addModuleStyles and addModules + // (style-only modules treat addModules as a no-op; modules with a scripts + // entry get their JS init via that call). + $this->assertContains( 'ext.bootstrapComponents.tooltip.fix', $loadedStyles ); + $this->assertContains( 'ext.bootstrapComponents.popover.fix', $loadedStyles ); + $this->assertContains( 'ext.bootstrapComponents.tooltip.fix', $loadedModules ); + $this->assertContains( 'ext.bootstrapComponents.popover.fix', $loadedModules ); + } }