From 5207a15b910ea6cfe33e4c1faadcb415d7c9d037 Mon Sep 17 00:00:00 2001
From: MusikAnimal <musikanimal@gmail.com>
Date: Tue, 23 Nov 2021 00:11:22 -0500
Subject: [PATCH] HTMLLegacySkinVersionField: accept string 'default' value

After I628435a4a, we were asserting a boolean was given because we're
extending HTMLFormField which requires a boolean value. This was safe
because GlobalPrefs would provide a boolean, but that changed with
I594f6297.

We could rework GlobalPrefs once again to ensure only a boolean is
passed in, but since HTMLLegacySkinVersionField already has special
handling around the data types, it seems to make sense to contain the
type transformation in this class.

Simply removing the Assertion is enough to prevent T296068, however
depending on when the global preference was saved (such as since MW
1.38.0-wmf.9 but before wmf.10), it's possible either a bool or a string
was saved, hence we check for both to ensure correct display.

Bug: T296068
Change-Id: If10b948617d2bb8346475f207fe425fb768cb987
---
 .../Fields/HTMLLegacySkinVersionField.php     | 12 ++++++-----
 .../Fields/HTMLLegacySkinVersionFieldTest.php | 21 ++++++++++++-------
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/includes/HTMLForm/Fields/HTMLLegacySkinVersionField.php b/includes/HTMLForm/Fields/HTMLLegacySkinVersionField.php
index 148812d..60b8e53 100644
--- a/includes/HTMLForm/Fields/HTMLLegacySkinVersionField.php
+++ b/includes/HTMLForm/Fields/HTMLLegacySkinVersionField.php
@@ -3,7 +3,6 @@
 namespace Vector\HTMLForm\Fields;
 
 use Vector\Constants;
-use Wikimedia\Assert\Assert;
 
 /**
  * The field on Special:Preferences (and Special:GlobalPreferences) that allows the user to
@@ -34,10 +33,13 @@ final class HTMLLegacySkinVersionField extends \HTMLFormField {
 	 * @inheritDoc
 	 */
 	public function __construct( $params ) {
-		Assert::precondition(
-			is_bool( $params['default'] ),
-			'The "default" param must be a boolean.'
-		);
+		/**
+		 * HTMLCheckField must be given a boolean as the 'default' value.
+		 * Since MW 1.38.0-wmf.9, we could be given a boolean or a string.
+		 * @see T296068
+		 */
+		$params['default'] = $params['default'] === true ||
+			$params['default'] === Constants::SKIN_VERSION_LEGACY;
 
 		parent::__construct( $params );
 
diff --git a/tests/phpunit/integration/HTMLForm/Fields/HTMLLegacySkinVersionFieldTest.php b/tests/phpunit/integration/HTMLForm/Fields/HTMLLegacySkinVersionFieldTest.php
index f6f3cb6..ded5fdb 100644
--- a/tests/phpunit/integration/HTMLForm/Fields/HTMLLegacySkinVersionFieldTest.php
+++ b/tests/phpunit/integration/HTMLForm/Fields/HTMLLegacySkinVersionFieldTest.php
@@ -31,20 +31,27 @@ use Vector\HTMLForm\Fields\HTMLLegacySkinVersionField;
 class HTMLLegacySkinVersionFieldTest extends \MediaWikiIntegrationTestCase {
 
 	public function provideDefault() {
-		yield [ 'true' ];
-		yield [ 1 ];
+		return [
+			[ false, '0' ],
+			[ false, false ],
+			[ true, '1' ],
+			[ true, true ],
+		];
 	}
 
 	/**
 	 * @dataProvider provideDefault
 	 * @covers ::__construct
 	 */
-	public function testConstructValidatesDefault( $default ) {
-		$this->expectException( \Wikimedia\Assert\PreconditionException::class );
-
-		new HTMLLegacySkinVersionField( [
-			'default' => $default
+	public function testConstructValidatesDefault( $expected, $default ) {
+		$field = new HTMLLegacySkinVersionField( [
+			'default' => $default,
+			'fieldname' => 'VectorSkinVersion',
 		] );
+		$this->assertSame(
+			$expected,
+			$field->getDefault()
+		);
 	}
 
 	public function provideGetInput() {