Inline diffs upstreamed from MobileFrontend to core.

A new inline mode is provided for diffs.
It is only available when wikidiff2 is installed.
There is no PHP implementation (one can be added later if
necessary)

For now, it is accessed by passing diff-type as a query string parameter
e.g ?diff-type=inline
A control for switching between the two is added as a follow up.
see Ie9bb17789d90b7492559782021937f3f3e4356f8

* The final method getSlotDiffRenderer now takes a second parameter
options
* The method `getSlotDiffRendererInternal` is deprecated and
replaced with the more flexible `getSlotDiffRendererWithOptions`
This has potential to be a breaking change but is unlikely to impact
any existing clients.

Note: PHP implementation can be added later if necessary

Bug: T117279
Change-Id: I4f81c8ccf253dd4aa6cf43c3fad257b4b0dd1ebd

RELEASE-NOTES-1.35

@@ -150,8 +150,10 @@ because of Phabricator reports.
   createAssocArgs, armorLinks, makeKnownLinkHolder, getImageParams,
   parseLinkParameter, stripAltText, replaceLinkHoldersText.
 * MediaWikiTestCase::prepareServices(), deprecated in 1.32, has been removed
+* The method ContentHandler::getSlotDiffRendererInternal is replaced with
+  ContentHandler::getSlotDiffRendererWithOptions. This breaks consumers which
+  call parent::getSlotDiffRendererInternal (no instances of which are known).
 * …
-
 === Deprecations in 1.35 ===
 * The PHPUnit4And6Compat class, used to provide compatibility with PHPUnit 4, is
   now deprecated. MediaWiki support for PHPUnit 4 ended with the removal of HHVM

includes/content/ContentHandler.php

@@ -615,10 +615,11 @@ abstract class ContentHandler {
 	 * Get an appropriate SlotDiffRenderer for this content model.
 	 * @since 1.32
 	 * @param IContextSource $context
+	 * @param array $options of the slot diff renderer (optional)
 	 * @return SlotDiffRenderer
 	 */
-	final public function getSlotDiffRenderer( IContextSource $context ) {
-		$slotDiffRenderer = $this->getSlotDiffRendererInternal( $context );
+	final public function getSlotDiffRenderer( IContextSource $context, array $options = [] ) {
+		$slotDiffRenderer = $this->getSlotDiffRendererWithOptions( $context, $options );
 		if ( get_class( $slotDiffRenderer ) === TextSlotDiffRenderer::class ) {
 			//  To keep B/C, when SlotDiffRenderer is not overridden for a given content type
 			// but DifferenceEngine is, use that instead.
@@ -639,10 +640,28 @@ abstract class ContentHandler {
 
 	/**
 	 * Return the SlotDiffRenderer appropriate for this content handler.
+	 * @deprecated use getSlotDiffRendererWithOptions instead
 	 * @param IContextSource $context
-	 * @return SlotDiffRenderer
+	 * @return SlotDiffRenderer|null
 	 */
 	protected function getSlotDiffRendererInternal( IContextSource $context ) {
+		return null;
+	}
+
+	/**
+	 * Return the SlotDiffRenderer appropriate for this content handler.
+	 * @param IContextSource $context
+	 * @param array $options
+	 * @return SlotDiffRenderer
+	 */
+	protected function getSlotDiffRendererWithOptions( IContextSource $context, $options = [] ) {
+		$internalRenderer = $this->getSlotDiffRendererInternal( $context );
+		// `getSlotDiffRendererInternal` has been overriden by a class using the deprecated method.
+		// Options will not work so exit early!
+		if ( $internalRenderer !== null ) {
+			return $internalRenderer;
+		}
+
 		$contentLanguage = MediaWikiServices::getInstance()->getContentLanguage();
 		$statsdDataFactory = MediaWikiServices::getInstance()->getStatsdDataFactory();
 		$slotDiffRenderer = new TextSlotDiffRenderer();
@@ -650,11 +669,18 @@ abstract class ContentHandler {
 		// XXX using the page language would be better, but it's unclear how that should be injected
 		$slotDiffRenderer->setLanguage( $contentLanguage );
 
+		$inline = ( $options['diff-type'] ?? '' ) === 'inline';
+		$engine = 'wikidiff2';
 		$engine = DifferenceEngine::getEngine();
+
 		if ( $engine === 'php' ) {
 			$slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_PHP );
 		} elseif ( $engine === 'wikidiff2' ) {
-			$slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_WIKIDIFF2 );
+			if ( $inline ) {
+				$slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_WIKIDIFF2_INLINE );
+			} else {
+				$slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_WIKIDIFF2 );
+			}
 		} else {
 			$slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_EXTERNAL, $engine );
 		}

includes/diff/DifferenceEngine.php

@@ -197,6 +197,11 @@ class DifferenceEngine extends ContextSource {
 	 */
 	protected $isSlotDiffRenderer = false;
 
+	/* A set of options that will be passed to the SlotDiffRenderer upon creation
+	 * @var array
+	 */
+	private $slotDiffOptions = [];
+
 	/** #@- */
 
 	/**
@@ -252,7 +257,12 @@ class DifferenceEngine extends ContextSource {
 			$this->slotDiffRenderers = array_map( function ( $contents ) {
 				/** @var Content $content */
 				$content = $contents['new'] ?: $contents['old'];
-				return $content->getContentHandler()->getSlotDiffRenderer( $this->getContext() );
+				$context = $this->getContext();
+
+				return $content->getContentHandler()->getSlotDiffRenderer(
+					$context,
+					$this->slotDiffOptions
+				);
 			}, $slotContents );
 		}
 		return $this->slotDiffRenderers;
