From e3fa6b8d34a9bbb43a8c6fbf5d2859bcbfe0791c Mon Sep 17 00:00:00 2001
From: =?utf8?q?Bartosz=20Dziewo=C5=84ski?= <matma.rex@gmail.com>
Date: Thu, 16 Nov 2017 11:24:47 +0100
Subject: [PATCH 1/1] OOUIHTMLForm: Prevent duplicate FieldsetLayout wrapping

The code in formatSection() assumed it was only called for the
toplevel section (the whole form), while it's actually called
for every subsection too. I think it was written before we added
support for subsections in OOUIHTMLForm.

Move code for toplevel section wrapping to wrapForm().

As a bonus, this also fixes display of custom headers and error
or warning messages for forms with subsections.

Bug: T180535
Change-Id: I6a88184d302a951be78387490404137acde3fa1a
---
 includes/htmlform/OOUIHTMLForm.php | 70 +++++++++++++++++-------------
 1 file changed, 39 insertions(+), 31 deletions(-)

diff --git a/includes/htmlform/OOUIHTMLForm.php b/includes/htmlform/OOUIHTMLForm.php
index 1755e98cfb..ba36888ca8 100644
--- a/includes/htmlform/OOUIHTMLForm.php
+++ b/includes/htmlform/OOUIHTMLForm.php
@@ -182,16 +182,16 @@ class OOUIHTMLForm extends HTMLForm {
 			return '';
 		}
 
-		$config = [
-			'items' => $fieldsHtml,
-		];
+		$html = implode( '', $fieldsHtml );
+
 		if ( $sectionName ) {
-			$config['id'] = Sanitizer::escapeIdForAttribute( $sectionName );
-		}
-		if ( is_string( $this->mWrapperLegend ) ) {
-			$config['label'] = $this->mWrapperLegend;
+			$html = Html::rawElement(
+				'div',
+				[ 'id' => Sanitizer::escapeIdForAttribute( $sectionName ) ],
+				$html
+			);
 		}
-		return new OOUI\FieldsetLayout( $config );
+		return $html;
 	}
 
 	/**
@@ -249,9 +249,8 @@ class OOUIHTMLForm extends HTMLForm {
 	}
 
 	public function getBody() {
-		$fieldset = parent::getBody();
-		// FIXME This only works for forms with no subsections
-		if ( $fieldset instanceof OOUI\FieldsetLayout ) {
+		$html = parent::getBody();
+		if ( $this->mHeader || $this->oouiErrors || $this->oouiWarnings ) {
 			$classes = [ 'mw-htmlform-ooui-header' ];
 			if ( $this->oouiErrors ) {
 				$classes[] = 'mw-htmlform-ooui-header-errors';
@@ -259,33 +258,42 @@ class OOUIHTMLForm extends HTMLForm {
 			if ( $this->oouiWarnings ) {
 				$classes[] = 'mw-htmlform-ooui-header-warnings';
 			}
-			if ( $this->mHeader || $this->oouiErrors || $this->oouiWarnings ) {
-				// if there's no header, don't create an (empty) LabelWidget, simply use a placeholder
-				if ( $this->mHeader ) {
-					$element = new OOUI\LabelWidget( [ 'label' => new OOUI\HtmlSnippet( $this->mHeader ) ] );
-				} else {
-					$element = new OOUI\Widget( [] );
-				}
-				$fieldset->addItems( [
-					new OOUI\FieldLayout(
-						$element,
-						[
-							'align' => 'top',
-							'errors' => $this->oouiErrors,
-							'notices' => $this->oouiWarnings,
-							'classes' => $classes,
-						]
-					)
-				], 0 );
+			// if there's no header, don't create an (empty) LabelWidget, simply use a placeholder
+			if ( $this->mHeader ) {
+				$element = new OOUI\LabelWidget( [ 'label' => new OOUI\HtmlSnippet( $this->mHeader ) ] );
+			} else {
+				$element = new OOUI\Widget( [] );
 			}
+			$html = new OOUI\FieldLayout(
+				$element,
+				[
+					'align' => 'top',
+					'errors' => $this->oouiErrors,
+					'notices' => $this->oouiWarnings,
+					'classes' => $classes,
+				]
+			) . $html;
 		}
-		return $fieldset;
+		return $html;
 	}
 
 	public function wrapForm( $html ) {
+		if ( is_string( $this->mWrapperLegend ) ) {
+			$content = new OOUI\FieldsetLayout( [
+				'label' => $this->mWrapperLegend,
+				'items' => [
+					new OOUI\Widget( [
+						'content' => new OOUI\HtmlSnippet( $html )
+					] ),
+				],
+			] );
+		} else {
+			$content = new OOUI\HtmlSnippet( $html );
+		}
+
 		$form = new OOUI\FormLayout( $this->getFormAttributes() + [
 			'classes' => [ 'mw-htmlform', 'mw-htmlform-ooui' ],
-			'content' => new OOUI\HtmlSnippet( $html ),
+			'content' => $content,
 		] );
 
 		// Include a wrapper for style, if requested.
-- 
2.20.1