diff: Add legend and tooltips to inline diff display

Add a legend at the top of the inline diff display, showing the
meanings of the colours of the inserted and deleted highlighting.
Also add the same text as tooltips on the highlighted elements.

The legend is added as part of a new area above the diff table
that can be modified via a new TextSlotDiffRendererTablePrefix
hook, so that extensions can add other buttons etc. there as
required.

This is a follow-up to the previous attempt, which added the
legend in DifferenceEngine::showDiff() and was called from
too many places. This patch moves it to be called in
DifferenceEngine::showDiffPage().

Bug: T324759
Change-Id: I2a3c67bcfa47313dee597e602a62073e4e298cd2
Follow-up: I6de30bf79eb5ac262285951792782b870d075e00
This commit is contained in:
Sam Wilson 2023-05-22 13:41:21 +08:00 committed by Tim Starling
parent 6a3e33f330
commit 1eb586013c
11 changed files with 193 additions and 6 deletions

View file

@ -136,6 +136,13 @@ For notes on 1.39.x and older releases, see HISTORY.
MW_SKIP_EXTERNAL_DEPENDENCIES=1 environment flag when running PHPUnit.
* ManualLogEntry::setForceBotFlag() has been added to allow the forcing of the
bot flag for log entries which are inserted to the recent changes.
* A new hook, TextSlotDiffRendererTablePrefixHook, has been added to allow
extensions to add content within #mw-content-text but after the
DifferenceEngineViewHeader or DifferenceEngineShowDiffPage hooks have been
run. The new hook is used to add elements within a horizontal display area,
where their order can be explicitly set. Examples of uses for this include
adding the VisualEditor diff-type switch, and the legend for inline diffs that
is displayed if Wikidiff2 is installed.
=== External library changes in 1.40 ===

View file

