From 3c5a0c862f834b791d6ac020a26f1d3d401bb34f Mon Sep 17 00:00:00 2001 From: Lucas Werkmeister Date: Thu, 13 Jul 2023 17:44:35 +0200 Subject: [PATCH] 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 | 5 +++ autoload.php | 3 +- includes/Html/Html.php | 45 +++++++++++++++++++ .../XmlJsCode.php => Html/HtmlJsCode.php} | 35 ++++++++------- includes/ResourceLoader/ResourceLoader.php | 30 ++++++------- includes/xml/Xml.php | 21 +++------ tests/phpunit/includes/HtmlTest.php | 44 ++++++++++++++++++ .../ResourceLoader/ResourceLoaderTest.php | 6 +-- .../phpunit/unit/includes/HtmlJsCodeTest.php | 18 ++++++++ tests/phpunit/unit/includes/XmlJsTest.php | 19 -------- tests/phpunit/unit/includes/XmlTest.php | 44 ------------------ 11 files changed, 157 insertions(+), 113 deletions(-) rename includes/{xml/XmlJsCode.php => Html/HtmlJsCode.php} (63%) create mode 100644 tests/phpunit/unit/includes/HtmlJsCodeTest.php delete mode 100644 tests/phpunit/unit/includes/XmlJsTest.php diff --git a/RELEASE-NOTES-1.41 b/RELEASE-NOTES-1.41 index d4a89f9edeb..1580808e0dc 100644 --- a/RELEASE-NOTES-1.41 +++ b/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 === diff --git a/autoload.php b/autoload.php index ba11a681ca3..6561381ccb5 100644 --- a/autoload.php +++ b/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', diff --git a/includes/Html/Html.php b/includes/Html/Html.php index ceb85f2a5ff..e4f145b8802 100644 --- a/includes/Html/Html.php +++ b/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' ); diff --git a/includes/xml/XmlJsCode.php b/includes/Html/HtmlJsCode.php similarity index 63% rename from includes/xml/XmlJsCode.php rename to includes/Html/HtmlJsCode.php index 8d850cce3ce..e857237f390 100644 --- a/includes/xml/XmlJsCode.php +++ b/includes/Html/HtmlJsCode.php @@ -18,26 +18,28 @@ * @file */ +namespace MediaWiki\Html; + /** - * A wrapper class which causes Xml::encodeJsVar() and Xml::encodeJsCall() to - * interpret a given string as being a JavaScript expression, instead of string - * data. + * A wrapper class which causes Html::encodeJsVar() and Html::encodeJsCall() + * (as well as their Xml::* counterparts) to interpret a given string as being + * 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 - * exception is the $args argument to Xml::encodeJsCall() because Xml::encodeJsVar() is + * @note As of 1.21, HtmlJsCode objects cannot be nested inside objects or arrays. The sole + * 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, - * and re-encodes it as a single XmlJsCode object. + * This takes an object or associative array where (some of) the values are HtmlJsCode objects, + * 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' ); diff --git a/includes/ResourceLoader/ResourceLoader.php b/includes/ResourceLoader/ResourceLoader.php index 4dbedfff0ee..572ea9c26e1 100644 --- a/includes/ResourceLoader/ResourceLoader.php +++ b/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 - * - XmlJsCode: Concatenated scripts to be wrapped in a closure - * - array: Package files array containing XmlJsCode for individual JS files, + * @param HtmlJsCode|array|string|string[] $scripts + * - HtmlJsCode: Concatenated scripts to be wrapped in a closure + * - 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, - * wrapped in an XmlJsCode object. + * @param HtmlJsCode|null $messages An already JSON-encoded map from message keys to values, + * wrapped in an HtmlJsCode object. * @param array $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] ); diff --git a/includes/xml/Xml.php b/includes/xml/Xml.php index 9e50d79392c..0c2a6c29a95 100644 --- a/includes/xml/Xml.php +++ b/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 $value->value; - } - return FormatJson::encode( $value, $pretty, FormatJson::UTF8_OK ); + return Html::encodeJsVar( $value, $pretty ); } /** @@ -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 ) { - $arg = self::encodeJsVar( $arg, $pretty ); - if ( $arg === false ) { - return false; - } - } - - return "$name(" . ( $pretty - ? ( ' ' . implode( ', ', $args ) . ' ' ) - : implode( ',', $args ) - ) . ");"; + return Html::encodeJsCall( $name, $args, $pretty ); } /** diff --git a/tests/phpunit/includes/HtmlTest.php b/tests/phpunit/includes/HtmlTest.php index ed59a9054c4..d7b436c1f23 100644 --- a/tests/phpunit/includes/HtmlTest.php +++ b/tests/phpunit/includes/HtmlTest.php @@ -1,6 +1,7 @@ '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 { diff --git a/tests/phpunit/includes/ResourceLoader/ResourceLoaderTest.php b/tests/phpunit/includes/ResourceLoader/ResourceLoaderTest.php index 1641b35abe7..87fb12dd191 100644 --- a/tests/phpunit/includes/ResourceLoader/ResourceLoaderTest.php +++ b/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'], diff --git a/tests/phpunit/unit/includes/HtmlJsCodeTest.php b/tests/phpunit/unit/includes/HtmlJsCodeTest.php new file mode 100644 index 00000000000..bff74757e13 --- /dev/null +++ b/tests/phpunit/unit/includes/HtmlJsCodeTest.php @@ -0,0 +1,18 @@ +assertSame( '', $obj->value ); + + $obj = new HtmlJsCode( null ); + $this->assertNull( $obj->value ); + } + +} diff --git a/tests/phpunit/unit/includes/XmlJsTest.php b/tests/phpunit/unit/includes/XmlJsTest.php deleted file mode 100644 index c371cd20cb9..00000000000 --- a/tests/phpunit/unit/includes/XmlJsTest.php +++ /dev/null @@ -1,19 +0,0 @@ -assertSame( '', $obj->value ); - - $obj = new XmlJsCode( null ); - $this->assertNull( $obj->value ); - } - -} diff --git a/tests/phpunit/unit/includes/XmlTest.php b/tests/phpunit/unit/includes/XmlTest.php index 36d26f63814..6ec037a754c 100644 --- a/tests/phpunit/unit/includes/XmlTest.php +++ b/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 ) - ); - } - }