diff --git a/docs/known-issues.md b/docs/known-issues.md index 23b8ea5..aae691f 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -2,6 +2,33 @@ 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 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 +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 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), the modals break. diff --git a/docs/release-notes.md b/docs/release-notes.md index 496db85..33c66e5 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -8,6 +8,10 @@ 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 +* fix Bootstrap's JavaScript loading twice on the Medik skin, which broke its navbar and action dropdowns + ### BootstrapComponents 6.0.0 Released on 16-July-2026 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 bcea7f2..6d90e7f 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 BOOTSTRAP_SKINS = [ '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::BOOTSTRAP_SKINS, 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/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/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(); 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; + } +}