From 5d5dbc83da4bc339e6c2f7fb3d346ba259192b68 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Bartosz=20Dziewo=C5=84ski?= <matma.rex@gmail.com>
Date: Mon, 19 Sep 2016 19:03:19 +0200
Subject: [PATCH] mw.htmlform: Fields hidden with 'hide-if' should be disabled

Otherwise, in browsers that support HTML5 form validation, it
would be impossible to submit the form if a field failing
validation was hidden (e.g. because it's marked as required).

Note that disabled fields are not submitted with the form. This
should be okay, as server-side validation ignores them entirely.
(I37e50799ba1f0e0e64a197818b58444f5b056bf0 fixes a corner case.)

For cloner fields, 'hide-if' rules of the parent are now copied
to the children, to make handling such nested fields simpler.

Bug: T145440
Change-Id: I81d04dca6cbb499a15828fd33b01746b68c694da
---
 .../htmlform/fields/HTMLFormFieldCloner.php   | 11 ++++
 resources/src/mediawiki/htmlform/hide-if.js   | 62 ++++++++++++++++---
 2 files changed, 64 insertions(+), 9 deletions(-)

diff --git a/includes/htmlform/fields/HTMLFormFieldCloner.php b/includes/htmlform/fields/HTMLFormFieldCloner.php
index 09fe1bc6ab..d8de8907fc 100644
--- a/includes/htmlform/fields/HTMLFormFieldCloner.php
+++ b/includes/htmlform/fields/HTMLFormFieldCloner.php
@@ -96,6 +96,17 @@ class HTMLFormFieldCloner extends HTMLFormField {
 			} else {
 				$info['id'] = Sanitizer::escapeId( "{$this->mID}--$key--$fieldname" );
 			}
+			// Copy the hide-if rules to "child" fields, so that the JavaScript code handling them
+			// (resources/src/mediawiki/htmlform/hide-if.js) doesn't have to handle nested fields.
+			if ( $this->mHideIf ) {
+				if ( isset( $info['hide-if'] ) ) {
+					// Hide child field if either its rules say it's hidden, or parent's rules say it's hidden
+					$info['hide-if'] = [ 'OR', $info['hide-if'], $this->mHideIf ];
+				} else {
+					// Hide child field if parent's rules say it's hidden
+					$info['hide-if'] = $this->mHideIf;
+				}
+			}
 			$field = HTMLForm::loadInputFromParameters( $name, $info, $this->mParent );
 			$fields[$fieldname] = $field;
 		}
diff --git a/resources/src/mediawiki/htmlform/hide-if.js b/resources/src/mediawiki/htmlform/hide-if.js
index 5f6009720a..f9cb5de685 100644
--- a/resources/src/mediawiki/htmlform/hide-if.js
+++ b/resources/src/mediawiki/htmlform/hide-if.js
@@ -201,22 +201,31 @@
 	}
 
 	mw.hook( 'htmlform.enhance' ).add( function ( $root ) {
-		$root.find( '.mw-htmlform-hide-if' ).each( function () {
-			var v, i, fields, test, func, spec, self, modules, data, extraModules,
-				$el = $( this );
-
+		var
+			$fields = $root.find( '.mw-htmlform-hide-if' ),
+			$oouiFields = $fields.filter( '[data-ooui]' ),
 			modules = [];
-			if ( $el.is( '[data-ooui]' ) ) {
-				modules.push( 'mediawiki.htmlform.ooui' );
+
+		if ( $oouiFields.length ) {
+			modules.push( 'mediawiki.htmlform.ooui' );
+			$oouiFields.each( function () {
+				var data, extraModules,
+					$el = $( this );
+
 				data = $el.data( 'mw-modules' );
 				if ( data ) {
 					// We can trust this value, 'data-mw-*' attributes are banned from user content in Sanitizer
 					extraModules = data.split( ',' );
 					modules.push.apply( modules, extraModules );
 				}
-			}
+			} );
+		}
+
+		mw.loader.using( modules ).done( function () {
+			$fields.each( function () {
+				var v, i, fields, test, func, spec, self,
+					$el = $( this );
 
-			mw.loader.using( modules ).done( function () {
 				if ( $el.is( '[data-ooui]' ) ) {
 					// self should be a FieldLayout that mixes in mw.htmlform.Element
 					self = OO.ui.FieldLayout.static.infuse( $el );
@@ -237,7 +246,42 @@
 				test = v[ 1 ];
 				// The .toggle() method works mostly the same for jQuery objects and OO.ui.Widget
 				func = function () {
-					self.toggle( !test() );
+					var shouldHide = test();
+					self.toggle( !shouldHide );
+
+					// It is impossible to submit a form with hidden fields failing validation, e.g. one that
+					// is required. However, validity is not checked for disabled fields, as these are not
+					// submitted with the form. So we should also disable fields when hiding them.
+					if ( self instanceof jQuery ) {
+						// This also finds elements inside any nested fields (in case of HTMLFormFieldCloner),
+						// which is problematic. But it works because:
+						// * HTMLFormFieldCloner::createFieldsForKey() copies 'hide-if' rules to nested fields
+						// * jQuery collections like $fields are in document order, so we register event
+						//   handlers for parents first
+						// * Event handlers are fired in the order they were registered, so even if the handler
+						//   for parent messed up the child, the handle for child will run next and fix it
+						self.find( 'input, textarea, select' ).each( function () {
+							var $this = $( this );
+							if ( shouldHide ) {
+								if ( $this.data( 'was-disabled' ) === undefined ) {
+									$this.data( 'was-disabled', $this.prop( 'disabled' ) );
+								}
+								$this.prop( 'disabled', true );
+							} else {
+								$this.prop( 'disabled', $this.data( 'was-disabled' ) );
+							}
+						} );
+					} else {
+						// self is a OO.ui.FieldLayout
+						if ( shouldHide ) {
+							if ( self.wasDisabled === undefined ) {
+								self.wasDisabled = self.fieldWidget.isDisabled();
+							}
+							self.fieldWidget.setDisabled( false );
+						} else if ( self.wasDisabled !== undefined ) {
+							self.fieldWidget.setDisabled( self.wasDisabled );
+						}
+					}
 				};
 				for ( i = 0; i < fields.length; i++ ) {
 					// The .on() method works mostly the same for jQuery objects and OO.ui.Widget
-- 
2.20.1