Skip to content

Commit 3d610bb

Browse files
malbertsclaude
andcommitted
Fix modal trigger not opening modals on MediaWiki 1.43+
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>
1 parent 0f82003 commit 3d610bb

8 files changed

Lines changed: 41 additions & 121 deletions

File tree

‎src/BootstrapComponents.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,6 @@
5656
*/
5757
class BootstrapComponents {
5858

59-
const EXTENSION_DATA_DEFERRED_CONTENT_KEY = 'bsc_deferredContent';
60-
6159
const EXTENSION_DATA_NO_IMAGE_MODAL = 'bsc_no_image_modal';
6260

6361

‎src/Hooks/OutputPageParserOutput.php‎

Lines changed: 4 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626

2727
namespace MediaWiki\Extension\BootstrapComponents\Hooks;
2828

29-
use MediaWiki\Extension\BootstrapComponents\BootstrapComponents;
3029
use MediaWiki\Extension\BootstrapComponents\BootstrapComponentsService;
3130
/*
3231
* TODO switch to these, wehen we drop support for mw < 1.40
@@ -39,27 +38,17 @@
3938
/**
4039
* Class OutputPageParserOutput
4140
*
42-
* Called after parse, before the HTML is added to the output.
43-
*
44-
* Method delegated to separate class to fix missing (deferred) content in
45-
* {@see \MediaWiki\Extension\BootstrapComponents\Tests\Integration\BootstrapComponentsJSONScriptTestCaseRunnerTest::assertParserOutputForCase}
41+
* Called after parse, before the HTML is added to the output. Modal markup is
42+
* now emitted inline by ModalBuilder::parse() rather than stashed via
43+
* ParserOutput::setExtensionData and re-injected here, so this hook only
44+
* handles the Vector-skin module wiring.
4645
*
4746
* @see https://www.mediawiki.org/wiki/Manual:Hooks/OutputPageParserOutput
4847
*
4948
* @since 1.2
5049
*/
5150
class OutputPageParserOutput {
5251

53-
/**
54-
* @var string
55-
*/
56-
const INJECTION_PREFIX = '<!-- injected by Extension:BootstrapComponents -->';
57-
58-
/**
59-
* @var string
60-
*/
61-
const INJECTION_SUFFIX = '<!-- /injected by Extension:BootstrapComponents -->';
62-
6352
/**
6453
* @var BootstrapComponentsService
6554
*/
@@ -94,36 +83,11 @@ public function __construct(
9483
* @return void
9584
*/
9685
public function process(): void {
97-
$deferredText = $this->getContentForLaterInjection( $this->getParserOutput() );
98-
if ( !empty( $deferredText ) ) {
99-
$this->getOutputPage()->addHTML( $deferredText );
100-
}
101-
10286
if ( $this->getBootstrapComponentsService()->vectorSkinInUse() ) {
10387
$this->getOutputPage()->addModules( [ 'ext.bootstrapComponents.vector-fix' ] );
10488
}
10589
}
10690

107-
/**
108-
* Returns the raw html that is to be inserted at the end of the page.
109-
*
110-
* @param ParserOutput $parserOutput
111-
*
112-
* @return string
113-
*/
114-
protected function getContentForLaterInjection( ParserOutput $parserOutput ): string {
115-
$deferredContent = $parserOutput
116-
->getExtensionData(BootstrapComponents::EXTENSION_DATA_DEFERRED_CONTENT_KEY );
117-
118-
if ( empty( $deferredContent ) || !is_array( $deferredContent ) ) {
119-
return '';
120-
}
121-
122-
// clearing extension data for unit and integration tests to work
123-
$parserOutput->setExtensionData( BootstrapComponents::EXTENSION_DATA_DEFERRED_CONTENT_KEY, null );
124-
return self::INJECTION_PREFIX . implode( array_values( $deferredContent ) ) . self::INJECTION_SUFFIX;
125-
}
126-
12791
protected function getBootstrapComponentsService(): BootstrapComponentsService {
12892
return $this->bootstrapComponentService;
12993
}

‎src/ModalBuilder.php‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -163,14 +163,16 @@ public function __construct( $id, $trigger, $content, $parserOutputHelper ) {
163163
/**
164164
* Parses the modal.
165165
*
166+
* Emits the trigger followed by the modal container inline. The container is
167+
* `position: fixed` and is addressed via `data-target="#id"`, so its DOM
168+
* placement is not significant. This sidesteps the parser-output-extension-data
169+
* lifecycle issue under MediaWiki 1.43+ that broke the old deferred-injection
170+
* pattern (oetterer/BootstrapComponents#68).
171+
*
166172
* @return string
167173
*/
168174
public function parse() {
169-
$this->parserOutputHelper->injectLater(
170-
$this->getId(),
171-
$this->buildModal()
172-
);
173-
return $this->buildTrigger();
175+
return $this->buildTrigger() . $this->buildModal();
174176
}
175177

176178
/**

‎src/ParserOutputHelper.php‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -138,30 +138,6 @@ public function areImageModalsSuppressed() {
138138
->getExtensionData( BootstrapComponents::EXTENSION_DATA_NO_IMAGE_MODAL );
139139
}
140140

141-
/**
142-
* Allows for html fragments for a given container id to be stored, so that it can be added to the page at a later time.
143-
*
144-
* @param string $id
145-
* @param string $rawHtml
146-
*
147-
* @return ParserOutputHelper $this (fluid)
148-
*/
149-
public function injectLater( $id, $rawHtml ) {
150-
if ( empty( $this->getParser()->getOutput() ) ) {
151-
# fix issues with PageForms file upload (issue #20)
152-
return $this;
153-
}
154-
if ( !empty( $rawHtml ) ) {
155-
$deferredContent = $this->getParser()->getOutput()->getExtensionData( BootstrapComponents::EXTENSION_DATA_DEFERRED_CONTENT_KEY );
156-
if ( empty( $deferredContent ) ) {
157-
$deferredContent = [];
158-
}
159-
$deferredContent[$id] = $rawHtml;
160-
$this->getParser()->getOutput()->setExtensionData( BootstrapComponents::EXTENSION_DATA_DEFERRED_CONTENT_KEY, $deferredContent );
161-
}
162-
return $this;
163-
}
164-
165141
/**
166142
* Formats a text as error text, so it can be added to the output.
167143
*

‎tests/phpunit/Unit/Components/ModalTest.php‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,9 @@ public function testCanConstruct() {
4949
*/
5050
public function testCanRender( $input, $arguments, $expectedTriggerOutput, $expectedModalOutput ) {
5151

52-
$modalInjection = '';
5352
$parserOutputHelper = $this->getMockBuilder( 'MediaWiki\\Extension\\BootstrapComponents\\ParserOutputHelper' )
5453
->disableOriginalConstructor()
5554
->getMock();
56-
$parserOutputHelper->expects( $this->any() )
57-
->method( 'injectLater' )
58-
->will( $this->returnCallback( function( $id, $text ) use ( &$modalInjection ) {
59-
$modalInjection .= $text;
60-
} ) );
6155
$parserOutputHelper->expects( $this->any() )
6256
->method( 'renderErrorMessage' )
6357
->will( $this->returnArgument( 0 ) );
@@ -74,11 +68,7 @@ public function testCanRender( $input, $arguments, $expectedTriggerOutput, $expe
7468
/** @noinspection PhpParamsInspection */
7569
$generatedOutput = $instance->parseComponent( $parserRequest );
7670

77-
$this->assertEquals( $expectedTriggerOutput, $generatedOutput );
78-
$this->assertEquals(
79-
$expectedModalOutput,
80-
$modalInjection
81-
);
71+
$this->assertEquals( $expectedTriggerOutput . $expectedModalOutput, $generatedOutput );
8272
}
8373

8474
/**

‎tests/phpunit/Unit/Hooks/OutputPageParserOutputTest.php‎

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -39,40 +39,46 @@ public function testCanConstruct() {
3939
);
4040
}
4141

42-
public function testHookOutputPageParserOutput() {
43-
$content = 'CONTENT';
44-
42+
public function testHookOutputPageParserOutputLoadsVectorFixUnderVector() {
4543
$outputPage = $this->createMock( OutputPage::class );
46-
$outputPage->expects( $this->once() )
47-
->method( 'addHTML' )
48-
->will( $this->returnCallback( function( $injection ) use ( &$content ) {
49-
$content .= $injection;
50-
} ) );
44+
// addHTML must NOT be called any more — modal markup is now emitted
45+
// inline by ModalBuilder::parse() rather than via a deferred-content
46+
// injection at this hook.
47+
$outputPage->expects( $this->never() )->method( 'addHTML' );
5148
$outputPage->expects( $this->once() )
5249
->method( 'addModules' )
5350
->with(
5451
$this->equalTo( [ 'ext.bootstrapComponents.vector-fix' ] )
5552
);
5653

57-
$observerParserOutput = $this->createMock( ParserOutput::class );
58-
$observerParserOutput->expects( $this->exactly( 1 ) )
59-
->method( 'getExtensionData' )
60-
->with(
61-
$this->stringContains( 'bsc_deferredContent' )
62-
)
63-
->willReturn( [ 'test' ] );
64-
6554
$bootstrapService = $this->createMock( BootstrapComponentsService::class );
6655
$bootstrapService->expects( $this->once() )
6756
->method( 'vectorSkinInUse' )
6857
->willReturn( true );
6958

70-
$instance = new OutputPageParserOutput( $outputPage, $observerParserOutput, $bootstrapService );
59+
$instance = new OutputPageParserOutput(
60+
$outputPage,
61+
$this->createMock( ParserOutput::class ),
62+
$bootstrapService
63+
);
7164
$instance->process();
65+
}
7266

73-
$this->assertEquals(
74-
'CONTENT<!-- injected by Extension:BootstrapComponents -->test<!-- /injected by Extension:BootstrapComponents -->',
75-
$content
67+
public function testHookDoesNothingWhenNotVector() {
68+
$outputPage = $this->createMock( OutputPage::class );
69+
$outputPage->expects( $this->never() )->method( 'addHTML' );
70+
$outputPage->expects( $this->never() )->method( 'addModules' );
71+
72+
$bootstrapService = $this->createMock( BootstrapComponentsService::class );
73+
$bootstrapService->expects( $this->once() )
74+
->method( 'vectorSkinInUse' )
75+
->willReturn( false );
76+
77+
$instance = new OutputPageParserOutput(
78+
$outputPage,
79+
$this->createMock( ParserOutput::class ),
80+
$bootstrapService
7681
);
82+
$instance->process();
7783
}
7884
}

‎tests/phpunit/Unit/ImageModalTest.php‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -254,13 +254,7 @@ function( $component ) {
254254
}
255255
) );
256256

257-
$modalInjection = '';
258257
$parserOutputHelper = $this->createMock( ParserOutputHelper::class );
259-
$parserOutputHelper->expects( $this->any() )
260-
->method( 'injectLater' )
261-
->will( $this->returnCallback( function( $id, $text ) use ( &$modalInjection ) {
262-
$modalInjection .= $text;
263-
} ) );
264258

265259
$instance = $this->createImageModalWithMocks( null, $title, $file, $nestingController, null, $parserOutputHelper );
266260
$time = false;
@@ -283,9 +277,9 @@ function( $component ) {
283277
. '-- ' . ($resultOfParseCall ?: $res)
284278
);
285279
}
286-
$this->assertEquals(
280+
$this->assertStringContainsString(
287281
$expectedModal,
288-
$modalInjection,
282+
$resultOfParseCall ?: $res,
289283
'failed modal with test data:' . $this->generatePhpCodeForManualProviderDataOneCase( $fp, $hp )
290284
);
291285
}

‎tests/phpunit/Unit/ModalBuilderTest.php‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,9 @@ public function testCanConstruct() {
4949
*/
5050
public function testCanParse( $id, $trigger, $content, $header, $footer, $outerClass, $outerStyle, $innerClass, $expectedTrigger, $expectedModal ) {
5151

52-
$modalInjection = '';
5352
$parserOutputHelper = $this->getMockBuilder( 'MediaWiki\\Extension\\BootstrapComponents\\ParserOutputHelper' )
5453
->disableOriginalConstructor()
5554
->getMock();
56-
$parserOutputHelper->expects( $this->any() )
57-
->method( 'injectLater' )
58-
->will( $this->returnCallback( function( $id, $text ) use ( &$modalInjection ) {
59-
$modalInjection .= $text;
60-
} ) );
6155

6256
/** @noinspection PhpParamsInspection */
6357
$instance = new ModalBuilder( $id, $trigger, $content, $parserOutputHelper );
@@ -77,13 +71,9 @@ public function testCanParse( $id, $trigger, $content, $header, $footer, $outerC
7771
$instance->setDialogClass( $innerClass );
7872
}
7973
$this->assertEquals(
80-
$expectedTrigger,
74+
$expectedTrigger . $expectedModal,
8175
$instance->parse()
8276
);
83-
$this->assertEquals(
84-
$expectedModal,
85-
$modalInjection
86-
);
8777
}
8878

8979
/**

0 commit comments

Comments
 (0)