SpecialGlobalGroupMembership: Avoid deprecated Xml::checkLabel

This method has a ton of layers of indirection, which makes for an
unintuitive signature, as well as various compromises in its API
design that are not something you can reasonably remember and learn.

```
Xml::checkLabel( $member, "wpGroup-" . $group,
    "wpGroup-" . $group, $set, $attr );
```

You can stare at this for quite a while and still have no confidence
in what it actually produces. You have to either run it in a REPL,
or setup up an elaborate UX test to find the exact scenario in a local
environment to inspect the DOM. The method docs don't mitigate this,
either, but with such a bad signature, docs would indeed be at best
help mitigate and manage expectations, instead of actually making it
intuitiive, productive, or memorable.

Xml::checkLabel() takes 4 unnamed parameters, of which two are often
the same value passed twice (name and ID), one applies only to the
label ($labelText) which is actually the first parameter despite being
used for only the last element in the output. It produces the `<input>`
element first and the `<label>` after it. The `$attrs` surprisingly
applies to both elements but in different ways. They're all blindly
copied to the input element, but then some hardcodes subset gets
duplicated to the `<label>` element as well.

> Xml::checkLabel(1, 2, 3, 4) modifies $attrs with ID.
-> Xml::check modifies $attrs again with $name, but ignores 1=labelText
-> -> Xml::attrib
-> -> Xml::element
-> Xml::label ignores most but not all (!) of the 4=attrs parameter.
-> -> Xml::element

Replace it all with a simple Html::element() instead.

Change-Id: I61c8f6712734dd07ce6f7b8d14f7d88de671aab4
这个提交包含在:
Timo Tijhof 2024-03-22 11:47:14 -07:00
父节点 6b35adf5a3
当前提交 2d7d5e3383
共有 3 个文件被更改,包括 10 次插入32 次删除

查看文件

@ -5,6 +5,7 @@
</rule>
<rule ref="Generic.Files.LineLength">
<exclude-pattern>CentralAuth\.*alias\.php</exclude-pattern>
<exclude-pattern>*/tests/*</exclude-pattern>
</rule>
<file>.</file>

查看文件

@ -651,11 +651,13 @@ class SpecialGlobalGroupMembership extends SpecialPage {
foreach ( $allgroups as $group ) {
$set = array_key_exists( $group, $currentGroups );
$attr = [ 'class' => 'mw-userrights-groupcheckbox' ];
$member = $uiLanguage->getGroupMemberName( $group, $user->getName() );
$checkboxHtml = Xml::checkLabel( $member, "wpGroup-" . $group,
"wpGroup-" . $group, $set, $attr );
$id = "wpGroup-$group";
$checkboxHtml = Html::element( 'input', [
'class' => 'mw-userrights-groupcheckbox',
'type' => 'checkbox', 'value' => '1', 'checked' => $set,
'id' => $id, 'name' => $id,
] ) . '&nbsp;' . Html::label( $member, $id );
$uiUser = $this->getUser();

查看文件

@ -30,7 +30,6 @@ use MediaWiki\Request\FauxRequest;
use MediaWiki\Session\SessionManager;
use MediaWiki\WikiMap\WikiMap;
use SpecialPageTestBase;
use Xml;
/**
* @coversDefaultClass \MediaWiki\Extension\CentralAuth\Special\SpecialGlobalGroupMembership
@ -145,29 +144,13 @@ class SpecialGlobalGroupMembershipTest extends SpecialPageTestBase {
// Group one: not a member
// TODO: would be cool to not test the raw HTML structure here
$this->assertStringContainsString(
Xml::checkLabel(
$specialPage->msg( 'group-group-one-member', $user->getName() )
->inLanguage( 'qqx' )
->text(),
'wpGroup-group-one',
'wpGroup-group-one',
false,
[ 'class' => 'mw-userrights-groupcheckbox' ],
),
'<input class="mw-userrights-groupcheckbox" type="checkbox" value="1" id="wpGroup-group-one" name="wpGroup-group-one">',
$html
);
// Group two: indefinite member
$this->assertStringContainsString(
Xml::checkLabel(
$specialPage->msg( 'group-group-two-member', $user->getName() )
->inLanguage( 'qqx' )
->text(),
'wpGroup-group-two',
'wpGroup-group-two',
true,
[ 'class' => 'mw-userrights-groupcheckbox' ],
),
'<input class="mw-userrights-groupcheckbox" type="checkbox" value="1" checked="" id="wpGroup-group-two" name="wpGroup-group-two">',
$html
);
$this->assertStringContainsString(
@ -183,15 +166,7 @@ class SpecialGlobalGroupMembershipTest extends SpecialPageTestBase {
// Group three: temporary member
$this->assertStringContainsString(
Xml::checkLabel(
$specialPage->msg( 'group-group-three-member', $user->getName() )
->inLanguage( 'qqx' )
->text(),
'wpGroup-group-three',
'wpGroup-group-three',
true,
[ 'class' => 'mw-userrights-groupcheckbox' ],
),
'<input class="mw-userrights-groupcheckbox" type="checkbox" value="1" checked="" id="wpGroup-group-three" name="wpGroup-group-three">',
$html
);
$this->assertStringContainsString(