Html: Move encodeJsVar() + encodeJsCall() from Xml

These methods really belong in the Html class, not Xml. Leave behind
soft-deprecated Xml methods that forward to the Html ones, as well as a
class alias for HtmlJsCode (renamed from XmlJsCode).

Bug: T341779
Change-Id: I99a5f9de1411d4eb5ee30226b4e8ace3ea8b2c3b

RELEASE-NOTES-1.41

@@ -365,6 +365,11 @@ because of Phabricator reports.
 * ResourceLoader (T127268): The targets system is deprecated. Modules that
   have been marked as desktop or mobile only are no longer supported and
   will send deprecation warnings.
+* The static methods encodeJsVar() and encodeJsCall() have been moved from the
+  Xml class to the more appropriate MediaWiki\Html\Html one, and the old ones
+  are now deprecated.
+* The XmlJsCode wrapper class has been renamed to MediaWiki\Html\HtmlJsCode,
+  and the old name is now deprecated.
 * …
 
 === Other changes in 1.41 ===

autoload.php

@@ -1451,6 +1451,7 @@ $wgAutoloadLocalClasses = [
 	'MediaWiki\\Html\\FormOptions' => __DIR__ . '/includes/Html/FormOptions.php',
 	'MediaWiki\\Html\\Html' => __DIR__ . '/includes/Html/Html.php',
 	'MediaWiki\\Html\\HtmlHelper' => __DIR__ . '/includes/Html/HtmlHelper.php',
+	'MediaWiki\\Html\\HtmlJsCode' => __DIR__ . '/includes/Html/HtmlJsCode.php',
 	'MediaWiki\\Html\\ListToggle' => __DIR__ . '/includes/Html/ListToggle.php',
 	'MediaWiki\\Html\\TemplateParser' => __DIR__ . '/includes/Html/TemplateParser.php',
 	'MediaWiki\\Http\\HttpRequestFactory' => __DIR__ . '/includes/http/HttpRequestFactory.php',
@@ -3074,7 +3075,7 @@ $wgAutoloadLocalClasses = [
 	'XhprofData' => __DIR__ . '/includes/libs/XhprofData.php',
 	'Xml' => __DIR__ . '/includes/xml/Xml.php',
 	'XmlDumpWriter' => __DIR__ . '/includes/export/XmlDumpWriter.php',
-	'XmlJsCode' => __DIR__ . '/includes/xml/XmlJsCode.php',
+	'XmlJsCode' => __DIR__ . '/includes/Html/HtmlJsCode.php',
 	'XmlSelect' => __DIR__ . '/includes/xml/XmlSelect.php',
 	'XmlTypeCheck' => __DIR__ . '/includes/libs/mime/XmlTypeCheck.php',
 	'ZhConverter' => __DIR__ . '/includes/language/converters/ZhConverter.php',

includes/Html/Html.php

@@ -25,6 +25,7 @@
 
 namespace MediaWiki\Html;
 
+use FormatJson;
 use InvalidArgumentException;
 use MediaWiki\MainConfigNames;
 use MediaWiki\MediaWikiServices;
@@ -1149,6 +1150,50 @@ class Html {
 
 		return implode( ", ", $candidates );
 	}
+
+	/**
+	 * Encode a variable of arbitrary type to JavaScript.
+	 * If the value is an HtmlJsCode object, pass through the object's value verbatim.
+	 *
+	 * @note Only use this function for generating JavaScript code. If generating output
+	 *       for a proper JSON parser, just call FormatJson::encode() directly.
+	 *
+	 * @since 1.41 (previously on {@link Xml})
+	 * @param mixed $value The value being encoded. Can be any type except a resource.
+	 * @param bool $pretty If true, add non-significant whitespace to improve readability.
+	 * @return string|false String if successful; false upon failure
+	 */
+	public static function encodeJsVar( $value, $pretty = false ) {
+		if ( $value instanceof HtmlJsCode ) {
+			return $value->value;
+		}
+		return FormatJson::encode( $value, $pretty, FormatJson::UTF8_OK );
+	}
+
+	/**
+	 * Create a call to a JavaScript function. The supplied arguments will be
+	 * encoded using Html::encodeJsVar().
+	 *
+	 * @since 1.41 (previously on {@link Xml} since 1.17)
+	 * @param string $name The name of the function to call, or a JavaScript expression
+	 *    which evaluates to a function object which is called.
+	 * @param array $args The arguments to pass to the function.
+	 * @param bool $pretty If true, add non-significant whitespace to improve readability.
+	 * @return string|false String if successful; false upon failure
+	 */
+	public static function encodeJsCall( $name, $args, $pretty = false ) {
+		foreach ( $args as &$arg ) {
+			$arg = self::encodeJsVar( $arg, $pretty );
+			if ( $arg === false ) {
+				return false;
+			}
+		}
+
+		return "$name(" . ( $pretty
+			? ( ' ' . implode( ', ', $args ) . ' ' )
+			: implode( ',', $args )
+		) . ");";
+	}
 }
 
 class_alias( Html::class, 'Html' );

includes/Html/HtmlJsCode.php

@@ -18,26 +18,28 @@
  * @file
  */
 
+namespace MediaWiki\Html;
+
 /**
- * A wrapper class which causes Xml::encodeJsVar() and Xml::encodeJsCall() to
+ * A wrapper class which causes Html::encodeJsVar() and Html::encodeJsCall()
- * interpret a given string as being a JavaScript expression, instead of string
+ * (as well as their Xml::* counterparts) to interpret a given string as being
- * data.
+ * a JavaScript expression, instead of string data.
  *
  * @par Example:
  * @code
- *     Xml::encodeJsVar( new XmlJsCode( 'a + b' ) );
+ *     Html::encodeJsVar( new HtmlJsCode( 'a + b' ) );
  * @endcode
  *
  * This returns "a + b".
  *
- * @note As of 1.21, XmlJsCode objects cannot be nested inside objects or arrays. The sole
+ * @note As of 1.21, HtmlJsCode objects cannot be nested inside objects or arrays. The sole
- *       exception is the $args argument to Xml::encodeJsCall() because Xml::encodeJsVar() is
+ *       exception is the $args argument to Html::encodeJsCall() because Html::encodeJsVar() is
  *       called for each individual element in that array. If you need to encode an object or array
- *       containing XmlJsCode objects, use XmlJsCode::encodeObject() to re-encode it first.
+ *       containing HtmlJsCode objects, use HtmlJsCode::encodeObject() to re-encode it first.
  *
- * @since 1.17
+ * @since 1.41 (renamed from XmlJsCode, which existed since 1.17)
  */
-class XmlJsCode {
+class HtmlJsCode {
 	public $value;
 
 	public function __construct( $value ) {
@@ -45,25 +47,25 @@ class XmlJsCode {
 	}
 
 	/**
-	 * Encode an object containing XmlJsCode objects.
+	 * Encode an object containing HtmlJsCode objects.
 	 *
-	 * This takes an object or associative array where (some of) the values are XmlJsCode objects,
+	 * This takes an object or associative array where (some of) the values are HtmlJsCode objects,
-	 * and re-encodes it as a single XmlJsCode object.
+	 * and re-encodes it as a single HtmlJsCode object.
 	 *
 	 * @since 1.33
 	 * @phpcs:ignore MediaWiki.Commenting.FunctionComment.ObjectTypeHintParam
 	 * @param object|array $obj Object or associative array to encode
 	 * @param bool $pretty If true, add non-significant whitespace to improve readability.
-	 * @return XmlJsCode
+	 * @return HtmlJsCode
 	 */
 	public static function encodeObject( $obj, $pretty = false ) {
 		$parts = [];
 		foreach ( $obj as $key => $value ) {
 			$parts[] =
 				( $pretty ? '    ' : '' ) .
-				Xml::encodeJsVar( $key, $pretty ) .
+				Html::encodeJsVar( $key, $pretty ) .
 				( $pretty ? ': ' : ':' ) .
-				Xml::encodeJsVar( $value, $pretty );
+				Html::encodeJsVar( $value, $pretty );
 		}
 		return new self(
 			'{' .
@@ -74,3 +76,6 @@ class XmlJsCode {
 		);
 	}
 }
+
+/** @deprecated since 1.41 */
+class_alias( HtmlJsCode::class, 'XmlJsCode' );

includes/ResourceLoader/ResourceLoader.php

@@ -35,6 +35,7 @@ use Less_Parser;
 use MediaWiki\CommentStore\CommentStore;
 use MediaWiki\HookContainer\HookContainer;
 use MediaWiki\Html\Html;
+use MediaWiki\Html\HtmlJsCode;
 use MediaWiki\MainConfigNames;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Profiler\ProfilingContext;
@@ -66,7 +67,6 @@ use Wikimedia\ScopedCallback;
 use Wikimedia\Timestamp\ConvertibleTimestamp;
 use Wikimedia\WrappedString;
 use Xml;
-use XmlJsCode;
 
 /**
  * @defgroup ResourceLoader ResourceLoader
@@ -1154,14 +1154,14 @@ MESSAGE;
 									$scripts = self::filter( 'minify-js', $scripts ); // T107377
 								}
 							} else {
-								$scripts = new XmlJsCode( $scripts );
+								$scripts = new HtmlJsCode( $scripts );
 							}
 						}
 						$strContent = self::makeLoaderImplementScript(
 							$implementKey,
 							$scripts,
 							$content['styles'] ?? [],
-							isset( $content['messagesBlob'] ) ? new XmlJsCode( $content['messagesBlob'] ) : null,
+							isset( $content['messagesBlob'] ) ? new HtmlJsCode( $content['messagesBlob'] ) : null,
 							$content['templates'] ?? []
 						);
 						break;
@@ -1262,9 +1262,9 @@ MESSAGE;
 	 * Return JS code that calls mw.loader.implement with given module properties.
 	 *
 	 * @param string $name Module name used as implement key (format "`[name]@[version]`")
-	 * @param XmlJsCode|array|string|string[] $scripts
+	 * @param HtmlJsCode|array|string|string[] $scripts
-	 *  - XmlJsCode: Concatenated scripts to be wrapped in a closure
+	 *  - HtmlJsCode: Concatenated scripts to be wrapped in a closure
-	 *  - array: Package files array containing XmlJsCode for individual JS files,
+	 *  - array: Package files array containing HtmlJsCode for individual JS files,
 	 *    as produced by Module::getScript().
 	 *  - string: Script contents to eval in global scope (for site/user scripts).
 	 *  - string[]: List of URLs (for debug mode).
@@ -1272,19 +1272,19 @@ MESSAGE;
 	 *   Under optional key "css", there is a concatenated CSS string.
 	 *   Under optional key "url", there is an array by media type withs URLs to stylesheets (for debug mode).
 	 *   These come from Module::getStyles(), formatted by Module:buildContent().
-	 * @param XmlJsCode|null $messages An already JSON-encoded map from message keys to values,
+	 * @param HtmlJsCode|null $messages An already JSON-encoded map from message keys to values,
-	 *   wrapped in an XmlJsCode object.
+	 *   wrapped in an HtmlJsCode object.
 	 * @param array<string,string> $templates Map from template name to template source.
 	 * @return string JavaScript code
 	 */
 	private static function makeLoaderImplementScript(
 		$name, $scripts, $styles, $messages, $templates
 	) {
-		if ( $scripts instanceof XmlJsCode ) {
+		if ( $scripts instanceof HtmlJsCode ) {
 			if ( $scripts->value === '' ) {
 				$scripts = null;
 			} else {
-				$scripts = new XmlJsCode( "function ( $, jQuery, require, module ) {\n{$scripts->value}\n}" );
+				$scripts = new HtmlJsCode( "function ( $, jQuery, require, module ) {\n{$scripts->value}\n}" );
 			}
 		} elseif ( is_array( $scripts ) && isset( $scripts['files'] ) ) {
 			$files = $scripts['files'];
@@ -1298,14 +1298,14 @@ MESSAGE;
 					// Provide CJS `exports` (in addition to CJS2 `module.exports`) to package modules (T284511).
 					// $/jQuery are simply used as globals instead.
 					// TODO: Remove $/jQuery param from traditional module closure too (and bump caching)
-					$file = new XmlJsCode( "function ( require, module, exports ) {\n$content}" );
+					$file = new HtmlJsCode( "function ( require, module, exports ) {\n$content}" );
 				} else {
 					$file = $file['content'];
 				}
 			}
-			$scripts = XmlJsCode::encodeObject( [
+			$scripts = HtmlJsCode::encodeObject( [
 				'main' => $scripts['main'],
-				'files' => XmlJsCode::encodeObject( $files, true )
+				'files' => HtmlJsCode::encodeObject( $files, true )
 			], true );
 		} elseif ( !is_string( $scripts ) && !is_array( $scripts ) ) {
 			throw new InvalidArgumentException( 'Script must be a string or an array of URLs' );
@@ -1422,7 +1422,7 @@ MESSAGE;
 	 *
 	 * - null
 	 * - []
-	 * - new XmlJsCode( '{}' )
+	 * - new HtmlJsCode( '{}' )
 	 * - new stdClass()
 	 * - (object)[]
 	 *
@@ -1433,7 +1433,7 @@ MESSAGE;
 		while ( $i-- ) {
 			if ( $array[$i] === null
 				|| $array[$i] === []
-				|| ( $array[$i] instanceof XmlJsCode && $array[$i]->value === '{}' )
+				|| ( $array[$i] instanceof HtmlJsCode && $array[$i]->value === '{}' )
 				|| ( $array[$i] instanceof stdClass && self::isEmptyObject( $array[$i] ) )
 			) {
 				unset( $array[$i] );

includes/xml/Xml.php

@@ -660,7 +660,7 @@ class Xml {
 
 	/**
 	 * Encode a variable of arbitrary type to JavaScript.
-	 * If the value is an XmlJsCode object, pass through the object's value verbatim.
+	 * If the value is an HtmlJsCode object, pass through the object's value verbatim.
 	 *
 	 * @note Only use this function for generating JavaScript code. If generating output
 	 *       for a proper JSON parser, just call FormatJson::encode() directly.
@@ -668,12 +668,10 @@ class Xml {
 	 * @param mixed $value The value being encoded. Can be any type except a resource.
 	 * @param bool $pretty If true, add non-significant whitespace to improve readability.
 	 * @return string|false String if successful; false upon failure
+	 * @deprecated since 1.41, use {@link Html::encodeJsVar()} instead
 	 */
 	public static function encodeJsVar( $value, $pretty = false ) {
-		if ( $value instanceof XmlJsCode ) {
+		return Html::encodeJsVar( $value, $pretty );
-			return $value->value;
-		}
-		return FormatJson::encode( $value, $pretty, FormatJson::UTF8_OK );
 	}
 
 	/**
@@ -686,19 +684,10 @@ class Xml {
 	 * @param array $args The arguments to pass to the function.
 	 * @param bool $pretty If true, add non-significant whitespace to improve readability.
 	 * @return string|false String if successful; false upon failure
+	 * @deprecated since 1.41, use {@link Html::encodeJsCall()} instead
 	 */
 	public static function encodeJsCall( $name, $args, $pretty = false ) {
-		foreach ( $args as &$arg ) {
+		return Html::encodeJsCall( $name, $args, $pretty );
-			$arg = self::encodeJsVar( $arg, $pretty );
-			if ( $arg === false ) {
-				return false;
-			}
-		}
-
-		return "$name(" . ( $pretty
-			? ( ' ' . implode( ', ', $args ) . ' ' )
-			: implode( ',', $args )
-		) . ");";
 	}
 
 	/**

tests/phpunit/includes/HtmlTest.php

@@ -1,6 +1,7 @@
 <?php
 
 use MediaWiki\Html\Html;
+use MediaWiki\Html\HtmlJsCode;
 use MediaWiki\MainConfigNames;
 
 /**
@@ -889,6 +890,49 @@ class HtmlTest extends MediaWikiIntegrationTestCase {
 			'expectedClassAttribute' => 'foo',
 		];
 	}
+
+	public static function provideEncodeJsVar() {
+		// $expected, $input
+		yield 'boolean' => [ 'true', true ];
+		yield 'null' => [ 'null', null ];
+		yield 'array' => [ '["a",1]', [ 'a', 1 ] ];
+		yield 'associative arary' => [ '{"a":"a","b":1}', [ 'a' => 'a', 'b' => 1 ] ];
+		yield 'object' => [ '{"a":"a","b":1}', (object)[ 'a' => 'a', 'b' => 1 ] ];
+		yield 'int' => [ '123456', 123456 ];
+		yield 'float' => [ '1.5', 1.5 ];
+		yield 'int-like string' => [ '"123456"', '123456' ];
+
+		$code = 'function () { foo( 42 ); }';
+		yield 'code' => [ $code, new HtmlJsCode( $code ) ];
+	}
+
+	/**
+	 * @covers \MediaWiki\Html\Html::encodeJsVar
+	 * @dataProvider provideEncodeJsVar
+	 */
+	public function testEncodeJsVar( string $expect, $input ) {
+		$this->assertEquals(
+			$expect,
+			Html::encodeJsVar( $input )
+		);
+	}
+
+	/**
+	 * @covers \MediaWiki\Html\Html::encodeJsVar
+	 * @covers \MediaWiki\Html\HtmlJsCode::encodeObject
+	 */
+	public function testEncodeObject() {
+		$codeA = 'function () { foo( 42 ); }';
+		$codeB = 'function ( jQuery ) { bar( 142857 ); }';
+		$obj = HtmlJsCode::encodeObject( [
+			'a' => new HtmlJsCode( $codeA ),
+			'b' => new HtmlJsCode( $codeB )
+		] );
+		$this->assertEquals(
+			"{\"a\":$codeA,\"b\":$codeB}",
+			Html::encodeJsVar( $obj )
+		);
+	}
 }
 
 class HtmlTestValue {

tests/phpunit/includes/ResourceLoader/ResourceLoaderTest.php

@@ -6,6 +6,7 @@ use EmptyResourceLoader;
 use Exception;
 use ExtensionRegistry;
 use InvalidArgumentException;
+use MediaWiki\Html\HtmlJsCode;
 use MediaWiki\MainConfigNames;
 use MediaWiki\Request\FauxRequest;
 use MediaWiki\ResourceLoader\Context;
@@ -20,7 +21,6 @@ use ResourceLoaderTestModule;
 use RuntimeException;
 use UnexpectedValueException;
 use Wikimedia\TestingAccessWrapper;
-use XmlJsCode;
 
 /**
  * @covers \MediaWiki\ResourceLoader\ResourceLoader
@@ -564,7 +564,7 @@ END
 			'wrap' => true,
 			'styles' => [],
 			'templates' => [],
-			'messages' => new XmlJsCode( '{}' ),
+			'messages' => new HtmlJsCode( '{}' ),
 			'packageFiles' => [],
 		];
 		$rl = TestingAccessWrapper::newFromClass( ResourceLoader::class );
@@ -573,7 +573,7 @@ END
 			$rl->makeLoaderImplementScript(
 				$case['name'],
 				( $case['wrap'] && is_string( $case['scripts'] ) )
-					? new XmlJsCode( $case['scripts'] )
+					? new HtmlJsCode( $case['scripts'] )
 					: $case['scripts'],
 				$case['styles'],
 				$case['messages'],

tests/phpunit/unit/includes/HtmlJsCodeTest.php

@@ -0,0 +1,18 @@
+<?php
+
+use MediaWiki\Html\HtmlJsCode;
+
+/**
+ * @covers \MediaWiki\Html\HtmlJsCode
+ */
+class HtmlJsCodeTest extends MediaWikiUnitTestCase {
+
+	public function testConstruction() {
+		$obj = new HtmlJsCode( '' );
+		$this->assertSame( '', $obj->value );
+
+		$obj = new HtmlJsCode( null );
+		$this->assertNull( $obj->value );
+	}
+
+}

tests/phpunit/unit/includes/XmlJsTest.php

@@ -1,19 +0,0 @@
-<?php
-
-/**
- * @group Xml
- */
-class XmlJsTest extends MediaWikiUnitTestCase {
-
-	/**
-	 * @covers XmlJsCode::__construct
-	 */
-	public function testConstruction() {
-		$obj = new XmlJsCode( '' );
-		$this->assertSame( '', $obj->value );
-
-		$obj = new XmlJsCode( null );
-		$this->assertNull( $obj->value );
-	}
-
-}

tests/phpunit/unit/includes/XmlTest.php

@@ -23,7 +23,6 @@ namespace MediaWiki\Tests\Unit;
 
 use MediaWikiUnitTestCase;
 use Xml;
-use XmlJsCode;
 
 /**
  * Split from \XmlTest integration tests
@@ -56,47 +55,4 @@ class XmlTest extends MediaWikiUnitTestCase {
 		);
 	}
 
-	public static function provideEncodeJsVar() {
-		// $expected, $input
-		yield 'boolean' => [ 'true', true ];
-		yield 'null' => [ 'null', null ];
-		yield 'array' => [ '["a",1]', [ 'a', 1 ] ];
-		yield 'associative arary' => [ '{"a":"a","b":1}', [ 'a' => 'a', 'b' => 1 ] ];
-		yield 'object' => [ '{"a":"a","b":1}', (object)[ 'a' => 'a', 'b' => 1 ] ];
-		yield 'int' => [ '123456', 123456 ];
-		yield 'float' => [ '1.5', 1.5 ];
-		yield 'int-like string' => [ '"123456"', '123456' ];
-
-		$code = 'function () { foo( 42 ); }';
-		yield 'code' => [ $code, new XmlJsCode( $code ) ];
-	}
-
-	/**
-	 * @covers Xml::encodeJsVar
-	 * @dataProvider provideEncodeJsVar
-	 */
-	public function testEncodeJsVar( string $expect, $input ) {
-		$this->assertEquals(
-			$expect,
-			Xml::encodeJsVar( $input )
-		);
-	}
-
-	/**
-	 * @covers Xml::encodeJsVar
-	 * @covers XmlJsCode::encodeObject
-	 */
-	public function testEncodeObject() {
-		$codeA = 'function () { foo( 42 ); }';
-		$codeB = 'function ( jQuery ) { bar( 142857 ); }';
-		$obj = XmlJsCode::encodeObject( [
-			'a' => new XmlJsCode( $codeA ),
-			'b' => new XmlJsCode( $codeB )
-		] );
-		$this->assertEquals(
-			"{\"a\":$codeA,\"b\":$codeB}",
-			Xml::encodeJsVar( $obj )
-		);
-	}
-
 }