From 8983c9d862959402e7818c779c8e1a3a957abae1 Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Thu, 15 Jun 2023 10:08:30 +1000 Subject: diff: Move SlotDiffRenderer::getTablePrefix() parts assembly up to DifferenceEngine getTablePrefix() is used to show the inline legend and inline switcher. It is not yet part of a released stable interface. Theoretically there may be multiple text slots on a page, and we don't want multiple inline legends. There was already a fragment assembly system, for the benefit of hook handlers, so move that up to the page level, so that it can also deduplicate prefix fragments coming from each slot. Add tests. Bug: T324759 Change-Id: I9baa5c24128c63bc318ba13e83a024843f4ab15e --- includes/diff/DifferenceEngine.php | 27 ++++++++-- .../Hook/TextSlotDiffRendererTablePrefixHook.php | 2 +- includes/diff/SlotDiffRenderer.php | 8 +-- includes/diff/TextSlotDiffRenderer.php | 13 +---- .../includes/diff/TextSlotDiffRendererTest.php | 57 ++++++++++++++++++++++ 5 files changed, 87 insertions(+), 20 deletions(-) diff --git a/includes/diff/DifferenceEngine.php b/includes/diff/DifferenceEngine.php index bcb0d400f059..265d3aff94fe 100644 --- a/includes/diff/DifferenceEngine.php +++ b/includes/diff/DifferenceEngine.php @@ -934,11 +934,7 @@ class DifferenceEngine extends ContextSource { $out->enableOOUI(); } - // Add table prefixes. - foreach ( $this->getSlotDiffRenderers() as $slotDiffRenderer ) { - $out->addHTML( $slotDiffRenderer->getTablePrefix( $this->getContext(), $this->mNewPage ) ); - } - + $this->showTablePrefixes(); $this->showDiff( $oldHeader, $newHeader, $notice ); if ( !$diffOnly ) { $this->renderNewRevision(); @@ -946,6 +942,27 @@ class DifferenceEngine extends ContextSource { } } + /** + * Add table prefixes + */ + private function showTablePrefixes() { + $parts = []; + foreach ( $this->getSlotDiffRenderers() as $slotDiffRenderer ) { + $parts += $slotDiffRenderer->getTablePrefix( $this->getContext(), $this->mNewPage ); + } + ksort( $parts ); + if ( count( array_filter( $parts ) ) > 0 ) { + $language = $this->getLanguage(); + $attrs = [ + 'class' => 'mw-diff-table-prefix', + 'dir' => $language->getDir(), + 'lang' => $language->getCode(), + ]; + $this->getOutput()->addHTML( + Html::rawElement( 'div', $attrs, implode( '', $parts ) ) ); + } + } + /** * Build a link to mark a change as patrolled. * diff --git a/includes/diff/Hook/TextSlotDiffRendererTablePrefixHook.php b/includes/diff/Hook/TextSlotDiffRendererTablePrefixHook.php index 15d20a43c3c6..0016a49a488d 100644 --- a/includes/diff/Hook/TextSlotDiffRendererTablePrefixHook.php +++ b/includes/diff/Hook/TextSlotDiffRendererTablePrefixHook.php @@ -21,7 +21,7 @@ interface TextSlotDiffRendererTablePrefixHook { * * @param TextSlotDiffRenderer $textSlotDiffRenderer * @param IContextSource $context - * @param mixed[] &$parts HTML strings to add to a container above the diff table. + * @param (string|null)[] &$parts HTML strings to add to a container above the diff table. * Will be sorted by key before being output. * @return bool|void True or no return value to continue or false to abort */ diff --git a/includes/diff/SlotDiffRenderer.php b/includes/diff/SlotDiffRenderer.php index 63e6b6a8a44b..f20d1f452a28 100644 --- a/includes/diff/SlotDiffRenderer.php +++ b/includes/diff/SlotDiffRenderer.php @@ -53,12 +53,14 @@ abstract class SlotDiffRenderer { /** * Get the content to add above the main diff table. * + * @since 1.41 * @param IContextSource $context * @param Title $newTitle - * @return string The full HTML for the prefix area, with its contents. + * @return (string|null)[] An array of HTML fragments to assemble into the prefix + * area. They will be deduplicated and sorted by key. */ - public function getTablePrefix( IContextSource $context, Title $newTitle ): string { - return ''; + public function getTablePrefix( IContextSource $context, Title $newTitle ): array { + return []; } /** diff --git a/includes/diff/TextSlotDiffRenderer.php b/includes/diff/TextSlotDiffRenderer.php index 1a397f1a2697..fa93acb1bbf9 100644 --- a/includes/diff/TextSlotDiffRenderer.php +++ b/includes/diff/TextSlotDiffRenderer.php @@ -178,7 +178,7 @@ class TextSlotDiffRenderer extends SlotDiffRenderer { /** * @inheritDoc */ - public function getTablePrefix( IContextSource $context, Title $newTitle ): string { + public function getTablePrefix( IContextSource $context, Title $newTitle ): array { $legend = null; $inlineSwitcher = null; @@ -233,16 +233,7 @@ class TextSlotDiffRenderer extends SlotDiffRenderer { if ( count( $parts ) > 1 && $parts[self::INLINE_LEGEND_KEY] === null ) { $parts[self::INLINE_LEGEND_KEY] = Html::element( 'div' ); } - ksort( $parts ); - if ( count( array_filter( $parts ) ) > 0 ) { - $attrs = [ - 'class' => 'mw-diff-table-prefix', - 'dir' => $this->language->getDir(), - 'lang' => $this->language->getCode(), - ]; - return Html::rawElement( 'div', $attrs, implode( '', $parts ) ); - } - return ''; + return $parts; } /** diff --git a/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php b/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php index 62c58c416239..fc467af2024c 100644 --- a/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php +++ b/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php @@ -1,5 +1,6 @@ [ + TextSlotDiffRenderer::ENGINE_PHP, + [ + TextSlotDiffRenderer::INLINE_LEGEND_KEY => '
', + TextSlotDiffRenderer::INLINE_SWITCHER_KEY => null + ] + ], + 'wikidiff2' => [ + TextSlotDiffRenderer::ENGINE_WIKIDIFF2, + [ + TextSlotDiffRenderer::INLINE_LEGEND_KEY => + 'class="mw-diff-inline-legend mw-diff-element-hidden"', + TextSlotDiffRenderer::INLINE_SWITCHER_KEY => 'mw-diffPage-inlineToggle-container' + ] + ], + 'inline' => [ + TextSlotDiffRenderer::ENGINE_WIKIDIFF2_INLINE, + [ + TextSlotDiffRenderer::INLINE_LEGEND_KEY => + 'class="mw-diff-inline-legend".*\(diff-inline-tooltip-ins\)', + TextSlotDiffRenderer::INLINE_SWITCHER_KEY => 'mw-diffPage-inlineToggle-container' + ] + ] + ]; + } + + /** + * @dataProvider provideGetTablePrefix + * @param string $engine + * @param string[] $expectedPatterns + */ + public function testGetTablePrefix( $engine, $expectedPatterns ) { + OOUI\Theme::setSingleton( new OOUI\BlankTheme() ); + $this->overrideConfigValue( MainConfigNames::ShowDiffToggleSwitch, true ); + + $slotDiffRenderer = $this->getTextSlotDiffRenderer(); + $slotDiffRenderer->setHookContainer( $this->createHookContainer() ); + $slotDiffRenderer->setEngine( $engine ); + + $context = new RequestContext; + $context->setLanguage( 'qqx' ); + + $title = $this->getServiceContainer()->getTitleFactory()->newFromText( 'Test' ); + $result = $slotDiffRenderer->getTablePrefix( $context, $title ); + $this->assertSameSize( $expectedPatterns, $result ); + foreach ( $expectedPatterns as $key => $pattern ) { + if ( $pattern === null ) { + $this->assertNull( $result[$key], "\$result[$key]" ); + } else { + $this->assertMatchesRegularExpression( + "#$pattern#", $result[$key], "\$result[$key]" ); + } + } + } } -- cgit v1.2.3