From f338a1cf3132805a962909ff789ad55ba4b67f2b Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bartosz=20Dziewo=C5=84ski?= Date: Wed, 24 Dec 2014 18:21:32 +0100 Subject: [PATCH] HTMLForm: Separate VForm code to a subclass MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit …and in general, work with the existing HTMLForm design for defining display formats, rather than against it. Breaking changes: * HTMLForm::isVForm() is now deprecated. * You can no longer do this: $form = new HTMLForm( … ); $form->setDisplayFormat( 'vform' ); // throws exception Instead, do this: $form = HTMLForm::factory( 'vform', … ); When using FormSpecialPage, override the new getDisplayFormat() method instead of calling $form->setDisplayFormat() in the alterForm() method. (Other display formats are not affected, only 'vform'.) Bug fixes: * Correctly suppress empty labels for VForm fields * Correctly disable
wrappers for VForms Other benefits: * Reduce code duplication related to $getFieldHtmlMethod * Introduce HTMLForm::factory() method for constructing HTMLForms Related cleanup: * Correctly style 'reset' buttons in MediaWiki UI mode * Label $wgHTMLFormAllowTableFormat as a mobile-specific hack * Display checkboxes normally in MediaWiki UI mode (removed weird override that only broke things). Also, always render checkboxes in VForms as .mw-ui-checkbox. * self:: → static:: Bug: T85285 Change-Id: I086a84f1c8cc6a16710709b7806c7f5f96462b32 --- autoload.php | 1 + includes/htmlform/HTMLCheckField.php | 31 ++-- includes/htmlform/HTMLForm.php | 167 ++++++++++----------- includes/htmlform/HTMLFormField.php | 17 ++- includes/htmlform/HTMLFormFieldCloner.php | 15 +- includes/htmlform/VFormHTMLForm.php | 140 +++++++++++++++++ includes/specialpage/FormSpecialPage.php | 20 ++- includes/specials/SpecialChangeEmail.php | 6 +- includes/specials/SpecialPageLanguage.php | 6 +- includes/specials/SpecialPasswordReset.php | 11 +- 10 files changed, 275 insertions(+), 139 deletions(-) create mode 100644 includes/htmlform/VFormHTMLForm.php diff --git a/autoload.php b/autoload.php index 72345feaba..46c8b01cc8 100644 --- a/autoload.php +++ b/autoload.php @@ -1269,6 +1269,7 @@ $wgAutoloadLocalClasses = array( 'UsersPager' => __DIR__ . '/includes/specials/SpecialListusers.php', 'UtfNormal' => __DIR__ . '/includes/normal/UtfNormal.php', 'UzConverter' => __DIR__ . '/languages/classes/LanguageUz.php', + 'VFormHTMLForm' => __DIR__ . '/includes/htmlform/VFormHTMLForm.php', 'ValidateRegistrationFile' => __DIR__ . '/maintenance/validateRegistrationFile.php', 'ViewAction' => __DIR__ . '/includes/actions/ViewAction.php', 'VirtualRESTService' => __DIR__ . '/includes/libs/virtualrest/VirtualRESTService.php', diff --git a/includes/htmlform/HTMLCheckField.php b/includes/htmlform/HTMLCheckField.php index 5f70362a5a..e54f748362 100644 --- a/includes/htmlform/HTMLCheckField.php +++ b/includes/htmlform/HTMLCheckField.php @@ -20,28 +20,19 @@ class HTMLCheckField extends HTMLFormField { $attr['class'] = $this->mClass; } - if ( $this->mParent->isVForm() ) { - // Nest checkbox inside label. - return Html::rawElement( 'label', - array( - 'class' => 'mw-ui-checkbox-label' - ), - Xml::check( $this->mName, $value, $attr ) . $this->mLabel ); - } else { - $chkLabel = Xml::check( $this->mName, $value, $attr ) - . ' ' - . Html::rawElement( 'label', array( 'for' => $this->mID ), $this->mLabel ); - - if ( $wgUseMediaWikiUIEverywhere ) { - $chkLabel = Html::rawElement( - 'div', - array( 'class' => 'mw-ui-checkbox' ), - $chkLabel - ); - } + $chkLabel = Xml::check( $this->mName, $value, $attr ) + . ' ' + . Html::rawElement( 'label', array( 'for' => $this->mID ), $this->mLabel ); - return $chkLabel; + if ( $wgUseMediaWikiUIEverywhere || $this->mParent instanceof VFormHTMLForm ) { + $chkLabel = Html::rawElement( + 'div', + array( 'class' => 'mw-ui-checkbox' ), + $chkLabel + ); } + + return $chkLabel; } /** diff --git a/includes/htmlform/HTMLForm.php b/includes/htmlform/HTMLForm.php index dc73522144..908fdf27fb 100644 --- a/includes/htmlform/HTMLForm.php +++ b/includes/htmlform/HTMLForm.php @@ -207,9 +207,40 @@ class HTMLForm extends ContextSource { 'table', 'div', 'raw', + ); + + /** + * Available formats in which to display the form + * @var array + */ + protected $availableSubclassDisplayFormats = array( 'vform', ); + /** + * Construct a HTMLForm object for given display type. May return a HTMLForm subclass. + * + * @throws MWException When the display format requested is not known + * @param string $displayFormat + * @param mixed $arguments... Additional arguments to pass to the constructor. + * @return HTMLForm + */ + public static function factory( $displayFormat/*, $arguments...*/ ) { + $arguments = func_get_args(); + array_shift( $arguments ); + + switch ( $displayFormat ) { + case 'vform': + $reflector = new ReflectionClass( 'VFormHTMLForm' ); + return $reflector->newInstanceArgs( $arguments ); + default: + $reflector = new ReflectionClass( 'HTMLForm' ); + $form = $reflector->newInstanceArgs( $arguments ); + $form->setDisplayFormat( $displayFormat ); + return $form; + } + } + /** * Build a new HTMLForm from an array of field attributes * @@ -233,6 +264,11 @@ class HTMLForm extends ContextSource { $this->mMessagePrefix = $context; } + // Evil hack for mobile :( + if ( !$this->getConfig()->get( 'HTMLFormAllowTableFormat' ) && $this->displayFormat === 'table' ) { + $this->displayFormat = 'div'; + } + // Expand out into a tree. $loadedDescriptor = array(); $this->mFlatFields = array(); @@ -246,12 +282,7 @@ class HTMLForm extends ContextSource { $this->mUseMultipart = true; } - $field = self::loadInputFromParameters( $fieldname, $info, $this ); - - // vform gets too much space if empty labels generate HTML. - if ( $this->isVForm() ) { - $field->setShowEmptyLabel( false ); - } + $field = static::loadInputFromParameters( $fieldname, $info, $this ); $setSection =& $loadedDescriptor; if ( $section ) { @@ -286,10 +317,24 @@ class HTMLForm extends ContextSource { * @return HTMLForm $this for chaining calls (since 1.20) */ public function setDisplayFormat( $format ) { + if ( + in_array( $format, $this->availableSubclassDisplayFormats ) || + in_array( $this->displayFormat, $this->availableSubclassDisplayFormats ) + ) { + throw new MWException( 'Cannot change display format after creation, ' . + 'use HTMLForm::factory() instead' ); + } + if ( !in_array( $format, $this->availableDisplayFormats ) ) { throw new MWException( 'Display format must be one of ' . print_r( $this->availableDisplayFormats, true ) ); } + + // Evil hack for mobile :( + if ( !$this->getConfig()->get( 'HTMLFormAllowTableFormat' ) && $format === 'table' ) { + $format = 'div'; + } + $this->displayFormat = $format; return $this; @@ -301,20 +346,17 @@ class HTMLForm extends ContextSource { * @return string */ public function getDisplayFormat() { - $format = $this->displayFormat; - if ( !$this->getConfig()->get( 'HTMLFormAllowTableFormat' ) && $format === 'table' ) { - $format = 'div'; - } - return $format; + return $this->displayFormat; } /** * Test if displayFormat is 'vform' * @since 1.22 + * @deprecated since 1.25 * @return bool */ public function isVForm() { - return $this->displayFormat === 'vform'; + return false; } /** @@ -337,7 +379,7 @@ class HTMLForm extends ContextSource { if ( isset( $descriptor['class'] ) ) { $class = $descriptor['class']; } elseif ( isset( $descriptor['type'] ) ) { - $class = self::$typeMappings[$descriptor['type']]; + $class = static::$typeMappings[$descriptor['type']]; $descriptor['class'] = $class; } else { $class = null; @@ -362,7 +404,7 @@ class HTMLForm extends ContextSource { * @return HTMLFormField Instance of a subclass of HTMLFormField */ public static function loadInputFromParameters( $fieldname, $descriptor, HTMLForm $parent = null ) { - $class = self::getClassFromDescriptor( $fieldname, $descriptor ); + $class = static::getClassFromDescriptor( $fieldname, $descriptor ); $descriptor['fieldname'] = $fieldname; if ( $parent ) { @@ -790,19 +832,6 @@ class HTMLForm extends ContextSource { # For good measure (it is the default) $this->getOutput()->preventClickjacking(); $this->getOutput()->addModules( 'mediawiki.htmlform' ); - if ( $this->isVForm() ) { - // This is required for VForm HTMLForms that use that style regardless - // of wgUseMediaWikiUIEverywhere (since they pre-date it). - // When wgUseMediaWikiUIEverywhere is removed, this should be consolidated - // with the addModuleStyles in SpecialPage->setHeaders. - $this->getOutput()->addModuleStyles( array( - 'mediawiki.ui', - 'mediawiki.ui.button', - 'mediawiki.ui.input', - ) ); - // @todo Should vertical form set setWrapperLegend( false ) - // to hide ugly fieldsets? - } $html = '' . $this->getErrors( $submitResult ) @@ -818,18 +847,10 @@ class HTMLForm extends ContextSource { } /** - * Wrap the form innards in an actual "
" element - * - * @param string $html HTML contents to wrap. - * - * @return string Wrapped HTML. + * Get HTML attributes for the `` tag. + * @return array */ - function wrapForm( $html ) { - - # Include a
wrapper for style, if requested. - if ( $this->mWrapperLegend !== false ) { - $html = Xml::fieldset( $this->mWrapperLegend, $html ); - } + protected function getFormAttributes() { # Use multipart/form-data $encType = $this->mUseMultipart ? 'multipart/form-data' @@ -844,12 +865,23 @@ class HTMLForm extends ContextSource { if ( !empty( $this->mId ) ) { $attribs['id'] = $this->mId; } + return $attribs; + } - if ( $this->isVForm() ) { - array_push( $attribs['class'], 'mw-ui-vform', 'mw-ui-container' ); + /** + * Wrap the form innards in an actual "" element + * + * @param string $html HTML contents to wrap. + * + * @return string Wrapped HTML. + */ + function wrapForm( $html ) { + # Include a
wrapper for style, if requested. + if ( $this->mWrapperLegend !== false ) { + $html = Xml::fieldset( $this->mWrapperLegend, $html ); } - return Html::rawElement( 'form', $attribs, $html ); + return Html::rawElement( 'form', $this->getFormAttributes(), $html ); } /** @@ -905,21 +937,10 @@ class HTMLForm extends ContextSource { $attribs['class'] = array( 'mw-htmlform-submit' ); - if ( $this->isVForm() || $useMediaWikiUIEverywhere ) { + if ( $useMediaWikiUIEverywhere ) { array_push( $attribs['class'], 'mw-ui-button', $this->mSubmitModifierClass ); } - if ( $this->isVForm() ) { - // mw-ui-block is necessary because the buttons aren't necessarily in an - // immediate child div of the vform. - // @todo Let client specify if the primary submit button is progressive or destructive - array_push( - $attribs['class'], - 'mw-ui-big', - 'mw-ui-block' - ); - } - $buttons .= Xml::submitButton( $this->getSubmitText(), $attribs ) . "\n"; } @@ -928,7 +949,8 @@ class HTMLForm extends ContextSource { 'input', array( 'type' => 'reset', - 'value' => $this->msg( 'htmlform-reset' )->text() + 'value' => $this->msg( 'htmlform-reset' )->text(), + 'class' => ( $useMediaWikiUIEverywhere ? 'mw-ui-button' : null ), ) ) . "\n"; } @@ -948,15 +970,9 @@ class HTMLForm extends ContextSource { $attrs['id'] = $button['id']; } - if ( $this->isVForm() || $useMediaWikiUIEverywhere ) { - if ( isset( $attrs['class'] ) ) { - $attrs['class'] .= ' mw-ui-button'; - } else { - $attrs['class'] = 'mw-ui-button'; - } - if ( $this->isVForm() ) { - $attrs['class'] .= ' mw-ui-big mw-ui-block'; - } + if ( $useMediaWikiUIEverywhere ) { + $attrs['class'] = isset( $attrs['class'] ) ? (array)$attrs['class'] : array(); + $attrs['class'][] = 'mw-ui-button'; } $buttons .= Html::element( 'input', $attrs ) . "\n"; @@ -965,13 +981,6 @@ class HTMLForm extends ContextSource { $html = Html::rawElement( 'span', array( 'class' => 'mw-htmlform-submit-buttons' ), "\n$buttons" ) . "\n"; - // Buttons are top-level form elements in table and div layouts, - // but vform wants all elements inside divs to get spaced-out block - // styling. - if ( $this->mShowSubmit && $this->isVForm() ) { - $html = Html::rawElement( 'div', null, "\n$html" ) . "\n"; - } - return $html; } @@ -1284,20 +1293,8 @@ class HTMLForm extends ContextSource { $subsectionHtml = ''; $hasLabel = false; - switch ( $displayFormat ) { - case 'table': - $getFieldHtmlMethod = 'getTableRow'; - break; - case 'vform': - // Close enough to a div. - $getFieldHtmlMethod = 'getDiv'; - break; - case 'div': - $getFieldHtmlMethod = 'getDiv'; - break; - default: - $getFieldHtmlMethod = 'get' . ucfirst( $displayFormat ); - } + // Conveniently, PHP method names are case-insensitive. + $getFieldHtmlMethod = $displayFormat == 'table' ? 'getTableRow' : ( 'get' . $displayFormat ); foreach ( $fields as $key => $value ) { if ( $value instanceof HTMLFormField ) { @@ -1369,7 +1366,7 @@ class HTMLForm extends ContextSource { $html = Html::rawElement( 'table', $attribs, Html::rawElement( 'tbody', array(), "\n$html\n" ) ) . "\n"; - } elseif ( $displayFormat === 'div' || $displayFormat === 'vform' ) { + } else { $html = Html::rawElement( 'div', $attribs, "\n$html\n" ); } } diff --git a/includes/htmlform/HTMLFormField.php b/includes/htmlform/HTMLFormField.php index a91f331949..a75190075b 100644 --- a/includes/htmlform/HTMLFormField.php +++ b/includes/htmlform/HTMLFormField.php @@ -513,9 +513,6 @@ abstract class HTMLFormField { $inputHtml . "\n$errors" ); $divCssClasses = array( "mw-htmlform-field-$fieldType", $this->mClass, $errorClass ); - if ( $this->mParent->isVForm() ) { - $divCssClasses[] = 'mw-ui-vform-field'; - } $wrapperAttributes = array( 'class' => $divCssClasses, @@ -554,6 +551,20 @@ abstract class HTMLFormField { return $html; } + /** + * Get the complete field for the input, including help text, + * labels, and whatever. Fall back from 'vform' to 'div' when not overridden. + * + * @since 1.25 + * @param string $value The value to set the input to. + * @return string Complete HTML field. + */ + public function getVForm( $value ) { + // Ewwww + $this->mClass .= ' mw-ui-vform-field'; + return $this->getDiv( $value ); + } + /** * Generate help text HTML in table format * @since 1.20 diff --git a/includes/htmlform/HTMLFormFieldCloner.php b/includes/htmlform/HTMLFormFieldCloner.php index d1b7746c1e..b06f10d573 100644 --- a/includes/htmlform/HTMLFormFieldCloner.php +++ b/includes/htmlform/HTMLFormFieldCloner.php @@ -262,17 +262,8 @@ class HTMLFormFieldCloner extends HTMLFormField { ? $this->mParams['format'] : $this->mParent->getDisplayFormat(); - switch ( $displayFormat ) { - case 'table': - $getFieldHtmlMethod = 'getTableRow'; - break; - case 'vform': - // Close enough to a div. - $getFieldHtmlMethod = 'getDiv'; - break; - default: - $getFieldHtmlMethod = 'get' . ucfirst( $displayFormat ); - } + // Conveniently, PHP method names are case-insensitive. + $getFieldHtmlMethod = $displayFormat == 'table' ? 'getTableRow' : ( 'get' . $displayFormat ); $html = ''; $hidden = ''; @@ -336,7 +327,7 @@ class HTMLFormFieldCloner extends HTMLFormField { $html = Html::rawElement( 'table', $attribs, Html::rawElement( 'tbody', array(), "\n$html\n" ) ) . "\n"; - } elseif ( $displayFormat === 'div' || $displayFormat === 'vform' ) { + } else { $html = Html::rawElement( 'div', $attribs, "\n$html\n" ); } } diff --git a/includes/htmlform/VFormHTMLForm.php b/includes/htmlform/VFormHTMLForm.php new file mode 100644 index 0000000000..7826a0c79d --- /dev/null +++ b/includes/htmlform/VFormHTMLForm.php @@ -0,0 +1,140 @@ +setShowEmptyLabel( false ); + return $field; + } + + function getHTML( $submitResult ) { + // This is required for VForm HTMLForms that use that style regardless + // of wgUseMediaWikiUIEverywhere (since they pre-date it). + // When wgUseMediaWikiUIEverywhere is removed, this should be consolidated + // with the addModuleStyles in SpecialPage->setHeaders. + $this->getOutput()->addModuleStyles( array( + 'mediawiki.ui', + 'mediawiki.ui.button', + 'mediawiki.ui.input', + 'mediawiki.ui.checkbox', + ) ); + + return parent::getHTML( $submitResult ); + } + + protected function getFormAttributes() { + $attribs = parent::getFormAttributes(); + array_push( $attribs['class'], 'mw-ui-vform', 'mw-ui-container' ); + return $attribs; + } + + function wrapForm( $html ) { + // Always discard $this->mWrapperLegend + return Html::rawElement( 'form', $this->getFormAttributes(), $html ); + } + + function getButtons() { + $buttons = ''; + + if ( $this->mShowSubmit ) { + $attribs = array(); + + if ( isset( $this->mSubmitID ) ) { + $attribs['id'] = $this->mSubmitID; + } + + if ( isset( $this->mSubmitName ) ) { + $attribs['name'] = $this->mSubmitName; + } + + if ( isset( $this->mSubmitTooltip ) ) { + $attribs += Linker::tooltipAndAccesskeyAttribs( $this->mSubmitTooltip ); + } + + $attribs['class'] = array( + 'mw-htmlform-submit', + 'mw-ui-button mw-ui-big mw-ui-block', + $this->mSubmitModifierClass, + ); + + $buttons .= Xml::submitButton( $this->getSubmitText(), $attribs ) . "\n"; + } + + if ( $this->mShowReset ) { + $buttons .= Html::element( + 'input', + array( + 'type' => 'reset', + 'value' => $this->msg( 'htmlform-reset' )->text(), + 'class' => 'mw-ui-button mw-ui-big mw-ui-block', + ) + ) . "\n"; + } + + foreach ( $this->mButtons as $button ) { + $attrs = array( + 'type' => 'submit', + 'name' => $button['name'], + 'value' => $button['value'] + ); + + if ( $button['attribs'] ) { + $attrs += $button['attribs']; + } + + if ( isset( $button['id'] ) ) { + $attrs['id'] = $button['id']; + } + + $attrs['class'] = isset( $attrs['class'] ) ? (array)$attrs['class'] : array(); + $attrs['class'][] = 'mw-ui-button mw-ui-big mw-ui-block'; + + $buttons .= Html::element( 'input', $attrs ) . "\n"; + } + + $html = Html::rawElement( 'div', + array( 'class' => 'mw-htmlform-submit-buttons' ), "\n$buttons" ) . "\n"; + + return $html; + } +} diff --git a/includes/specialpage/FormSpecialPage.php b/includes/specialpage/FormSpecialPage.php index fd3bc7b8da..f727c05381 100644 --- a/includes/specialpage/FormSpecialPage.php +++ b/includes/specialpage/FormSpecialPage.php @@ -74,6 +74,16 @@ abstract class FormSpecialPage extends SpecialPage { return strtolower( $this->getName() ); } + /** + * Get display format for the form. See HTMLForm documentation for available values. + * + * @since 1.25 + * @return string + */ + protected function getDisplayFormat() { + return 'table'; + } + /** * Get the HTMLForm to control behavior * @return HTMLForm|null @@ -81,15 +91,9 @@ abstract class FormSpecialPage extends SpecialPage { protected function getForm() { $this->fields = $this->getFormFields(); - $form = new HTMLForm( $this->fields, $this->getContext(), $this->getMessagePrefix() ); + $form = HTMLForm::factory( $this->getDisplayFormat(), $this->fields, $this->getContext(), $this->getMessagePrefix() ); $form->setSubmitCallback( array( $this, 'onSubmit' ) ); - // If the form is a compact vertical form, then don't output this ugly - // fieldset surrounding it. - // XXX Special pages can setDisplayFormat to 'vform' in alterForm(), but that - // is called after this. - if ( !$form->isVForm() ) { - $form->setWrapperLegendMsg( $this->getMessagePrefix() . '-legend' ); - } + $form->setWrapperLegendMsg( $this->getMessagePrefix() . '-legend' ); $headerMsg = $this->msg( $this->getMessagePrefix() . '-text' ); if ( !$headerMsg->isDisabled() ) { diff --git a/includes/specials/SpecialChangeEmail.php b/includes/specials/SpecialChangeEmail.php index 06ede617de..674cbc8cee 100644 --- a/includes/specials/SpecialChangeEmail.php +++ b/includes/specials/SpecialChangeEmail.php @@ -106,11 +106,13 @@ class SpecialChangeEmail extends FormSpecialPage { return $fields; } + protected function getDisplayFormat() { + return 'vform'; + } + protected function alterForm( HTMLForm $form ) { - $form->setDisplayFormat( 'vform' ); $form->setId( 'mw-changeemail-form' ); $form->setTableId( 'mw-changeemail-table' ); - $form->setWrapperLegend( false ); $form->setSubmitTextMsg( 'changeemail-submit' ); $form->addHiddenFields( $this->getRequest()->getValues( 'returnto', 'returntoquery' ) ); } diff --git a/includes/specials/SpecialPageLanguage.php b/includes/specials/SpecialPageLanguage.php index 52c2460d45..79b2444ea9 100644 --- a/includes/specials/SpecialPageLanguage.php +++ b/includes/specials/SpecialPageLanguage.php @@ -90,9 +90,11 @@ class SpecialPageLanguage extends FormSpecialPage { return $this->showLogFragment( $this->par ); } + protected function getDisplayFormat() { + return 'vform'; + } + public function alterForm( HTMLForm $form ) { - $form->setDisplayFormat( 'vform' ); - $form->setWrapperLegend( false ); Hooks::run( 'LanguageSelector', array( $this->getOutput(), 'mw-languageselector' ) ); } diff --git a/includes/specials/SpecialPasswordReset.php b/includes/specials/SpecialPasswordReset.php index 6ee3290c76..a2dc2add28 100644 --- a/includes/specials/SpecialPasswordReset.php +++ b/includes/specials/SpecialPasswordReset.php @@ -103,16 +103,13 @@ class SpecialPasswordReset extends FormSpecialPage { return $a; } + protected function getDisplayFormat() { + return 'vform'; + } + public function alterForm( HTMLForm $form ) { $resetRoutes = $this->getConfig()->get( 'PasswordResetRoutes' ); - $form->setDisplayFormat( 'vform' ); - // Turn the old-school line around the form off. - // XXX This wouldn't be necessary here if we could set the format of - // the HTMLForm to 'vform' at its creation, but there's no way to do so - // from a FormSpecialPage class. - $form->setWrapperLegend( false ); - $form->addHiddenFields( $this->getRequest()->getValues( 'returnto', 'returntoquery' ) ); $i = 0; -- 2.20.1