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

diff --git a/includes/htmlform/fields/HTMLFormFieldCloner.php b/includes/htmlform/fields/HTMLFormFieldCloner.php
index 09fe1bc..d8de890 100644 (file)
--- 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 5f60097..f9cb5de 100644 (file)
--- 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