From 23f5c681627a026ba2961ba0b0aed755483975ab Mon Sep 17 00:00:00 2001 From: Morne Alberts Date: Thu, 16 Jul 2026 19:59:20 +0200 Subject: [PATCH 1/2] Skip loading Bootstrap when the active skin provides it Extension:Bootstrap's ext.bootstrap.styles and ext.bootstrap.scripts were added in ParserAfterParse, which is baked into the skin-agnostic parser cache. Move them to the OutputPageParserOutput hook, where the active skin is known, and skip them for skins that put Bootstrap on the page themselves. Chameleon registers Extension:Bootstrap's styles under the name zzz.ext.bootstrap.styles, which ResourceLoader cannot deduplicate against ext.bootstrap.styles, so a Chameleon page requested the same Bootstrap CSS twice. Medik and Tweeki bundle their own Bootstrap, so they received a second, different build. Skins that do not load Bootstrap keep getting it from Extension:Bootstrap. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/known-issues.md | 23 +++++ docs/release-notes.md | 3 + src/Hooks/OutputPageParserOutput.php | 24 +++++ src/HooksHandler.php | 2 - .../Unit/Hooks/OutputPageParserOutputTest.php | 90 ++++++++++++++----- 5 files changed, 120 insertions(+), 22 deletions(-) diff --git a/docs/known-issues.md b/docs/known-issues.md index 23b8ea5..a6a3dc4 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -2,6 +2,29 @@ Some components cause problems with other components or "external" elements. +### Skins that load Bootstrap themselves +Some skins put Bootstrap on the page on their own. For those, this extension does not +request Extension:Bootstrap's `ext.bootstrap.styles`, since that would load Bootstrap's +CSS twice. Chameleon, Medik and Tweeki are treated this way. Every other skin, Vector +and Timeless among them, keeps getting Bootstrap from Extension:Bootstrap. + +Bootstrap's JavaScript is only partly covered by this. This extension stops requesting +`ext.bootstrap.scripts` for those skins as well, but its own carousel, modal, popover +and tooltip modules declare that module as a dependency, so a page using any of those +components still loads it. On Chameleon that is harmless, because Chameleon pulls in +the same module and ResourceLoader serves it once. On Medik and Tweeki it means +Extension:Bootstrap's JavaScript runs alongside the skin's own bundled copy. + +With Medik and Tweeki, matching the Bootstrap versions is up to you. They bundle their +own Bootstrap and declare no dependency on Extension:Bootstrap, so nothing checks that +their Bootstrap matches the one this extension expects. BootstrapComponents 6.x targets +Bootstrap 5.3, so pair it with a Bootstrap 5 release of these skins, and stay on +BootstrapComponents 5.x for their Bootstrap 4 releases. Chameleon is not affected: it +depends on Extension:Bootstrap itself, so Composer keeps the two in step. + +This extension cannot read the exact Bootstrap version a skin provides, so a mismatch +surfaces as a component that looks or behaves wrong, not as an error message. + ### Modals and popovers When you put popovers on a page with modals (or image modals), the modals break. diff --git a/docs/release-notes.md b/docs/release-notes.md index 496db85..b362e45 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -8,6 +8,9 @@ Changes: * add a migration guide for the Bootstrap 4 to 5 upgrade * split the migration guide into one document per upgrade and link both guides from the README +Fixes: +* fix Bootstrap's CSS loading twice on the Chameleon, Medik, and Tweeki skins, which provide Bootstrap themselves + ### BootstrapComponents 6.0.0 Released on 16-July-2026 diff --git a/src/Hooks/OutputPageParserOutput.php b/src/Hooks/OutputPageParserOutput.php index bcea7f2..d4e9fb1 100644 --- a/src/Hooks/OutputPageParserOutput.php +++ b/src/Hooks/OutputPageParserOutput.php @@ -40,6 +40,13 @@ */ class OutputPageParserOutput { + /** + * Skins that put Bootstrap on the page themselves, either by bundling their own copy + * or, in Chameleon's case, by registering Extension:Bootstrap's modules under a + * different name that ResourceLoader cannot deduplicate against ext.bootstrap.*. + */ + private const SKINS_LOADING_BOOTSTRAP_THEMSELVES = [ 'chameleon', 'medik', 'tweeki' ]; + public function __construct( private readonly OutputPage $outputPage, private readonly BootstrapComponentsService $bootstrapComponentService, @@ -50,11 +57,28 @@ public function __construct( * @return void */ public function process(): void { + $this->addBootstrapModules(); + if ( $this->getBootstrapComponentsService()->vectorSkinInUse() ) { $this->getOutputPage()->addModules( [ 'ext.bootstrapComponents.vector-fix' ] ); } } + private function addBootstrapModules(): void { + if ( $this->activeSkinLoadsBootstrapItself() ) { + return; + } + + $this->getOutputPage()->addModuleStyles( [ 'ext.bootstrap.styles' ] ); + $this->getOutputPage()->addModules( [ 'ext.bootstrap.scripts' ] ); + } + + private function activeSkinLoadsBootstrapItself(): bool { + $activeSkin = strtolower( $this->getOutputPage()->getSkin()->getSkinName() ?? '' ); + + return in_array( $activeSkin, self::SKINS_LOADING_BOOTSTRAP_THEMSELVES, true ); + } + protected function getBootstrapComponentsService(): BootstrapComponentsService { return $this->bootstrapComponentService; } diff --git a/src/HooksHandler.php b/src/HooksHandler.php index 2f2d49a..2ceaef6 100644 --- a/src/HooksHandler.php +++ b/src/HooksHandler.php @@ -197,8 +197,6 @@ 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' ] ); $skin = $this->getBootstrapComponentsService()->getNameOfActiveSkin(); foreach ( $this->getBootstrapComponentsService()->getActiveComponents() as $activeComponent ) { if ( !$this->getComponentLibrary()->isRegistered( $activeComponent ) ) { diff --git a/tests/phpunit/Unit/Hooks/OutputPageParserOutputTest.php b/tests/phpunit/Unit/Hooks/OutputPageParserOutputTest.php index 5098ece..de50e3c 100644 --- a/tests/phpunit/Unit/Hooks/OutputPageParserOutputTest.php +++ b/tests/phpunit/Unit/Hooks/OutputPageParserOutputTest.php @@ -2,10 +2,12 @@ namespace MediaWiki\Extension\BootstrapComponents\Tests\Unit\Hooks; +use MediaWiki\Context\RequestContext; use MediaWiki\Extension\BootstrapComponents\BootstrapComponentsService; use MediaWiki\Extension\BootstrapComponents\Hooks\OutputPageParserOutput; use MediaWiki\Output\OutputPage; use PHPUnit\Framework\TestCase; +use Skin; /** * @covers \MediaWiki\Extension\BootstrapComponents\Hooks\OutputPageParserOutput @@ -37,33 +39,81 @@ public function testCanConstruct() { ); } + public function testAddsBootstrapStylesForSkinWithoutOwnBootstrap() { + $outputPage = $this->newOutputPageForSkin( 'monobook' ); + + $this->process( $outputPage, vectorSkinInUse: false ); + + $this->assertContains( 'ext.bootstrap.styles', $outputPage->getModuleStyles() ); + } + + public function testAddsBootstrapScriptsForSkinWithoutOwnBootstrap() { + $outputPage = $this->newOutputPageForSkin( 'monobook' ); + + $this->process( $outputPage, vectorSkinInUse: false ); + + $this->assertContains( 'ext.bootstrap.scripts', $outputPage->getModules() ); + } + + /** + * @dataProvider skinLoadingBootstrapItselfProvider + */ + public function testOmitsBootstrapStylesForSkinLoadingBootstrapItself( string $skinName ) { + $outputPage = $this->newOutputPageForSkin( $skinName ); + + $this->process( $outputPage, vectorSkinInUse: false ); + + $this->assertNotContains( 'ext.bootstrap.styles', $outputPage->getModuleStyles() ); + } + + /** + * @dataProvider skinLoadingBootstrapItselfProvider + */ + public function testOmitsBootstrapScriptsForSkinLoadingBootstrapItself( string $skinName ) { + $outputPage = $this->newOutputPageForSkin( $skinName ); + + $this->process( $outputPage, vectorSkinInUse: false ); + + $this->assertNotContains( 'ext.bootstrap.scripts', $outputPage->getModules() ); + } + + public static function skinLoadingBootstrapItselfProvider(): array { + return [ + 'chameleon' => [ 'chameleon' ], + 'medik' => [ 'medik' ], + 'tweeki' => [ 'tweeki' ], + ]; + } + public function testHookOutputPageParserOutputLoadsVectorFixUnderVector() { - $outputPage = $this->createMock( OutputPage::class ); - $outputPage->expects( $this->never() )->method( 'addHTML' ); - $outputPage->expects( $this->once() ) - ->method( 'addModules' ) - ->with( - $this->equalTo( [ 'ext.bootstrapComponents.vector-fix' ] ) - ); + $outputPage = $this->newOutputPageForSkin( 'vector' ); - $bootstrapService = $this->createMock( BootstrapComponentsService::class ); - $bootstrapService->expects( $this->once() ) - ->method( 'vectorSkinInUse' ) - ->willReturn( true ); + $this->process( $outputPage, vectorSkinInUse: true ); - $instance = new OutputPageParserOutput( $outputPage, $bootstrapService ); - $instance->process(); + $this->assertContains( 'ext.bootstrapComponents.vector-fix', $outputPage->getModules() ); } - public function testHookDoesNothingWhenNotVector() { - $outputPage = $this->createMock( OutputPage::class ); - $outputPage->expects( $this->never() )->method( 'addHTML' ); - $outputPage->expects( $this->never() )->method( 'addModules' ); + public function testOmitsVectorFixWhenVectorIsNotInUse() { + $outputPage = $this->newOutputPageForSkin( 'monobook' ); + + $this->process( $outputPage, vectorSkinInUse: false ); + + $this->assertNotContains( 'ext.bootstrapComponents.vector-fix', $outputPage->getModules() ); + } + + private function newOutputPageForSkin( string $skinName ): OutputPage { + $skin = $this->createMock( Skin::class ); + $skin->method( 'getSkinName' )->willReturn( $skinName ); + + $context = new RequestContext(); + $context->setSkin( $skin ); + + return new OutputPage( $context ); + } + private function process( OutputPage $outputPage, bool $vectorSkinInUse ): void { $bootstrapService = $this->createMock( BootstrapComponentsService::class ); - $bootstrapService->expects( $this->once() ) - ->method( 'vectorSkinInUse' ) - ->willReturn( false ); + $bootstrapService->method( 'vectorSkinInUse' )->willReturn( $vectorSkinInUse ); $instance = new OutputPageParserOutput( $outputPage, $bootstrapService ); $instance->process(); From a739d8e0e3aff32efae266631c951fa078015d8e Mon Sep 17 00:00:00 2001 From: Morne Alberts Date: Mon, 20 Jul 2026 14:24:28 +0200 Subject: [PATCH 2/2] Resolve component modules' Bootstrap dependency per active skin The carousel, modal, popover and tooltip fix modules each ship an init script that calls the Bootstrap JavaScript global, so they must load after Bootstrap. The static ext.bootstrap.scripts dependency guaranteed that ordering but forced Extension:Bootstrap's Bootstrap onto every skin, including skins that bundle their own. On those skins the page carried two copies of Bootstrap, and because each registers Bootstrap's data-api on the document, events fired twice. Route the four modules through BootstrapDependentModule, whose getDependencies() follows the active skin: a skin that bundles its own Bootstrap depends on that skin's module instead of ext.bootstrap.scripts, keeping the load-order guarantee without the duplicate. The map currently swaps medik to skins.medik.js. Other skins keep ext.bootstrap.scripts; Chameleon loads it itself under the same name, so ResourceLoader deduplicates it. Tweeki also bundles Bootstrap but exposes no JavaScript global for the init scripts to reach, so it stays on ext.bootstrap.scripts until the init scripts learn the jQuery plugin bridge. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/known-issues.md | 20 +++--- docs/release-notes.md | 1 + extension.json | 8 +-- src/Hooks/OutputPageParserOutput.php | 4 +- .../BootstrapDependentModule.php | 29 +++++++++ .../BootstrapDependentModuleTest.php | 65 +++++++++++++++++++ 6 files changed, 113 insertions(+), 14 deletions(-) create mode 100644 src/ResourceLoader/BootstrapDependentModule.php create mode 100644 tests/phpunit/Unit/ResourceLoader/BootstrapDependentModuleTest.php diff --git a/docs/known-issues.md b/docs/known-issues.md index a6a3dc4..aae691f 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -8,12 +8,14 @@ request Extension:Bootstrap's `ext.bootstrap.styles`, since that would load Boot CSS twice. Chameleon, Medik and Tweeki are treated this way. Every other skin, Vector and Timeless among them, keeps getting Bootstrap from Extension:Bootstrap. -Bootstrap's JavaScript is only partly covered by this. This extension stops requesting -`ext.bootstrap.scripts` for those skins as well, but its own carousel, modal, popover -and tooltip modules declare that module as a dependency, so a page using any of those -components still loads it. On Chameleon that is harmless, because Chameleon pulls in -the same module and ResourceLoader serves it once. On Medik and Tweeki it means -Extension:Bootstrap's JavaScript runs alongside the skin's own bundled copy. +Bootstrap's JavaScript is handled the same way, per skin. The carousel, modal, popover +and tooltip modules depend on the active skin's own Bootstrap when it provides one, so +Medik gets its `skins.medik.js` instead of Extension:Bootstrap's copy, and Chameleon +shares the single `ext.bootstrap.scripts` it already loads. Tweeki is the exception: it +bundles Bootstrap but exposes no `window.bootstrap` global for those modules' init +scripts to reach, so it still loads Extension:Bootstrap's JavaScript alongside its own. +Making Tweeki clean needs those init scripts to fall back to Bootstrap's jQuery plugin +bridge. With Medik and Tweeki, matching the Bootstrap versions is up to you. They bundle their own Bootstrap and declare no dependency on Extension:Bootstrap, so nothing checks that @@ -22,8 +24,10 @@ Bootstrap 5.3, so pair it with a Bootstrap 5 release of these skins, and stay on BootstrapComponents 5.x for their Bootstrap 4 releases. Chameleon is not affected: it depends on Extension:Bootstrap itself, so Composer keeps the two in step. -This extension cannot read the exact Bootstrap version a skin provides, so a mismatch -surfaces as a component that looks or behaves wrong, not as an error message. +This extension cannot read the exact Bootstrap version a skin provides where it decides +what to load, on the server. That version is only visible in the browser, too late to +act on. So a mismatch surfaces as a component that looks or behaves wrong, not as an +error message. ### Modals and popovers When you put popovers on a page with modals (or image modals), diff --git a/docs/release-notes.md b/docs/release-notes.md index b362e45..33c66e5 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -10,6 +10,7 @@ Changes: Fixes: * fix Bootstrap's CSS loading twice on the Chameleon, Medik, and Tweeki skins, which provide Bootstrap themselves +* fix Bootstrap's JavaScript loading twice on the Medik skin, which broke its navbar and action dropdowns ### BootstrapComponents 6.0.0 diff --git a/extension.json b/extension.json index 89b6e5c..2b43883 100644 --- a/extension.json +++ b/extension.json @@ -104,12 +104,12 @@ "styles": "ext.bootstrapComponents.card.fix.css" }, "ext.bootstrapComponents.carousel.fix": { - "dependencies": "ext.bootstrap.scripts", + "class": "MediaWiki\\Extension\\BootstrapComponents\\ResourceLoader\\BootstrapDependentModule", "styles": "ext.bootstrapComponents.carousel.fix.css", "scripts": "ext.bootstrapComponents.carousel.js" }, "ext.bootstrapComponents.modal.fix": { - "dependencies": "ext.bootstrap.scripts", + "class": "MediaWiki\\Extension\\BootstrapComponents\\ResourceLoader\\BootstrapDependentModule", "styles": "ext.bootstrapComponents.modal.fix.css", "scripts": "ext.bootstrapComponents.modal.js" }, @@ -117,14 +117,14 @@ "styles": "ext.bootstrapComponents.modal.vector-fix.css" }, "ext.bootstrapComponents.popover.fix": { - "dependencies": "ext.bootstrap.scripts", + "class": "MediaWiki\\Extension\\BootstrapComponents\\ResourceLoader\\BootstrapDependentModule", "scripts": "ext.bootstrapComponents.popover.js" }, "ext.bootstrapComponents.popover.vector-fix": { "styles": "ext.bootstrapComponents.popover.vector-fix.css" }, "ext.bootstrapComponents.tooltip.fix": { - "dependencies": "ext.bootstrap.scripts", + "class": "MediaWiki\\Extension\\BootstrapComponents\\ResourceLoader\\BootstrapDependentModule", "scripts": "ext.bootstrapComponents.tooltip.js", "styles": "ext.bootstrapComponents.tooltip.fix.css" }, diff --git a/src/Hooks/OutputPageParserOutput.php b/src/Hooks/OutputPageParserOutput.php index d4e9fb1..6d90e7f 100644 --- a/src/Hooks/OutputPageParserOutput.php +++ b/src/Hooks/OutputPageParserOutput.php @@ -45,7 +45,7 @@ class OutputPageParserOutput { * or, in Chameleon's case, by registering Extension:Bootstrap's modules under a * different name that ResourceLoader cannot deduplicate against ext.bootstrap.*. */ - private const SKINS_LOADING_BOOTSTRAP_THEMSELVES = [ 'chameleon', 'medik', 'tweeki' ]; + private const BOOTSTRAP_SKINS = [ 'chameleon', 'medik', 'tweeki' ]; public function __construct( private readonly OutputPage $outputPage, @@ -76,7 +76,7 @@ private function addBootstrapModules(): void { private function activeSkinLoadsBootstrapItself(): bool { $activeSkin = strtolower( $this->getOutputPage()->getSkin()->getSkinName() ?? '' ); - return in_array( $activeSkin, self::SKINS_LOADING_BOOTSTRAP_THEMSELVES, true ); + return in_array( $activeSkin, self::BOOTSTRAP_SKINS, true ); } protected function getBootstrapComponentsService(): BootstrapComponentsService { diff --git a/src/ResourceLoader/BootstrapDependentModule.php b/src/ResourceLoader/BootstrapDependentModule.php new file mode 100644 index 0000000..8fc941d --- /dev/null +++ b/src/ResourceLoader/BootstrapDependentModule.php @@ -0,0 +1,29 @@ + 'skins.medik.js', + ]; + + /** @inheritDoc */ + public function getDependencies( ?Context $context = null ) { + $skin = $context?->getSkin() ?? ''; + $bootstrap = self::BOOTSTRAP_SCRIPTS_MODULE_BY_SKIN[ $skin ] ?? self::DEFAULT_BOOTSTRAP_SCRIPTS_MODULE; + + return array_merge( parent::getDependencies( $context ), [ $bootstrap ] ); + } +} diff --git a/tests/phpunit/Unit/ResourceLoader/BootstrapDependentModuleTest.php b/tests/phpunit/Unit/ResourceLoader/BootstrapDependentModuleTest.php new file mode 100644 index 0000000..6f88f66 --- /dev/null +++ b/tests/phpunit/Unit/ResourceLoader/BootstrapDependentModuleTest.php @@ -0,0 +1,65 @@ +newModuleWithDependencies( [ 'some.other.module' ] ); + + $dependencies = $module->getDependencies( $this->newContextForSkin( 'medik' ) ); + + $this->assertContains( 'skins.medik.js', $dependencies ); + $this->assertNotContains( 'ext.bootstrap.scripts', $dependencies ); + $this->assertContains( 'some.other.module', $dependencies ); + } + + /** + * @dataProvider skinWithoutReachableOwnBootstrapProvider + */ + public function testAddsExtBootstrapScriptsForOtherSkins( string $skin ) { + $module = $this->newModuleWithDependencies( [] ); + + $this->assertSame( + [ 'ext.bootstrap.scripts' ], + $module->getDependencies( $this->newContextForSkin( $skin ) ) + ); + } + + public static function skinWithoutReachableOwnBootstrapProvider(): array { + return [ + 'vector' => [ 'vector' ], + 'chameleon' => [ 'chameleon' ], + 'tweeki' => [ 'tweeki' ], + 'monobook' => [ 'monobook' ], + ]; + } + + public function testAddsExtBootstrapScriptsWhenNoSkinContext() { + $module = $this->newModuleWithDependencies( [] ); + + $this->assertSame( [ 'ext.bootstrap.scripts' ], $module->getDependencies() ); + } + + private function newModuleWithDependencies( array $dependencies ): BootstrapDependentModule { + return new BootstrapDependentModule( [ 'dependencies' => $dependencies ] ); + } + + private function newContextForSkin( string $skin ): Context { + $context = $this->createMock( Context::class ); + $context->method( 'getSkin' )->willReturn( $skin ); + return $context; + } +}