@ -1054,6 +1054,7 @@ $wgAutoloadLocalClasses = [
'MediaWiki\\Diff\\Hook\\DifferenceEngineShowEmptyOldContentHook' => __DIR__ . '/includes/diff/Hook/DifferenceEngineShowEmptyOldContentHook.php',
'MediaWiki\\Diff\\Hook\\DifferenceEngineViewHeaderHook' => __DIR__ . '/includes/diff/Hook/DifferenceEngineViewHeaderHook.php',
'MediaWiki\\Diff\\Hook\\NewDifferenceEngineHook' => __DIR__ . '/includes/diff/Hook/NewDifferenceEngineHook.php',
'MediaWiki\\Diff\\Hook\\TextSlotDiffRendererTablePrefixHook' => __DIR__ . '/includes/diff/Hook/TextSlotDiffRendererTablePrefixHook.php',
'MediaWiki\\Diff\\WordAccumulator' => __DIR__ . '/includes/diff/WordAccumulator.php',
'MediaWiki\\EditPage\\Constraint\\AccidentalRecreationConstraint' => __DIR__ . '/includes/editpage/Constraint/AccidentalRecreationConstraint.php',
'MediaWiki\\EditPage\\Constraint\\AutoSummaryMissingSummaryConstraint' => __DIR__ . '/includes/editpage/Constraint/AutoSummaryMissingSummaryConstraint.php',

View file

@ -83,6 +83,7 @@ class HookRunner implements
\MediaWiki\Diff\Hook\AbortDiffCacheHook,
\MediaWiki\Diff\Hook\ArticleContentOnDiffHook,
\MediaWiki\Diff\Hook\DifferenceEngineAfterLoadNewTextHook,
\MediaWiki\Diff\Hook\TextSlotDiffRendererTablePrefixHook,
\MediaWiki\Diff\Hook\DifferenceEngineLoadTextAfterNewContentIsLoadedHook,
\MediaWiki\Diff\Hook\DifferenceEngineMarkPatrolledLinkHook,
\MediaWiki\Diff\Hook\DifferenceEngineMarkPatrolledRCIDHook,
@ -1322,6 +1323,17 @@ class HookRunner implements
);
}
public function onTextSlotDiffRendererTablePrefix(
\TextSlotDiffRenderer $textSlotDiffRenderer,
IContextSource $context,
array &$parts
) {
return $this->container->run(
'TextSlotDiffRendererTablePrefix',
[ $textSlotDiffRenderer, $context, &$parts ]
);
}
public function onDifferenceEngineLoadTextAfterNewContentIsLoaded(
$differenceEngine
) {

View file

@ -679,6 +679,8 @@ abstract class ContentHandler {
$slotDiffRenderer->setStatsdDataFactory( $statsdDataFactory );
// XXX using the page language would be better, but it's unclear how that should be injected
$slotDiffRenderer->setLanguage( $contentLanguage );
$slotDiffRenderer->setHookContainer( MediaWikiServices::getInstance()->getHookContainer() );
$slotDiffRenderer->setContentModel( $this->getModelID() );
$inline = ( $options['diff-type'] ?? '' ) === 'inline';
$engine = DifferenceEngine::getEngine();

View file

@ -919,6 +919,11 @@ class DifferenceEngine extends ContextSource {
$notice .= Html::warningBox( $msg->parse() );
}
// Add table prefixes.
foreach ( $this->getSlotDiffRenderers() as $slotDiffRenderer ) {
$out->addHTML( $slotDiffRenderer->getTablePrefix( $this->getContext() ) );
}
$this->showDiff( $oldHeader, $newHeader, $notice );
if ( !$diffOnly ) {
$this->renderNewRevision();
@ -1615,13 +1620,14 @@ class DifferenceEngine extends ContextSource {
}
/**
* Add title attributes for tooltips on moved paragraph indicators
* Add title attributes for tooltips on various diff elements
*
* @param string $text
* @return string
*/
private function addLocalisedTitleTooltips( $text ) {
return preg_replace_callback(
// Moved paragraph indicators.
$text = preg_replace_callback(
'/class="mw-diff-movedpara-(left|right)"/',
function ( array $matches ) {
$key = $matches[1] === 'right' ?
@ -1631,6 +1637,20 @@ class DifferenceEngine extends ContextSource {
},
$text
);
// For inline diffs, add tooltips to `<ins>` and `<del>`.
if ( isset( $this->slotDiffOptions['diff-type'] ) && $this->slotDiffOptions['diff-type'] == 'inline' ) {
$text = str_replace(
[ '<ins>', '<del>' ],
[
Html::openElement( 'ins', [ 'title' => $this->msg( 'diff-inline-tooltip-ins' )->plain() ] ),
Html::openElement( 'del', [ 'title' => $this->msg( 'diff-inline-tooltip-del' )->plain() ] ),
],
$text
);
}
return $text;
}
/**

View file

@ -0,0 +1,33 @@
<?php
namespace MediaWiki\Diff\Hook;
use IContextSource;
use TextSlotDiffRenderer;
/**
* This is a hook handler interface, see docs/Hooks.md.
* Use the hook name "TextSlotDiffRendererTablePrefix" to register handlers implementing this interface.
*
* @stable to implement
* @ingroup Hooks
*/
interface TextSlotDiffRendererTablePrefixHook {
/**
* Use this hook to change the HTML that is included in a prefix container directly before the diff table.
*
* @since 1.41
*
* @param TextSlotDiffRenderer $textSlotDiffRenderer
* @param IContextSource $context
* @param mixed[] &$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
*/
public function onTextSlotDiffRendererTablePrefix(
TextSlotDiffRenderer $textSlotDiffRenderer,
IContextSource $context,
array &$parts
);
}

View file

@ -49,6 +49,16 @@ abstract class SlotDiffRenderer {
*/
abstract public function getDiff( Content $oldContent = null, Content $newContent = null );
/**
* Get the content to add above the main diff table.
*
* @param IContextSource $context
* @return string The full HTML for the prefix area, with its contents.
*/
public function getTablePrefix( IContextSource $context ): string {
return '';
}
/**
* Add modules needed for correct styling/behavior of the diff.
* @stable to override

View file

@ -21,6 +21,9 @@
* @ingroup DifferenceEngine
*/
use MediaWiki\HookContainer\HookContainer;
use MediaWiki\HookContainer\HookRunner;
use MediaWiki\Html\Html;
use MediaWiki\MediaWikiServices;
use MediaWiki\Shell\Shell;
use Wikimedia\Assert\Assert;
@ -49,18 +52,26 @@ class TextSlotDiffRenderer extends SlotDiffRenderer {
/** Use an external executable. */
public const ENGINE_EXTERNAL = 'external';
public const INLINE_LEGEND_KEY = '10_mw-diff-inline-legend';
/** @var IBufferingStatsdDataFactory|null */
private $statsdDataFactory;
/** @var Language|null The language this content is in. */
private $language;
/** @var HookRunner|null */
private $hookRunner;
/** @var string One of the ENGINE_* constants. */
private $engine = self::ENGINE_PHP;
/** @var string|null Path to an executable to be used as the diff engine. */
private $externalEngine;
/** @var string */
private $contentModel;
/** @inheritDoc */
public function getExtraCacheKeys() {
// Tell DifferenceEngine this is a different variant from the standard wikidiff2 variant
@ -99,6 +110,22 @@ class TextSlotDiffRenderer extends SlotDiffRenderer {
$this->language = $language;
}
/**
* @since 1.41
* @param HookContainer $hookContainer
*/
public function setHookContainer( HookContainer $hookContainer ): void {
$this->hookRunner = new HookRunner( $hookContainer );
}
/**
* @param string $contentModel
* @since 1.41
*/
public function setContentModel( string $contentModel ) {
$this->contentModel = $contentModel;
}
/**
* Set which diff engine to use.
* @param string $type One of the ENGINE_* constants.
@ -120,6 +147,16 @@ class TextSlotDiffRenderer extends SlotDiffRenderer {
$this->externalEngine = $executable;
}
/**
* Get the content model ID that this renderer acts on
*
* @since 1.41
* @return string
*/
public function getContentModel(): string {
return $this->contentModel;
}
/** @inheritDoc */
public function getDiff( Content $oldContent = null, Content $newContent = null ) {
$this->normalizeContents( $oldContent, $newContent, TextContent::class );
@ -130,6 +167,41 @@ class TextSlotDiffRenderer extends SlotDiffRenderer {
return $this->getTextDiff( $oldText, $newText );
}
/**
* @inheritDoc
*/
public function getTablePrefix( IContextSource $context ): string {
$legend = null;
// wikidiff2 inline type gets a legend to explain the highlighting colours.
if ( $this->engine === self::ENGINE_WIKIDIFF2_INLINE ) {
$ins = Html::element(
'span', [ 'class' => 'mw-diff-inline-legend-ins' ], $context->msg( 'diff-inline-tooltip-ins' )->plain()
);
$del = Html::element(
'span', [ 'class' => 'mw-diff-inline-legend-del' ], $context->msg( 'diff-inline-tooltip-del' )->plain()
);
$legend = Html::rawElement( 'div', [ 'class' => 'mw-diff-inline-legend' ], "$ins $del" );
}
// Allow extensions to add other parts to this area (or modify the legend).
// An empty placeholder for the legend is added when it's not in use and other items have been added.
$parts = [ self::INLINE_LEGEND_KEY => $legend ];
$this->hookRunner->onTextSlotDiffRendererTablePrefix( $this, $context, $parts );
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 '';
}
/**
* Diff the text representations of two content objects (or just two pieces of text in general).
* @param string $oldText

View file

@ -1014,6 +1014,8 @@
"diff-multi-manyusers": "({{PLURAL:$1|One intermediate revision|$1 intermediate revisions}} by more than $2 {{PLURAL:$2|user|users}} not shown)",
"diff-paragraph-moved-tonew": "Paragraph was moved. Click to jump to new location.",
"diff-paragraph-moved-toold": "Paragraph was moved. Click to jump to old location.",
"diff-inline-tooltip-ins": "Content added",
"diff-inline-tooltip-del": "Content deleted",
"difference-missing-revision": "{{PLURAL:$2|One revision|$2 revisions}} of this difference ($1) {{PLURAL:$2|was|were}} not found.\n\nThis is usually caused by following an outdated diff link to a page that has been deleted.\nDetails can be found in the [{{fullurl:{{#Special:Log}}/delete|page={{FULLPAGENAMEE}}}} deletion log].",
"difference-bad-old-revision": "The content of the old revision is missing or corrupted.",
"difference-bad-new-revision": "The content of the new revision is missing or corrupted.",

View file

@ -1265,6 +1265,8 @@
"diff-multi-manyusers": "This message appears in the revision history of a page when comparing two versions which aren't consecutive, and the intermediate revisions have been edited by more than 100 users.\n\nParameters:\n* $1 - the number of revisions, will always be 101 or more\n* $2 - the number of users that were found, which was limited at 100\n{{Related|Diff-multi}}",
"diff-paragraph-moved-tonew": "Explaining title tag for the indicating symbols when a paragraph was moved hinting to the new location in the Diff view.",
"diff-paragraph-moved-toold": "Explaining title tag for the indicating symbols when a paragraph was moved hinting to the old location in the Diff view.",
"diff-inline-tooltip-ins": "Title for inserted content in a diff, shown as a tooltip on the content and also in a legend at the top of the diff.",
"diff-inline-tooltip-del": "Title for deleted content in a diff, shown as a tooltip on the content and also in a legend at the top of the diff.",
"difference-missing-revision": "Text displayed when the requested revision does not exist using a diff link.\n\nExample: [{{canonicalurl:Project:News|diff=426850&oldid=99999999}} Diff with invalid revision#]\n\nParameters:\n* $1 - the list of missing revision IDs\n* $2 - the number of items in $1 (one or two)",
"difference-bad-old-revision": "Text displayed when the old (left-hand side) revision of a diff cannot be loaded due to database corruption.",
"difference-bad-new-revision": "Text displayed when the new (right-hand side) revision of a diff cannot be loaded due to database corruption.",

View file

@ -2,6 +2,9 @@
* Diff rendering
*/
@diffInserted: #a3d3ff;
@diffDeleted: #ffe49c;
/* stylelint-disable selector-class-pattern */
.diff {
@ -74,12 +77,35 @@
.mw-diff-inline-added ins,
.mw-diff-inline-changed ins {
background: #a3d3ff;
background: @diffInserted;
}
.mw-diff-inline-deleted del,
.mw-diff-inline-changed del {
background: #ffe49c;
background: @diffDeleted;
}
.mw-diff-table-prefix {
display: flex;
justify-content: space-between;
align-items: center;
margin: 1em 0;
.mw-diff-inline-legend {
.mw-diff-inline-legend-ins,
.mw-diff-inline-legend-del {
display: inline-block;
padding: 4px 6px;
}
.mw-diff-inline-legend-ins {
background: @diffInserted;
}
.mw-diff-inline-legend-del {
background: @diffDeleted;
}
}
}
.diff-addedline,
@ -125,11 +151,11 @@
}
.diff-addedline {
border-color: #a3d3ff;
border-color: @diffInserted;
}
.diff-deletedline {
border-color: #ffe49c;
border-color: @diffDeleted;
}
.diffchange {