Page MenuHomePhabricator

HTMLSelectField doesn't use the `default` value when an invalid value is submitted
Open, Needs TriagePublicBUG REPORT

Description

Coming from T213479 it seems like [[ https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/master/includes/specials/SpecialAllMessages.php#95 | default value ]] of HTMLSelectField in SpecialAllMessage is ignoredt when the submitted value (from the URL query) is not present among the options.

Visit http://localhost:8888/w/index.php/Special:AllMessages?lang=zzzzz
Expected: the 'default' value should be selected (en), rather than the first value (aa) if overridden by the URL query value (zz).

Developer notes

I will note that this behavior is the same for OOUI and non-OOUI HTMLForm, and that is has been the same basically forever. But maybe it would make sense to change it.

If you want to change this, I think the way to do it would be to implement an override for HTMLSelectField::loadDataFromRequest() that pretends the value was not submitted if it is not one of the available options.

Event Timeline

Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)
Volker_E renamed this task from HTMLSelectFiled doesn't accept a `default` value to HTMLSelectField doesn't accept a `default` value.Jan 14 2019, 10:49 PM
Volker_E updated the task description. (Show Details)
Volker_E added a subscriber: Jdlrobson.

I don't see how this is the case. When I visit https://www.mediawiki.org/wiki/Special:AllMessages, the default option "en" is selected. The bug T213479 is caused by setting the 'default' to a value that is not actually present among the options, which results in the first option being selected. Can you explain how to reproduce this issue otherwise?

To reproduce the issue try the pseudo code in the description outside Special:AllMessages.
'en' is a valid default but that's being ignored.

My patch for T213479 does exactly what you are proposing and should fix the problem but doesn't fix the problem (try it out!).
I spent 30 minutes debugging this and it seems to be a problem somewhere inside HTMLFormField::getOOUI

Okay, I see now. You're saying that when the submitted value (from the URL query) is not present among the options, then the 'default' value should be selected, rather than the first value. The 'default' value is definitely not ignored, as long as it's not overridden by the URL query value.

I will note that this behavior is the same for OOUI and non-OOUI HTMLForm, and that is has been the same basically forever. But maybe it would make sense to change it.

If you want to change this, I think the way to do it would be to implement an override for HTMLSelectField::loadDataFromRequest() that pretends the value was not submitted if it is not one of the available options.

matmarex renamed this task from HTMLSelectField doesn't accept a `default` value to HTMLSelectField doesn't use the `default` value when an invalid value is submitted.Jan 15 2019, 8:33 PM

I think there's a misunderstanding somewhere here. 'default' => 'en', is a valid default and value in my code but completely ignored
Can you verify you are using this exact code: ?

	$formDescriptor = [
			'lang' => [
				'type' => 'select',
				'name' => 'lang',
				'options' => [
					'aa' => 'arabic',
					'en' => 'English',
					'fr' => 'french',
				],
				'label-message' => 'allmessages-language',
				'default' => 'en',
			],
		];

		$htmlForm = HTMLForm::factory( 'ooui', $formDescriptor, $this->getContext() );
		$htmlForm
			->setMethod( 'get' )
			->prepareForm()
			->displayForm( false );

@Jdlrobson Your example has the option labels and values swapped in the 'options' array. That array, confusingly, maps labels to values (not values to labels). It should be:

				'options' => [
					'arabic' => 'aa',
					'English' => 'en',
					'french' => 'fr',
				],

Then it works the way I described.

Ah... in that case, I guess my code is wrong and this task should be declined?

I think it's a valid issue? The problem is slightly different than you described it originally, but there is a real problem here, as I hope I explained in the comments above.

On Special:AllMessages the parameter limit has the same problem. The drop down menu has values for 20, 50, 100, 250, 500 and 5000. The default is 50. When using a different value in the URL parameter limit like limit=2 (Example: https://www.mediawiki.org/wiki/Special:AllMessages?prefix=&filter=all&lang=en&limit=2) the query on the system messages get correctly limited to 2 entries. In case of a value which is not in the list of the drop down menu the preselected value is always 20 because 20 is the first entry in the list. Expected value is 50 because this is the default value when the parameter limit is not present.

Fomafix changed the subtype of this task from "Task" to "Bug Report".Mar 14 2019, 8:35 PM