Page MenuHomePhorge
Files F585028

No OneTemporary
Actions

Size
9 KB
Referenced Files
None
Subscribers
None

View Options

diff --git a/includes/Html/Html.php b/includes/Html/Html.php
index 5cc56f462ac..8612456b8e0 100644
--- a/includes/Html/Html.php
+++ b/includes/Html/Html.php
@@ -133,6 +133,32 @@ class Html {
return $attrs;
}
+ /**
+ * Add a class to a 'class' attribute in a format accepted by Html::element().
+ *
+ * This method may also be used for any other space-separated attribute, such as 'rel'.
+ *
+ * @param array|string|null &$classes Class list to modify in-place
+ * @param string $class Class to add
+ * @phan-assert non-empty-array $classes
+ */
+ public static function addClass( &$classes, string $class ): void {
+ $classes = (array)$classes;
+ // Detect mistakes where $attrs is passed as $classes instead of $attrs['class']
+ foreach ( $classes as $key => $val ) {
+ if (
+ ( is_int( $key ) && is_string( $val ) ) ||
+ ( is_string( $key ) && is_bool( $val ) )
+ ) {
+ // Valid formats for class array entries
+ continue;
+ }
+ wfWarn( __METHOD__ . ": Argument doesn't look like a class array: " . var_export( $classes, true ) );
+ break;
+ }
+ $classes[] = $class;
+ }
+
/**
* Returns an HTML link element in a string.
*
@@ -700,18 +726,8 @@ class Html {
if ( $heading !== '' ) {
$html = self::element( 'h2', [], $heading ) . $html;
}
- $coreClasses = [
- 'cdx-message',
- 'cdx-message--block'
- ];
- if ( is_array( $className ) ) {
- $className = array_merge(
- $coreClasses,
- $className
- );
- } else {
- $className .= ' ' . implode( ' ', $coreClasses );
- }
+ self::addClass( $className, 'cdx-message' );
+ self::addClass( $className, 'cdx-message--block' );
return self::rawElement( 'div', [ 'class' => $className ],
self::element( 'span', [ 'class' => [
'cdx-message__icon',
diff --git a/includes/OutputTransform/Stages/HandleSectionLinks.php b/includes/OutputTransform/Stages/HandleSectionLinks.php
index 45912bc18ab..fed94e51af4 100644
--- a/includes/OutputTransform/Stages/HandleSectionLinks.php
+++ b/includes/OutputTransform/Stages/HandleSectionLinks.php
@@ -131,8 +131,7 @@ class HandleSectionLinks extends ContentTextTransformStage {
if ( $this->isHtmlHeading( $attrs ) ) {
// This is a <h#> tag with attributes added using HTML syntax.
// Mark it with a class to make them easier to distinguish (T68637).
- $attrs['class'] = isset( $attrs['class'] ) ? (array)$attrs['class'] : [];
- $attrs['class'][] = 'mw-html-heading';
+ Html::addClass( $attrs['class'], 'mw-html-heading' );
// Do not add the wrapper if the heading has attributes added using HTML syntax (T353489).
// In this case it's also guaranteed that there's no edit link, so we don't need wrappers.
diff --git a/includes/editpage/TextboxBuilder.php b/includes/editpage/TextboxBuilder.php
index 382a26ba324..b5a587e2c5f 100644
--- a/includes/editpage/TextboxBuilder.php
+++ b/includes/editpage/TextboxBuilder.php
@@ -20,6 +20,7 @@
namespace MediaWiki\EditPage;
+use MediaWiki\Html\Html;
use MediaWiki\MediaWikiServices;
use MediaWiki\Page\PageIdentity;
use MediaWiki\Parser\Sanitizer;
@@ -119,16 +120,7 @@ class TextboxBuilder {
// * mw-editfont-serif
$userOptionsLookup = MediaWikiServices::getInstance()->getUserOptionsLookup();
$class = 'mw-editfont-' . $userOptionsLookup->getOption( $user, 'editfont' );
-
- if ( isset( $attribs['class'] ) ) {
- if ( is_string( $attribs['class'] ) ) {
- $attribs['class'] .= ' ' . $class;
- } elseif ( is_array( $attribs['class'] ) ) {
- $attribs['class'][] = $class;
- }
- } else {
- $attribs['class'] = $class;
- }
+ Html::addClass( $attribs['class'], $class );
$title = Title::newFromPageIdentity( $page );
$pageLang = $title->getPageLanguage();
diff --git a/includes/htmlform/CodexHTMLForm.php b/includes/htmlform/CodexHTMLForm.php
index de28be0e23f..48e84a7c32c 100644
--- a/includes/htmlform/CodexHTMLForm.php
+++ b/includes/htmlform/CodexHTMLForm.php
@@ -236,13 +236,7 @@ class CodexHTMLForm extends HTMLForm {
$attrs['id'] = $button['id'];
}
- if ( isset( $attrs['class'] ) ) {
- // Ensure $attrs['class'] is always treated as an array whether it's initially set
- // as an array or a string.
- $attrs['class'] = (array)( $attrs['class'] ?? [] );
- }
-
- $attrs['class'][] = 'cdx-button';
+ Html::addClass( $attrs['class'], 'cdx-button' );
$buttons[] = Html::rawElement( 'button', $attrs, $label ) . "\n";
}
diff --git a/tests/phpunit/includes/Html/HtmlTest.php b/tests/phpunit/includes/Html/HtmlTest.php
index 6fa9063141a..b7ce4fcb312 100644
--- a/tests/phpunit/includes/Html/HtmlTest.php
+++ b/tests/phpunit/includes/Html/HtmlTest.php
@@ -68,6 +68,68 @@ class HtmlTest extends MediaWikiIntegrationTestCase {
$this->setUserLang( $userLangObj );
}
+ public function testAddClassSpecialCases() {
+ // You probably shouldn't do this, but it works
+ Html::addClass( $attrs['class'], 'foo' );
+ $this->assertEquals( [ 'class' => [ 'foo' ] ], $attrs, 'Variable is defined if missing' );
+
+ // This is intended though and supported
+ $attrs = [];
+ Html::addClass( $attrs['class'], 'foo' );
+ $this->assertEquals( [ 'class' => [ 'foo' ] ], $attrs, 'Attribute is added if missing' );
+
+ // Warning is emitted if attributes are passed instead of classes
+ $attrs = [ 'title' => 'hello' ];
+ $this->expectPHPError(
+ E_USER_NOTICE,
+ static function () use ( &$attrs ) {
+ Html::addClass( $attrs, 'foo' );
+ },
+ "Argument doesn't look like a class array"
+ );
+ $this->assertEquals( [ 'title' => 'hello', 'foo' ], $attrs );
+ }
+
+ /**
+ * @dataProvider provideAddClass
+ */
+ public function testAddClass( $input, $class, $expected ) {
+ Html::addClass( $input, $class );
+ $this->assertEquals( $expected, $input );
+ }
+
+ public static function provideAddClass() {
+ yield "Null" =>
+ [ null, 'foo', [ 'foo' ] ];
+
+ yield "Empty array" =>
+ [ [], 'foo', [ 'foo' ] ];
+
+ yield "Array" =>
+ [ [ 'foo' ], 'bar', [ 'foo', 'bar' ] ];
+
+ yield "Empty string" =>
+ [ '', 'bar', [ '', 'bar' ] ];
+
+ yield "String" =>
+ [ 'foo', 'bar', [ 'foo', 'bar' ] ];
+
+ yield "Assoc" =>
+ [ [ 'foo' => false ], 'bar', [ 'foo' => false, 'bar' ] ];
+
+ yield "Duplicate" =>
+ [ [ 'foo' ], 'foo', [ 'foo', 'foo' ] ];
+
+ yield "Duplicate string" =>
+ [ 'foo', 'foo', [ 'foo', 'foo' ] ];
+
+ yield "Duplicate assoc" =>
+ [ [ 'foo' => false ], 'foo', [ 'foo' => false, 'foo' ] ];
+
+ yield "No cleanup" =>
+ [ ' a b ', ' c d ', [ ' a b ', ' c d ' ] ];
+ }
+
public function testOpenElement() {
$this->expectPHPError(
E_USER_NOTICE,
@@ -480,7 +542,7 @@ class HtmlTest extends MediaWikiIntegrationTestCase {
public function testWarningBox() {
$this->assertEquals(
- '<div class="cdx-message cdx-message--block cdx-message--warning">'
+ '<div class="cdx-message--warning cdx-message cdx-message--block">'
. '<span class="cdx-message__icon"></span>'
. '<div class="cdx-message__content">warn</div></div>',
Html::warningBox( 'warn' )
@@ -489,13 +551,13 @@ class HtmlTest extends MediaWikiIntegrationTestCase {
public function testErrorBox() {
$this->assertEquals(
- '<div class="cdx-message cdx-message--block cdx-message--error">'
+ '<div class="cdx-message--error cdx-message cdx-message--block">'
. '<span class="cdx-message__icon"></span>'
. '<div class="cdx-message__content">err</div></div>',
Html::errorBox( 'err' )
);
$this->assertEquals(
- '<div class="cdx-message cdx-message--block cdx-message--error errorbox-custom-class">'
+ '<div class="cdx-message--error errorbox-custom-class cdx-message cdx-message--block">'
. '<span class="cdx-message__icon"></span>'
. '<div class="cdx-message__content">'
. '<h2>heading</h2>err'
@@ -503,7 +565,7 @@ class HtmlTest extends MediaWikiIntegrationTestCase {
Html::errorBox( 'err', 'heading', 'errorbox-custom-class' )
);
$this->assertEquals(
- '<div class="cdx-message cdx-message--block cdx-message--error">'
+ '<div class="cdx-message--error cdx-message cdx-message--block">'
. '<span class="cdx-message__icon"></span>'
. '<div class="cdx-message__content">'
. '<h2>0</h2>err'
@@ -514,13 +576,13 @@ class HtmlTest extends MediaWikiIntegrationTestCase {
public function testSuccessBox() {
$this->assertEquals(
- '<div class="cdx-message cdx-message--block cdx-message--success">'
+ '<div class="cdx-message--success cdx-message cdx-message--block">'
. '<span class="cdx-message__icon"></span>'
. '<div class="cdx-message__content">great</div></div>',
Html::successBox( 'great' )
);
$this->assertEquals(
- '<div class="cdx-message cdx-message--block cdx-message--success">'
+ '<div class="cdx-message--success cdx-message cdx-message--block">'
. '<span class="cdx-message__icon"></span>'
. '<div class="cdx-message__content">'
. '<script>beware no escaping!</script>'
diff --git a/tests/phpunit/includes/editpage/TextboxBuilderTest.php b/tests/phpunit/includes/editpage/TextboxBuilderTest.php
index cd110de0b4f..b732166320a 100644
--- a/tests/phpunit/includes/editpage/TextboxBuilderTest.php
+++ b/tests/phpunit/includes/editpage/TextboxBuilderTest.php
@@ -124,7 +124,7 @@ class TextboxBuilderTest extends MediaWikiIntegrationTestCase {
// custom attrib showed up
$this->assertArrayHasKey( 'data-foo', $attribs );
// classes merged properly (string)
- $this->assertSame( 'foo bar mw-editfont-monospace', $attribs['class'] );
+ $this->assertSame( [ 'foo bar', 'mw-editfont-monospace' ], $attribs['class'] );
// overrides in custom attrib worked
$this->assertSame( 30, $attribs['rows'] );
$this->assertSame( 'en', $attribs['lang'] );
@@ -139,6 +139,6 @@ class TextboxBuilderTest extends MediaWikiIntegrationTestCase {
'mw-textbox3', [], $user, $title
);
// classes ok when nothing to be merged
- $this->assertSame( 'mw-editfont-monospace', $attribs3['class'] );
+ $this->assertSame( [ 'mw-editfont-monospace' ], $attribs3['class'] );
}
}

File Metadata

Mime Type
text/x-diff
Expires
Sat, Jul 5, 5:31 AM (12 h, 55 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
227479
Default Alt Text
(9 KB)

Event Timeline