From 66604c472f2361796c7f2df64f34826ef38afcdf Mon Sep 17 00:00:00 2001 From: Morne Alberts Date: Tue, 21 Jul 2026 16:35:27 +0200 Subject: [PATCH 1/4] Load Bootstrap once per skin Skins that bundle their own Bootstrap (for example Medik and Tweeki) also received Extension:Bootstrap's copy, because BootstrapComponents loaded ext.bootstrap.styles and ext.bootstrap.scripts unconditionally. Two copies of Bootstrap each registered Bootstrap's document data-api, so one click was handled twice and the skin's navbar and action dropdowns opened and immediately closed. From a BeforePageDisplay hook, where the active skin is known, BootstrapComponents now names the modules carrying this skin's Bootstrap in a config variable: the script module to wait on (ext.bootstrap.scripts on skins without their own, and only there is Extension:Bootstrap's copy loaded, the skin's own module on Medik, or Tweeki's configured script module, mirroring SkinTweeki::initPage) and the stylesheet module to load for dynamically rendered content, null on skins that provide their own stylesheet. The shared resolver module waits on the script module with mw.loader.using, loads the stylesheet on demand, and runs the component init scripts on wikipage.content, so live preview and VisualEditor re-renders are covered too. On Tweeki, which exposes no window.bootstrap, the components initialize through Bootstrap's jQuery plugin bridge. Whether a skin provides Bootstrap scripts is derived from the same per-skin map that names the wait module, so the two decisions cannot drift apart. The hook runs only where content was parsed, marked by the styles fix module the parser adds on every parsed page; cache entries have carried it since 5.2.0, so warm parser caches keep working across the upgrade. The load moved off the ParserOutput (it has to, so the decision can depend on the skin, which the shared parser cache cannot), so api.php?action=parse no longer lists ext.bootstrap.styles / ext.bootstrap.scripts. Co-Authored-By: Claude Opus 4.8 Co-Authored-By: Claude Fable 5 --- docs/release-notes.md | 11 +++ extension.json | 4 +- modules/ext.bootstrapComponents.bootstrap.js | 25 +++++- modules/ext.bootstrapComponents.carousel.js | 16 +--- modules/ext.bootstrapComponents.modal.js | 16 +--- modules/ext.bootstrapComponents.popover.js | 20 ++--- modules/ext.bootstrapComponents.tooltip.js | 18 ++-- src/BootstrapComponentsService.php | 51 ++++++++++++ src/HooksHandler.php | 51 ++++++++++-- .../Unit/BootstrapComponentsServiceTest.php | 82 +++++++++++++++++++ tests/phpunit/Unit/HooksHandlerTest.php | 77 +++++++++++++++-- 11 files changed, 304 insertions(+), 67 deletions(-) diff --git a/docs/release-notes.md b/docs/release-notes.md index 096f072..68fbcb2 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -1,5 +1,16 @@ ## Release Notes +### BootstrapComponents 6.1.0 + +Released on TBD + +Fixes: +* fix dropdown menus, accordions and other click-driven components on Medik and Tweeki +* skins that provide their own Bootstrap (Medik and Tweeki) no longer also load Extension:Bootstrap's copy + +Changes: +* `api.php?action=parse` no longer lists the `ext.bootstrap.*` modules; anything rendering parsed content outside a normal page view must load Bootstrap itself + ### BootstrapComponents 6.0.1 Released on 20-July-2026 diff --git a/extension.json b/extension.json index 74851e2..64aae5f 100644 --- a/extension.json +++ b/extension.json @@ -33,6 +33,9 @@ } }, "Hooks": { + "BeforePageDisplay": { + "handler": "BootStrapHooks" + }, "GalleryGetModes": { "handler": "BootStrapHooks" }, @@ -92,7 +95,6 @@ "styles": "ext.bootstrapComponents.bootstrap.fix.css" }, "ext.bootstrapComponents.bootstrap": { - "dependencies": "ext.bootstrap.scripts", "packageFiles": [ "ext.bootstrapComponents.bootstrap.js" ] }, "ext.bootstrapComponents.accordion.fix": { diff --git a/modules/ext.bootstrapComponents.bootstrap.js b/modules/ext.bootstrapComponents.bootstrap.js index be8f9d2..a990bf2 100644 --- a/modules/ext.bootstrapComponents.bootstrap.js +++ b/modules/ext.bootstrapComponents.bootstrap.js @@ -12,6 +12,29 @@ function getComponentClass( name ) { return ( plugin && plugin.Constructor ) || null; } +// Defers component initialization until the module carrying the active skin's Bootstrap has +// loaded. A static ResourceLoader dependency cannot express this, since the module depends on +// the skin. The modules are provided in a config variable instead. The callback runs for the +// initial page content and again for every re-render (VisualEditor, live preview). +function whenReady( callback ) { + const bootstrapModules = mw.config.get( 'wgBootstrapComponentsBootstrapModules' ); + if ( !bootstrapModules ) { + mw.log.warn( 'BootstrapComponents: no Bootstrap modules were configured for this skin.' ); + return; + } + if ( bootstrapModules.styles ) { + mw.loader.load( bootstrapModules.styles ); + } + mw.loader.using( bootstrapModules.scripts ).then( function () { + mw.hook( 'wikipage.content' ).add( function ( $content ) { + callback( ( $content && $content[ 0 ] ) || document ); + } ); + }, function () { + mw.log.warn( 'BootstrapComponents: Bootstrap module "' + bootstrapModules.scripts + '" failed to load.' ); + } ); +} + module.exports = { - getComponentClass: getComponentClass + getComponentClass: getComponentClass, + whenReady: whenReady }; diff --git a/modules/ext.bootstrapComponents.carousel.js b/modules/ext.bootstrapComponents.carousel.js index 32dbfa0..264ea0c 100644 --- a/modules/ext.bootstrapComponents.carousel.js +++ b/modules/ext.bootstrapComponents.carousel.js @@ -26,25 +26,17 @@ ( function () { 'use strict'; - const { getComponentClass } = require( 'ext.bootstrapComponents.bootstrap' ); + const { getComponentClass, whenReady } = require( 'ext.bootstrapComponents.bootstrap' ); - // Wait for DOM to be ready - if ( document.readyState === 'loading' ) { - document.addEventListener( 'DOMContentLoaded', initCarousels ); - } else { - initCarousels(); - } - - function initCarousels() { + whenReady( function ( content ) { const Carousel = getComponentClass( 'Carousel' ); if ( !Carousel ) { // eslint-disable-next-line no-console console.warn( 'BootstrapComponents: bootstrap.Carousel is not available; carousels will not cycle.' ); return; } - const carouselElements = document.querySelectorAll( '.carousel' ); - carouselElements.forEach( function ( element ) { + content.querySelectorAll( '.carousel' ).forEach( function ( element ) { new Carousel( element ); } ); - } + } ); }() ); diff --git a/modules/ext.bootstrapComponents.modal.js b/modules/ext.bootstrapComponents.modal.js index 0347fe0..096b3f6 100644 --- a/modules/ext.bootstrapComponents.modal.js +++ b/modules/ext.bootstrapComponents.modal.js @@ -5,16 +5,9 @@ ( function () { 'use strict'; - const { getComponentClass } = require( 'ext.bootstrapComponents.bootstrap' ); + const { getComponentClass, whenReady } = require( 'ext.bootstrapComponents.bootstrap' ); - // Wait for DOM to be ready - if ( document.readyState === 'loading' ) { - document.addEventListener( 'DOMContentLoaded', initModals ); - } else { - initModals(); - } - - function initModals() { + whenReady( function ( content ) { const Modal = getComponentClass( 'Modal' ); if ( !Modal ) { // eslint-disable-next-line no-console @@ -23,9 +16,8 @@ } // Instantiate every .modal element so trigger clicks (or programmatic // bootstrap.Modal.getOrCreateInstance(el).show()) work as expected. - const modalList = document.querySelectorAll( '.modal' ); - modalList.forEach( function ( modalEl ) { + content.querySelectorAll( '.modal' ).forEach( function ( modalEl ) { Modal.getOrCreateInstance( modalEl ); } ); - } + } ); }() ); diff --git a/modules/ext.bootstrapComponents.popover.js b/modules/ext.bootstrapComponents.popover.js index f5308ce..5002d9a 100644 --- a/modules/ext.bootstrapComponents.popover.js +++ b/modules/ext.bootstrapComponents.popover.js @@ -26,27 +26,17 @@ ( function () { 'use strict'; - const { getComponentClass } = require( 'ext.bootstrapComponents.bootstrap' ); + const { getComponentClass, whenReady } = require( 'ext.bootstrapComponents.bootstrap' ); - // Wait for DOM to be ready - if ( document.readyState === 'loading' ) { - document.addEventListener( 'DOMContentLoaded', initPopovers ); - } else { - initPopovers(); - } - - function initPopovers() { + whenReady( function ( content ) { const Popover = getComponentClass( 'Popover' ); if ( !Popover ) { // eslint-disable-next-line no-console console.warn( 'BootstrapComponents: bootstrap.Popover is not available; popover triggers will not work.' ); return; } - const popoverTriggerList = document.querySelectorAll( '[data-bs-toggle="popover"]' ); - popoverTriggerList.forEach( function ( popoverTriggerEl ) { - new Popover( popoverTriggerEl, { - html: true - } ); + content.querySelectorAll( '[data-bs-toggle="popover"]' ).forEach( function ( el ) { + new Popover( el, { html: true } ); } ); - } + } ); }() ); diff --git a/modules/ext.bootstrapComponents.tooltip.js b/modules/ext.bootstrapComponents.tooltip.js index b67683d..d2dcd72 100644 --- a/modules/ext.bootstrapComponents.tooltip.js +++ b/modules/ext.bootstrapComponents.tooltip.js @@ -26,25 +26,17 @@ ( function () { 'use strict'; - const { getComponentClass } = require( 'ext.bootstrapComponents.bootstrap' ); + const { getComponentClass, whenReady } = require( 'ext.bootstrapComponents.bootstrap' ); - // Wait for DOM to be ready - if ( document.readyState === 'loading' ) { - document.addEventListener( 'DOMContentLoaded', initTooltips ); - } else { - initTooltips(); - } - - function initTooltips() { + whenReady( function ( content ) { const Tooltip = getComponentClass( 'Tooltip' ); if ( !Tooltip ) { // eslint-disable-next-line no-console console.warn( 'BootstrapComponents: bootstrap.Tooltip is not available; tooltip triggers will not work.' ); return; } - const tooltipTriggerList = document.querySelectorAll( '[data-bs-toggle="tooltip"]' ); - tooltipTriggerList.forEach( function ( tooltipTriggerEl ) { - new Tooltip( tooltipTriggerEl ); + content.querySelectorAll( '[data-bs-toggle="tooltip"]' ).forEach( function ( el ) { + new Tooltip( el ); } ); - } + } ); }() ); diff --git a/src/BootstrapComponentsService.php b/src/BootstrapComponentsService.php index 2af0701..8d7ebef 100644 --- a/src/BootstrapComponentsService.php +++ b/src/BootstrapComponentsService.php @@ -76,6 +76,57 @@ public function vectorSkinInUse(): bool { return in_array( strtolower( $this->getNameOfActiveSkin() ), [ 'vector', 'vector-2022' ] ) ; } + /** + * Whether the given skin ships its own Bootstrap JavaScript. + */ + public function skinProvidesBootstrapScripts( string $skin ): bool { + return $this->getBootstrapScriptsModule( $skin ) !== 'ext.bootstrap.scripts'; + } + + /** + * The ResourceLoader script module carrying the Bootstrap this skin should use: the skin's own on + * skins that ship Bootstrap, Extension:Bootstrap's `ext.bootstrap.scripts` otherwise. The + * component initialization waits on this module. + */ + public function getBootstrapScriptsModule( string $skin ): string { + return match ( strtolower( $skin ) ) { + 'medik' => 'skins.medik.js', + 'tweeki' => $this->getTweekiScriptModule(), + default => 'ext.bootstrap.scripts', + }; + } + + /** + * Mirrors Tweeki's own script-module selection ({@see \SkinTweeki::initPage}), so we depend on + * whichever module the running wiki actually loads Bootstrap through. + */ + private function getTweekiScriptModule(): string { + if ( $this->mainConfig->has( 'TweekiSkinCustomScriptModule' ) + && $this->mainConfig->get( 'TweekiSkinCustomScriptModule' ) + ) { + return $this->mainConfig->get( 'TweekiSkinCustomScriptModule' ); + } + if ( $this->mainConfig->has( 'TweekiSkinUseCustomFiles' ) + && $this->mainConfig->get( 'TweekiSkinUseCustomFiles' ) + ) { + return 'skins.tweeki.custom.scripts'; + } + return 'skins.tweeki.scripts'; + } + + public function getBootstrapStylesModule( string $skin ): ?string { + return $this->skinProvidesBootstrapStyles( $skin ) ? null : 'ext.bootstrap.styles'; + } + + /** + * Skins that put the Bootstrap stylesheet on the page themselves. + */ + private const SKINS_WITH_OWN_BOOTSTRAP_STYLES = [ 'medik', 'tweeki' ]; + + public function skinProvidesBootstrapStyles( string $skin ): bool { + return in_array( strtolower( $skin ), self::SKINS_WITH_OWN_BOOTSTRAP_STYLES, true ); + } + /** * @param bool $useConfig set this to true, if we can't rely on {@see \RequestContext::getSkin} * diff --git a/src/HooksHandler.php b/src/HooksHandler.php index 2f2d49a..c942c4d 100644 --- a/src/HooksHandler.php +++ b/src/HooksHandler.php @@ -6,6 +6,7 @@ use MediaWiki\Config\Config; use MediaWiki\Extension\BootstrapComponents\Hooks\OutputPageParserOutput; use MediaWiki\Extension\BootstrapComponents\Hooks\ParserFirstCallInit; +use MediaWiki\Hook\BeforePageDisplayHook; use MediaWiki\Hook\GalleryGetModesHook; use MediaWiki\Hook\ImageBeforeProduceHTMLHook; use MediaWiki\Hook\InternalParseBeforeLinksHook; @@ -37,6 +38,7 @@ * @since 5.0 */ class HooksHandler implements + BeforePageDisplayHook, GalleryGetModesHook, ImageBeforeProduceHTMLHook, InternalParseBeforeLinksHook, @@ -45,6 +47,12 @@ class HooksHandler implements ParserFirstCallInitHook, SetupAfterCacheHook { + /** + * The styles fix module added on every parsed page. onBeforePageDisplay reads it as the + * marker that the page rendered parsed content. + */ + private const BOOTSTRAP_FIX_MODULE = 'ext.bootstrapComponents.bootstrap.fix'; + private Config $config; public function __construct( @@ -69,6 +77,40 @@ public static function onScribuntoExternalLibraries( $engine, array &$extraLibra return true; } + /** + * Hook: BeforePageDisplay + * + * Loads Bootstrap for the active skin: Extension:Bootstrap's stylesheet and script are each + * added only when the skin does not put its own on the page, so a single copy of each is on + * the page, and only where the page rendered parsed content, the same scope in which + * onParserAfterParse adds the styles fix. Component initialization waits on whichever module + * carries this skin's Bootstrap, the modules are named in a config variable set on every page: + * content can also arrive after page display (live preview on an edit page), and the init + * scripts loaded with it read the variable and fetch the named module on demand. + * + * @see https://www.mediawiki.org/wiki/Manual:Hooks/BeforePageDisplay + */ + public function onBeforePageDisplay( $out, $skin ): void { + $skinName = $skin->getSkinName(); + $service = $this->getBootstrapComponentsService(); + + $out->addJsConfigVars( 'wgBootstrapComponentsBootstrapModules', [ + 'scripts' => $service->getBootstrapScriptsModule( $skinName ), + 'styles' => $service->getBootstrapStylesModule( $skinName ), + ] ); + + if ( !in_array( self::BOOTSTRAP_FIX_MODULE, $out->getModuleStyles(), true ) ) { + return; + } + + if ( !$service->skinProvidesBootstrapStyles( $skinName ) ) { + $out->addModuleStyles( [ 'ext.bootstrap.styles' ] ); + } + if ( !$service->skinProvidesBootstrapScripts( $skinName ) ) { + $out->addModules( [ 'ext.bootstrap.scripts' ] ); + } + } + /** * Hook: GalleryGetModes * @@ -194,11 +236,10 @@ public function onOutputPageParserOutput( $outputPage, $parserOutput ): void { * @return bool */ public function onParserAfterParse( $parser, &$text, $stripState ): bool { - // once, this was only loaded, when a component was paced on the page. now, we load it always - // to keep the layout of all the wiki pages consistent. - $parser->getOutput()->addModuleStyles( [ 'ext.bootstrapComponents.bootstrap.fix' ] ); - $parser->getOutput()->addModuleStyles( [ 'ext.bootstrap.styles' ] ); - $parser->getOutput()->addModules( [ 'ext.bootstrap.scripts' ] ); + // Always add the styles fix on parsed pages; it doubles as the marker onBeforePageDisplay + // reads to load Bootstrap for the active skin (which the shared, skin agnostic parser + // cache cannot decide). + $parser->getOutput()->addModuleStyles( [ self::BOOTSTRAP_FIX_MODULE ] ); $skin = $this->getBootstrapComponentsService()->getNameOfActiveSkin(); foreach ( $this->getBootstrapComponentsService()->getActiveComponents() as $activeComponent ) { if ( !$this->getComponentLibrary()->isRegistered( $activeComponent ) ) { diff --git a/tests/phpunit/Unit/BootstrapComponentsServiceTest.php b/tests/phpunit/Unit/BootstrapComponentsServiceTest.php index bba192f..3a793c8 100644 --- a/tests/phpunit/Unit/BootstrapComponentsServiceTest.php +++ b/tests/phpunit/Unit/BootstrapComponentsServiceTest.php @@ -61,6 +61,88 @@ public function testVectorSkinInUse() { $this->assertIsBool( $instance->vectorSkinInUse() ); } + /** + * @dataProvider skinProvidesBootstrapScriptsProvider + */ + public function testSkinProvidesBootstrapScripts( string $skin, bool $expected ) { + $instance = new BootstrapComponentsService( $this->getMockBuilder( Config::class )->getMock() ); + $this->assertSame( $expected, $instance->skinProvidesBootstrapScripts( $skin ) ); + } + + public static function skinProvidesBootstrapScriptsProvider(): array { + return [ + 'medik ships its own Bootstrap' => [ 'medik', true ], + 'tweeki ships its own Bootstrap' => [ 'tweeki', true ], + 'vector does not' => [ 'vector', false ], + 'vector-2022 does not' => [ 'vector-2022', false ], + 'chameleon uses Extension:Bootstrap scripts' => [ 'chameleon', false ], + 'monobook does not' => [ 'monobook', false ], + 'unknown skin does not' => [ 'serenity', false ], + ]; + } + + /** + * @dataProvider skinProvidesBootstrapStylesProvider + */ + public function testSkinProvidesBootstrapStyles( string $skin, bool $expected ) { + $instance = new BootstrapComponentsService( $this->getMockBuilder( Config::class )->getMock() ); + $this->assertSame( $expected, $instance->skinProvidesBootstrapStyles( $skin ) ); + } + + public static function skinProvidesBootstrapStylesProvider(): array { + return [ + 'medik ships its own stylesheet' => [ 'medik', true ], + 'tweeki ships its own stylesheet' => [ 'tweeki', true ], + 'vector does not' => [ 'vector', false ], + 'unknown skin does not' => [ 'serenity', false ], + ]; + } + + /** + * @dataProvider getBootstrapScriptsModuleProvider + */ + public function testGetBootstrapScriptsModule( string $skin, string $expected ) { + $instance = new BootstrapComponentsService( $this->getMockBuilder( Config::class )->getMock() ); + $this->assertSame( $expected, $instance->getBootstrapScriptsModule( $skin ) ); + } + + public static function getBootstrapScriptsModuleProvider(): array { + return [ + 'medik uses its own skin module' => [ 'medik', 'skins.medik.js' ], + 'vector uses Extension:Bootstrap' => [ 'vector', 'ext.bootstrap.scripts' ], + 'chameleon uses Extension:Bootstrap' => [ 'chameleon', 'ext.bootstrap.scripts' ], + 'unknown skin falls back to Extension:Bootstrap' => [ 'serenity', 'ext.bootstrap.scripts' ], + ]; + } + + public function testGetBootstrapScriptsModuleForTweekiDefaultsToItsScriptModule() { + $instance = new BootstrapComponentsService( new TestConfig() ); + $this->assertSame( 'skins.tweeki.scripts', $instance->getBootstrapScriptsModule( 'tweeki' ) ); + } + + public function testGetBootstrapScriptsModuleForTweekiTreatsFalseCustomModuleAsUnset() { + $config = new TestConfig(); + $config->set( 'TweekiSkinCustomScriptModule', false ); + $config->set( 'TweekiSkinUseCustomFiles', false ); + $instance = new BootstrapComponentsService( $config ); + $this->assertSame( 'skins.tweeki.scripts', $instance->getBootstrapScriptsModule( 'tweeki' ) ); + } + + public function testGetBootstrapScriptsModuleForTweekiUsesCustomFilesModule() { + $config = new TestConfig(); + $config->set( 'TweekiSkinUseCustomFiles', true ); + $instance = new BootstrapComponentsService( $config ); + $this->assertSame( 'skins.tweeki.custom.scripts', $instance->getBootstrapScriptsModule( 'tweeki' ) ); + } + + public function testGetBootstrapScriptsModuleForTweekiHonorsConfiguredScriptModule() { + $config = new TestConfig(); + $config->set( 'TweekiSkinCustomScriptModule', 'skins.tweeki.my.scripts' ); + $config->set( 'TweekiSkinUseCustomFiles', true ); + $instance = new BootstrapComponentsService( $config ); + $this->assertSame( 'skins.tweeki.my.scripts', $instance->getBootstrapScriptsModule( 'tweeki' ) ); + } + /** * @throws ReflectionException */ diff --git a/tests/phpunit/Unit/HooksHandlerTest.php b/tests/phpunit/Unit/HooksHandlerTest.php index 827d40d..3a3405e 100644 --- a/tests/phpunit/Unit/HooksHandlerTest.php +++ b/tests/phpunit/Unit/HooksHandlerTest.php @@ -6,7 +6,12 @@ use MediaWiki\Extension\BootstrapComponents\ComponentLibrary; use MediaWiki\Extension\BootstrapComponents\HooksHandler; use MediaWiki\Extension\BootstrapComponents\NestingController; +use MediaWiki\Extension\BootstrapComponents\Tests\Fixtures\TestConfig; +use MediaWiki\Output\OutputPage; +use MediaWiki\Parser\Parser; +use MediaWiki\Parser\ParserOutput; use PHPUnit\Framework\TestCase; +use Skin; /** * @covers \MediaWiki\Extension\BootstrapComponents\HooksHandler @@ -63,13 +68,69 @@ public function testOnGalleryGetModes() { $this->assertEquals( 'MediaWiki\\Extension\\BootstrapComponents\\CarouselGallery', $modes['carousel'] ); } - /** - * this hook is tested in - * @see OutputPageParserOutputTest::testHookOutputPageParserOutput - * - * @return void - */ - public function testOnOutputPageParserOutput() { - $this->assertTrue( true ); + public function testOnParserAfterParseAddsOnlyTheStylesFix() { + $parserOutput = new ParserOutput(); + $parser = $this->createMock( Parser::class ); + $parser->method( 'getOutput' )->willReturn( $parserOutput ); + $text = ''; + + $this->newHooksHandler()->onParserAfterParse( $parser, $text, null ); + + $this->assertContains( 'ext.bootstrapComponents.bootstrap.fix', $parserOutput->getModuleStyles() ); + $this->assertNotContains( 'ext.bootstrap.styles', $parserOutput->getModuleStyles() ); + $this->assertNotContains( 'ext.bootstrap.scripts', $parserOutput->getModuleStyles() ); + $this->assertNotContains( 'ext.bootstrap.styles', $parserOutput->getModules() ); + $this->assertNotContains( 'ext.bootstrap.scripts', $parserOutput->getModules() ); + } + + public function testOnBeforePageDisplayLoadsExtensionBootstrapOnNonProviderContentPage() { + $out = $this->createMock( OutputPage::class ); + $out->method( 'getModuleStyles' )->willReturn( [ 'ext.bootstrapComponents.bootstrap.fix' ] ); + $out->expects( $this->once() )->method( 'addJsConfigVars' ) + ->with( 'wgBootstrapComponentsBootstrapModules', [ 'scripts' => 'ext.bootstrap.scripts', 'styles' => 'ext.bootstrap.styles' ] ); + $out->expects( $this->once() )->method( 'addModuleStyles' ) + ->with( [ 'ext.bootstrap.styles' ] ); + $out->expects( $this->once() )->method( 'addModules' ) + ->with( [ 'ext.bootstrap.scripts' ] ); + + $this->newHooksHandler()->onBeforePageDisplay( $out, $this->newSkin( 'vector' ) ); + } + + public function testOnBeforePageDisplaySkipsExtensionBootstrapOnSkinThatShipsItsOwn() { + $out = $this->createMock( OutputPage::class ); + $out->method( 'getModuleStyles' )->willReturn( [ 'ext.bootstrapComponents.bootstrap.fix' ] ); + $out->expects( $this->once() )->method( 'addJsConfigVars' ) + ->with( 'wgBootstrapComponentsBootstrapModules', [ 'scripts' => 'skins.medik.js', 'styles' => null ] ); + $out->expects( $this->never() )->method( 'addModuleStyles' ); + $out->expects( $this->never() )->method( 'addModules' ); + + $this->newHooksHandler()->onBeforePageDisplay( $out, $this->newSkin( 'medik' ) ); + } + + + public function testOnBeforePageDisplayOnlySetsTheConfigVariableWhenNoContentWasParsed() { + $out = $this->createMock( OutputPage::class ); + $out->method( 'getModuleStyles' )->willReturn( [] ); + $out->expects( $this->once() )->method( 'addJsConfigVars' ) + ->with( 'wgBootstrapComponentsBootstrapModules', [ 'scripts' => 'ext.bootstrap.scripts', 'styles' => 'ext.bootstrap.styles' ] ); + $out->expects( $this->never() )->method( 'addModules' ); + $out->expects( $this->never() )->method( 'addModuleStyles' ); + + $this->newHooksHandler()->onBeforePageDisplay( $out, $this->newSkin( 'vector' ) ); + } + + private function newHooksHandler(): HooksHandler { + return new HooksHandler( + new BootstrapComponentsService( new TestConfig() ), + $this->createMock( ComponentLibrary::class ), + $this->createMock( NestingController::class ), + ); + } + + private function newSkin( string $skinName ): Skin { + $skin = $this->createMock( Skin::class ); + $skin->method( 'getSkinName' )->willReturn( $skinName ); + + return $skin; } } From e725bf8de33bedee0d12a667125ad153c87e19f7 Mon Sep 17 00:00:00 2001 From: Morne Alberts Date: Tue, 21 Jul 2026 16:35:27 +0200 Subject: [PATCH 2/4] Skip the Bootstrap stylesheet on Chameleon Chameleon registers Extension:Bootstrap's stylesheet under its own module name (zzz.ext.bootstrap.styles), which ResourceLoader cannot deduplicate against ext.bootstrap.styles, so pages carried the Bootstrap CSS twice. Chameleon provides its own stylesheet but consumes Extension:Bootstrap's scripts, so only the stylesheet is skipped there. Co-Authored-By: Claude Fable 5 --- docs/release-notes.md | 2 +- src/BootstrapComponentsService.php | 7 +++++-- tests/phpunit/Unit/BootstrapComponentsServiceTest.php | 1 + tests/phpunit/Unit/HooksHandlerTest.php | 11 +++++++++++ 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/docs/release-notes.md b/docs/release-notes.md index 68fbcb2..6e99cfa 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -6,7 +6,7 @@ Released on TBD Fixes: * fix dropdown menus, accordions and other click-driven components on Medik and Tweeki -* skins that provide their own Bootstrap (Medik and Tweeki) no longer also load Extension:Bootstrap's copy +* skins that provide their own Bootstrap (Chameleon, Medik and Tweeki) no longer also load Extension:Bootstrap's copy Changes: * `api.php?action=parse` no longer lists the `ext.bootstrap.*` modules; anything rendering parsed content outside a normal page view must load Bootstrap itself diff --git a/src/BootstrapComponentsService.php b/src/BootstrapComponentsService.php index 8d7ebef..d820759 100644 --- a/src/BootstrapComponentsService.php +++ b/src/BootstrapComponentsService.php @@ -119,9 +119,12 @@ public function getBootstrapStylesModule( string $skin ): ?string { } /** - * Skins that put the Bootstrap stylesheet on the page themselves. + * Skins that put the Bootstrap stylesheet on the page themselves. Chameleon belongs here but + * not in the scripts map: it registers Extension:Bootstrap's stylesheet under its own module + * name (zzz.ext.bootstrap.styles), which ResourceLoader cannot deduplicate against + * ext.bootstrap.styles, while its Bootstrap JavaScript is ext.bootstrap.scripts itself. */ - private const SKINS_WITH_OWN_BOOTSTRAP_STYLES = [ 'medik', 'tweeki' ]; + private const SKINS_WITH_OWN_BOOTSTRAP_STYLES = [ 'chameleon', 'medik', 'tweeki' ]; public function skinProvidesBootstrapStyles( string $skin ): bool { return in_array( strtolower( $skin ), self::SKINS_WITH_OWN_BOOTSTRAP_STYLES, true ); diff --git a/tests/phpunit/Unit/BootstrapComponentsServiceTest.php b/tests/phpunit/Unit/BootstrapComponentsServiceTest.php index 3a793c8..a1427e2 100644 --- a/tests/phpunit/Unit/BootstrapComponentsServiceTest.php +++ b/tests/phpunit/Unit/BootstrapComponentsServiceTest.php @@ -91,6 +91,7 @@ public function testSkinProvidesBootstrapStyles( string $skin, bool $expected ) public static function skinProvidesBootstrapStylesProvider(): array { return [ + 'chameleon registers its own copy of the stylesheet' => [ 'chameleon', true ], 'medik ships its own stylesheet' => [ 'medik', true ], 'tweeki ships its own stylesheet' => [ 'tweeki', true ], 'vector does not' => [ 'vector', false ], diff --git a/tests/phpunit/Unit/HooksHandlerTest.php b/tests/phpunit/Unit/HooksHandlerTest.php index 3a3405e..aa1fb6c 100644 --- a/tests/phpunit/Unit/HooksHandlerTest.php +++ b/tests/phpunit/Unit/HooksHandlerTest.php @@ -107,6 +107,17 @@ public function testOnBeforePageDisplaySkipsExtensionBootstrapOnSkinThatShipsIts $this->newHooksHandler()->onBeforePageDisplay( $out, $this->newSkin( 'medik' ) ); } + public function testOnBeforePageDisplaySkipsOnlyTheStylesheetOnChameleon() { + $out = $this->createMock( OutputPage::class ); + $out->method( 'getModuleStyles' )->willReturn( [ 'ext.bootstrapComponents.bootstrap.fix' ] ); + $out->expects( $this->once() )->method( 'addJsConfigVars' ) + ->with( 'wgBootstrapComponentsBootstrapModules', [ 'scripts' => 'ext.bootstrap.scripts', 'styles' => null ] ); + $out->expects( $this->never() )->method( 'addModuleStyles' ); + $out->expects( $this->once() )->method( 'addModules' ) + ->with( [ 'ext.bootstrap.scripts' ] ); + + $this->newHooksHandler()->onBeforePageDisplay( $out, $this->newSkin( 'chameleon' ) ); + } public function testOnBeforePageDisplayOnlySetsTheConfigVariableWhenNoContentWasParsed() { $out = $this->createMock( OutputPage::class ); From 23290215d36f4207f3569696cd5f510c7bea8e4c Mon Sep 17 00:00:00 2001 From: Morne Alberts Date: Tue, 21 Jul 2026 16:35:27 +0200 Subject: [PATCH 3/4] Tolerate a non-string Tweeki custom script module Tweeki passes $wgTweekiSkinCustomScriptModule straight to OutputPage::addModules, which also accepts arrays, so a truthy non-string value works there even though nothing documents it. We mirror only the string form; any other truthy value falls back to Extension:Bootstrap's copy, the previously released behavior, instead of fataling on every page view. Co-Authored-By: Claude Fable 5 --- src/BootstrapComponentsService.php | 4 +++- tests/phpunit/Unit/BootstrapComponentsServiceTest.php | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/BootstrapComponentsService.php b/src/BootstrapComponentsService.php index d820759..a5f912c 100644 --- a/src/BootstrapComponentsService.php +++ b/src/BootstrapComponentsService.php @@ -104,7 +104,9 @@ private function getTweekiScriptModule(): string { if ( $this->mainConfig->has( 'TweekiSkinCustomScriptModule' ) && $this->mainConfig->get( 'TweekiSkinCustomScriptModule' ) ) { - return $this->mainConfig->get( 'TweekiSkinCustomScriptModule' ); + $module = $this->mainConfig->get( 'TweekiSkinCustomScriptModule' ); + // A shape we do not mirror; fall back to Extension:Bootstrap's copy. + return is_string( $module ) ? $module : 'ext.bootstrap.scripts'; } if ( $this->mainConfig->has( 'TweekiSkinUseCustomFiles' ) && $this->mainConfig->get( 'TweekiSkinUseCustomFiles' ) diff --git a/tests/phpunit/Unit/BootstrapComponentsServiceTest.php b/tests/phpunit/Unit/BootstrapComponentsServiceTest.php index a1427e2..4d0f3a5 100644 --- a/tests/phpunit/Unit/BootstrapComponentsServiceTest.php +++ b/tests/phpunit/Unit/BootstrapComponentsServiceTest.php @@ -144,6 +144,13 @@ public function testGetBootstrapScriptsModuleForTweekiHonorsConfiguredScriptModu $this->assertSame( 'skins.tweeki.my.scripts', $instance->getBootstrapScriptsModule( 'tweeki' ) ); } + public function testGetBootstrapScriptsModuleForTweekiFallsBackOnANonStringModule() { + $config = new TestConfig(); + $config->set( 'TweekiSkinCustomScriptModule', [ 'skins.a', 'skins.b' ] ); + $instance = new BootstrapComponentsService( $config ); + $this->assertSame( 'ext.bootstrap.scripts', $instance->getBootstrapScriptsModule( 'tweeki' ) ); + } + /** * @throws ReflectionException */ From 7c23f6d3b916e25244557d9a537d49e7c825473b Mon Sep 17 00:00:00 2001 From: Morne Alberts Date: Tue, 21 Jul 2026 16:35:27 +0200 Subject: [PATCH 4/4] Initialize components with getOrCreateInstance Initialization runs on every wikipage.content fire, and new stacks a second instance on an element initialized in an earlier pass, with the first instance's listeners left bound. getOrCreateInstance is idempotent, and it is also order-independent against Bootstrap's own data-api, which instantiates carousels with data-bs-ride itself. Co-Authored-By: Claude Fable 5 --- modules/ext.bootstrapComponents.carousel.js | 2 +- modules/ext.bootstrapComponents.popover.js | 2 +- modules/ext.bootstrapComponents.tooltip.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/ext.bootstrapComponents.carousel.js b/modules/ext.bootstrapComponents.carousel.js index 264ea0c..f459a0d 100644 --- a/modules/ext.bootstrapComponents.carousel.js +++ b/modules/ext.bootstrapComponents.carousel.js @@ -36,7 +36,7 @@ return; } content.querySelectorAll( '.carousel' ).forEach( function ( element ) { - new Carousel( element ); + Carousel.getOrCreateInstance( element ); } ); } ); }() ); diff --git a/modules/ext.bootstrapComponents.popover.js b/modules/ext.bootstrapComponents.popover.js index 5002d9a..4176832 100644 --- a/modules/ext.bootstrapComponents.popover.js +++ b/modules/ext.bootstrapComponents.popover.js @@ -36,7 +36,7 @@ return; } content.querySelectorAll( '[data-bs-toggle="popover"]' ).forEach( function ( el ) { - new Popover( el, { html: true } ); + Popover.getOrCreateInstance( el, { html: true } ); } ); } ); }() ); diff --git a/modules/ext.bootstrapComponents.tooltip.js b/modules/ext.bootstrapComponents.tooltip.js index d2dcd72..883dcdc 100644 --- a/modules/ext.bootstrapComponents.tooltip.js +++ b/modules/ext.bootstrapComponents.tooltip.js @@ -36,7 +36,7 @@ return; } content.querySelectorAll( '[data-bs-toggle="tooltip"]' ).forEach( function ( el ) { - new Tooltip( el ); + Tooltip.getOrCreateInstance( el ); } ); } ); }() );