Skip loading Bootstrap when the active skin provides it#113
Draft
malberts wants to merge 1 commit into
Draft
Conversation
malberts
force-pushed
the
fix/skip-bootstrap-for-skins-providing-it
branch
from
July 16, 2026 21:45
9b26c13 to
4ac37e6
Compare
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) <noreply@anthropic.com>
malberts
force-pushed
the
fix/skip-bootstrap-for-skins-providing-it
branch
from
July 17, 2026 09:08
4ac37e6 to
23f5c68
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For #70
Extension:Bootstrap's
ext.bootstrap.stylesandext.bootstrap.scriptswere added inParserAfterParse. That is the parse phase, so the decision is baked into the skin-agnostic parser cache and cannot depend on the skin. They are now added in theOutputPageParserOutputhook, where the active skin is known, and skipped for skins that put Bootstrap on the page themselves. The list of those skins is internal (Chameleon, Medik, Tweeki) rather than configurable, since whether a skin loads its own Bootstrap is a fact about the skin, not a per-wiki policy.Extension:Bootstrap stays a Composer
require, the init gate is untouched, andSetupAfterCache/addAllBootstrapModules()is untouched: that call only configures what Extension:Bootstrap compiles intoext.bootstrap.styles, it puts nothing on a page. Not skipping remains the safe default, so skipping is an optimisation rather than a load-bearing correctness decision. The rationale is worked through in the issue.Evidence
Verified on one MediaWiki 1.43 wiki with Chameleon, Medik 6.0.0, Tweeki 5.43.2, Extension:Bootstrap 6.0.0 and the bundled core skins, all exercising the same page (alert, badge, button, card, carousel, modal, popover, tooltip, accordion, collapse, jumbotron) through
?useskin=. Because the decision is now made at output phase, one page serves a different module set per skin, which is also the regression test: a parse-phase decision cannot do this.Distinct stylesheets delivering the Bootstrap library, per skin.
ext.bootstrapComponents.button.fixis excluded throughout: it is this extension's own fix CSS, not a Bootstrap build.ext.bootstrap.styles+zzz.ext.bootstrap.styles(2)zzz.ext.bootstrap.styles(1)ext.bootstrap.styles+skins.medik(2)skins.medik(1)ext.bootstrap.styles+skins.tweeki.styles(2)skins.tweeki.styles(1)ext.bootstrap.styles(1)ext.bootstrap.styles(1)ext.bootstrap.styles(1)ext.bootstrap.styles(1)ext.bootstrap.styles(1)ext.bootstrap.styles(1)ext.bootstrap.styles(1)ext.bootstrap.styles(1)Chameleon's double request, the case reported in the issue, is gone:
Components were exercised in a browser (automated, Playwright) on every skin above. Modal opens, dismissible alert closes, carousel cycles, tooltip and popover trigger, accordion toggles, Bootstrap's own CSS still applies (
.alert-inforesolves to Bootstrap'srgb(207, 244, 252)on Medik and Tweeki, from the skin's own copy). One pre-existing failure is unchanged by this pull request: the accordion does not toggle on Medik, and it does not toggle onmastereither, so it is not a regression from this change.Bootstrap's JavaScript is only partly addressed
Skipping
addModules( [ 'ext.bootstrap.scripts' ] )does not keep Extension:Bootstrap's JavaScript off a Medik or Tweeki page, becauseext.bootstrapComponents.carousel.fix,.modal.fix,.popover.fixand.tooltip.fixeach declareext.bootstrap.scriptsas a ResourceLoader dependency inextension.json. Any page using one of those components still pulls it in, alongside the skin's own bundle. Confirmed in the browser: on Tweeki the page requests noext.bootstrap.styles, butext.bootstrap.scriptsstill arrives as a dependency of the fix modules.So this change removes the duplicate Bootstrap CSS everywhere, and removes the duplicate Bootstrap JavaScript only on pages with no JavaScript-backed component. On Chameleon the JavaScript was never the problem, since Chameleon registers it under the same module name and ResourceLoader deduplicates it.
Resolving that properly means changing what those modules depend on, which cannot be expressed as a per-skin condition in
extension.jsonand is a larger change than this one. It seems better suited to its own issue.Known behaviour change:
action=parsewithoutuseskinext.bootstrap.stylesandext.bootstrap.scriptsused to sit on theParserOutput, which travels with the parsed content. They now sit on theOutputPage, which only exists when a page is rendered for a skin. Soapi.php?action=parsewithout auseskinparameter no longer lists them. The scripts still reach any caller that loads the carousel, modal, popover or tooltip fix modules, since those depend onext.bootstrap.scripts. The styles have no such dependency, so they simply go missing.With
useskinthe hook runs and the answer is correct per skin, which is an improvement on master: master answered the same way for every skin, so on a Chameleon wiki it told callers to load Bootstrap that Chameleon already provides.Ordinary page views are unaffected and no known caller depends on this, so it is left as-is for now. Adding
ext.bootstrap.stylesas a dependency of the fix modules would restore it only by reintroducing the double load, since ResourceLoader resolves dependencies client-side and cannot know the skin.Tests
OutputPageParserOutputTestasserts the outcome on a realOutputPage:ext.bootstrap.stylesandext.bootstrap.scriptsare present in its module lists for a skin that does not load Bootstrap, and absent for chameleon, medik and tweeki. Asserting the resulting module list rather than the calls made to a mock keeps the tests meaningful across a refactor of how the modules get added. Both directions were seen failing before the change: the two "adds" cases fail againstmaster, and the six "omits" cases fail when the skin guard is removed.The two existing tests in that file asserted
addModulesinvocations on a mockedOutputPage, which this change necessarily breaks, so they were rewritten in the same style.