Fix modal trigger not opening modals on MediaWiki 1.43+#75
Merged
Conversation
malberts
force-pushed
the
fix/modal-mw143
branch
3 times, most recently
from
7e666d2 to
ff8f3df
Compare
The deferred-content mechanism (ParserOutput::setExtensionData on the parse side, ParserOutput::getExtensionData in the OutputPageParserOutput hook) no longer preserves data across the parser hook -> render hook transition under MediaWiki 1.43's ParserOutputAccess lifecycle changes. The set-side and get-side operate on different ParserOutput instances, so the modal container markup never reaches the page. The modal trigger still renders but has nothing to open. Switch to inline emission: ModalBuilder::parse() returns the trigger HTML and the modal container HTML concatenated. The modal container is position: fixed and addressed via data-target="#id", so its DOM placement is no longer significant. This works identically on every supported MediaWiki version because the underlying Bootstrap behaviour is the same and RemexHtml (default since MW 1.36) cleanly handles a block element inside a wikitext paragraph. Removes the now-unused ParserOutputHelper::injectLater() helper and the EXTENSION_DATA_DEFERRED_CONTENT_KEY constant. Updates the four parallel test files (ModalBuilderTest, ImageModalTest, ModalTest, OutputPageParserOutputTest) to assert the new concatenated return shape and drop the deferred-content mock expectations. Verified on a MW 1.39 + Vector + BC stack: modal opens identically pre- and post-patch (no regression on the oldest supported MW version). PHPUnit unit suite stays green relative to baseline; the 6 pre-existing ImageModalTriggerTest failures are present in both runs and unrelated to this change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
malberts
added a commit
that referenced
this pull request
May 21, 2026
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 <noreply@anthropic.com>
This was referenced May 21, 2026
Merged
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
1 participant
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.
Partial fix for #68 — addresses the modal symptom only. The tooltip and popover symptoms of #68 will follow in a separate PR.
The deferred-content mechanism that historically passed modal markup from the parser hook to
OutputPageParserOutputviaParserOutput::setExtensionDatano longer works under MediaWiki 1.43+ — theParserOutputAccesslifecycle changes between 1.39 and 1.43 mean the set-side and get-side operate on differentParserOutputinstances. The modal trigger renders, but the modal container never reaches the page.This PR switches
ModalBuilder::parse()to inline emission: trigger HTML and modal container HTML are returned concatenated. The modal container isposition: fixedand addressed viadata-target="#id", so its DOM placement is irrelevant; it can sit as a sibling of its trigger in the article body. Block-in-inline placements are handled cleanly by RemexHtml (default since MW 1.36).Scope
ModalBuilder.php,ParserOutputHelper.php,BootstrapComponents.php,Hooks/OutputPageParserOutput.php,ApplicationFactory.phpNet: +41 / −123 lines. No JS, CSS, or extension.json changes — purely the deferred-content rewrite.
Two now-unused fields (
ModalBuilder::$parserOutputHelper,OutputPageParserOutput::$parserOutput) and their associated constructor parameters / getter are marked@deprecatedrather than removed, so this stays a SemVer-clean maintenance fix. They will be dropped in the next major release.Verification
Click the trigger on a wikitext-
<bootstrap_modal>page on two stacks (MW 1.43.8 + Chameleon BS4 baseline; MW 1.39.13 + Vector), with and without the patch:mastertip)click does nothing — modal div absent from DOM
modal opens
modal opens — bug doesn't manifest on this MW
modal still opens — no regression
A Vector-only CSS override in BC suppresses the modal-backdrop on Vector (
ext.bootstrapComponents.modal.vector-fix.css), so the bottom-row 1.39 screenshots show no dim overlay while the top-row 1.43/Chameleon shots do — that's pre-existing skin-specific styling, unchanged by this PR.Tests
PHPUnit unit suite (
--group mediawiki-databaseless):Identical 6 failures in both runs — all in
BootstrapComponentsServiceTestandImageModalTriggerTest(manual thumbnailvariants). All pre-existing, unrelated to the modal-builder path. (+1 test count comes from splittingOutputPageParserOutputTest::testHookOutputPageParserOutputinto the two assertions that survive the inline-emission refactor.)Why a standalone fix
PR #74 ports BC to Bootstrap 5 and contains an equivalent modal-fix commit. Splitting this fix out lets BC 5.2.x ship a 5.2.3 maintenance release with just the MW 1.43+ unblocker for sites not yet ready for the BS5 migration.
The fix is BS-version-independent: under both BS4 and BS5 the modal container is
position: fixed, so inline emission works identically on either side.🤖 Generated with Claude Code