Fix modal trigger not opening modals on MediaWiki 1.43+#75
Draft
malberts wants to merge 1 commit into
Draft
Conversation
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>
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.
Fixes #68.
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.phpNet: +41 / −121 lines. No JS, CSS, or extension.json changes — purely the deferred-content rewrite.
Verification
Tested on a fresh MediaWiki 1.39 + Vector + BC stack (LTS, oldest supported floor):
Modal opens cleanly on MW 1.39 with the patch — no regression on the supported floor where the original bug does not manifest.
The fix was previously verified on MW 1.43 during development of PR #74, where the issue reproduces and the patch resolves it.
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