@@ -568,7 +578,7 @@ class DifferenceEngine extends ContextSource {
 
 		$rollback = '';
 
-		$query = [];
+		$query = $this->slotDiffOptions;
 		# Carry over 'diffonly' param via navigation links
 		if ( $diffOnly != $user->getBoolOption( 'diffonly' ) ) {
 			$query['diffonly'] = $diffOnly;
@@ -1288,6 +1298,13 @@ class DifferenceEngine extends ContextSource {
 		return $params;
 	}
 
+	/**
+	 * @param array $options for the difference engine - accepts keys 'diff-type'
+	 */
+	public function setSlotDiffOptions( $options ) {
+		$this->slotDiffOptions = $options;
+	}
+
 	/**
 	 * Generate a diff, no caching.
 	 *

includes/diff/TextSlotDiffRenderer.php

@@ -42,6 +42,9 @@ class TextSlotDiffRenderer extends SlotDiffRenderer {
 	/** Use the wikidiff2 PHP module. */
 	const ENGINE_WIKIDIFF2 = 'wikidiff2';
 
+	/** Use the wikidiff2 PHP module. */
+	const ENGINE_WIKIDIFF2_INLINE = 'wikidiff2inline';
+
 	/** Use an external executable. */
 	const ENGINE_EXTERNAL = 'external';
 
@@ -57,6 +60,17 @@ class TextSlotDiffRenderer extends SlotDiffRenderer {
 	/** @var string Path to an executable to be used as the diff engine. */
 	private $externalEngine;
 
+	/**
+	 * @inheritDoc
+	 * @return array
+	 */
+	public function getExtraCacheKeys() {
+		// Tell DifferenceEngine this is a different variant from the standard wikidiff2 variant
+		return $this->engine === self::ENGINE_WIKIDIFF2_INLINE ? [
+			phpversion( 'wikidiff2' ), 'inline'
+		] : [];
+	}
+
 	/**
 	 * Convenience helper to use getTextDiff without an instance.
 	 * @param string $oldText
@@ -85,7 +99,8 @@ class TextSlotDiffRenderer extends SlotDiffRenderer {
 	 * @param string|null $executable Path to an external exectable, only when type is ENGINE_EXTERNAL.
 	 */
 	public function setEngine( $type, $executable = null ) {
-		$engines = [ self::ENGINE_PHP, self::ENGINE_WIKIDIFF2, self::ENGINE_EXTERNAL ];
+		$engines = [ self::ENGINE_PHP, self::ENGINE_WIKIDIFF2, self::ENGINE_EXTERNAL,
+			self::ENGINE_WIKIDIFF2_INLINE ];
 		Assert::parameter( in_array( $type, $engines, true ), '$type',
 			'must be one of the TextSlotDiffRenderer::ENGINE_* constants' );
 		if ( $type === self::ENGINE_EXTERNAL ) {
@@ -248,6 +263,12 @@ class TextSlotDiffRenderer extends SlotDiffRenderer {
 			}
 
 			return $difftext;
+		} elseif ( $this->engine === self::ENGINE_WIKIDIFF2_INLINE ) {
+			// Note wikidiff2_inline_diff returns an element sans table.
+			// Due to the way other diffs work (return a table with before and after), we need to wrap
+			// the output in a row that spans the 4 columns that are expected, so that our diff appears in
+			// the correct place!
+			return '<tr><td colspan="4">' . wikidiff2_inline_diff( $oldText, $newText, 2 ) . '</td></tr>';
 		}
 		throw new LogicException( 'Invalid engine: ' . $this->engine );
 	}

includes/page/Article.php

@@ -948,6 +948,9 @@ class Article implements Page {
 			$purge,
 			$unhide
 		);
+		$de->setSlotDiffOptions( [
+			'diff-type' => $request->getVal( 'diff-type' )
+		] );
 
 		// DifferenceEngine directly fetched the revision:
 		$this->mRevIdFetched = $de->getNewid();

resources/src/mediawiki.diff.styles/diff.css

@@ -57,6 +57,14 @@ td.diff-marker {
 	line-height: 1.2;
 }
 
+.mw-diff-inline-deleted del,
+.mw-diff-inline-added ins,
+.mw-diff-inline-changed ins,
+.mw-diff-inline-changed del {
+	display: inline-block;
+	text-decoration: none;
+}
+
 .diff-addedline,
 .diff-deletedline,
 .diff-context {
@@ -70,10 +78,20 @@ td.diff-marker {
 	border-radius: 0.33em;
 }
 
+.mw-diff-inline-added ins,
+.mw-diff-inline-changed ins {
+	background: #a3d3ff;
+}
+
 .diff-addedline {
 	border-color: #a3d3ff;
 }
 
+.mw-diff-inline-deleted del,
+.mw-diff-inline-changed del {
+	background: #ffe49c;
+}
+
 .diff-deletedline {
 	border-color: #ffe49c;
 }
@@ -168,3 +186,10 @@ td.diff-marker {
 .rtl .mw-diff-movedpara-left:after {
 	content: '↩';
 }
+
+#mw-inlinediff-header #mw-diff-otitle1,
+#mw-inlinediff-header #mw-diff-otitle2,
+#mw-inlinediff-header #mw-diff-otitle3,
+#mw-inlinediff-header #mw-diff-otitle5 {
+	display: none;
+}

tests/phpunit/includes/diff/TextSlotDiffRendererTest.php

@@ -8,6 +8,15 @@ use Wikimedia\Assert\ParameterTypeException;
  */
 class TextSlotDiffRendererTest extends MediaWikiTestCase {
 
+	public function testGetExtraCacheKeys() {
+		$slotDiffRenderer = $this->getTextSlotDiffRenderer();
+		$key = $slotDiffRenderer->getExtraCacheKeys();
+		$slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_WIKIDIFF2_INLINE );
+		$inlineKey = $slotDiffRenderer->getExtraCacheKeys();
+		$this->assertSame( $key, [] );
+		$this->assertSame( $inlineKey, [ phpversion( 'wikidiff2' ), 'inline' ] );
+	}
+
 	/**
 	 * @dataProvider provideGetDiff
 	 * @param array|null $oldContentArgs To pass to makeContent() (if not null)