Select Bug When using $form->error?

[Midnight Modding]

Seems about definite this is a bug, but I am testing on 4.4.2, so maybe it was already known about and fixed?

When putting a Form\Select on a form and having its default be some value and submitting the form causes a $form->error, when the form is then shown again on the page with the error message above it, the select no longer has the default choice selected.

And no, I did not change what was selected when submitting the form. I left it totally unchanged, so both the default AND my selected choice were the same, but on the next page with the form error it has a totally different choice selected by default (the first choice in the select).

I confirmed by var dumping that the default should still be the same as originally, ie nothing changed on my end between submitting the form and arriving back at the form again.

I assume this is another of the issues where it loses the form value itself. Like when I tested and found that Number fields wouldn't keep their values either. I am guessing this was fixed later on or will be, though? In the past it was very inconsistent between input types, with some carrying their values over to the error page and some not.

[A Zayed]

4 hours ago, Midnight Modding said:

Seems about definite this is a bug, but I am testing on 4.4.2, so maybe it was already known about and fixed?

When putting a Form\Select on a form and having its default be some value and submitting the form causes a $form->error, when the form is then shown again on the page with the error message above it, the select no longer has the default choice selected.

And no, I did not change what was selected when submitting the form. I left it totally unchanged, so both the default AND my selected choice were the same, but on the next page with the form error it has a totally different choice selected by default (the first choice in the select).

I confirmed by var dumping that the default should still be the same as originally, ie nothing changed on my end between submitting the form and arriving back at the form again.

I assume this is another of the issues where it loses the form value itself. Like when I tested and found that Number fields wouldn't keep their values either. I am guessing this was fixed later on or will be, though? In the past it was very inconsistent between input types, with some carrying their values over to the error page and some not.

I'm having the same issue with 4.4.6 as well, with select field also, whatever value you select - Or even not select - unset the default value after form submit.

[Midnight Modding]

15 minutes ago, A Zayed said:

I'm having the same issue with 4.4.6 as well, with select field also, whatever value you select - Or even not select - unset the default value after form submit.

ok thanks. I tested about every form field type in the past, but I guess I just flat forgot on this type. Some types would carry the user's values over and some wouldn't. (I guess technically it's not supposed to have the original default, but the default should be whatever the user left it at).

Also I recall when setting different field types to disabled, some would still have values on the next page and some wouldn't. I try to just avoid it when I can. Eh on this example Select I can change my check to be done in the Select's validation check instead of the $form->values() check and solve this, though, I guess.

[CodingJungle]

maybe i'm misunderstanding the problem, but it appears to be preserving the values and throwing the errors likes it is suppose to:

 

 

[A Zayed]

1 hour ago, CodingJungle said:

maybe i'm misunderstanding the problem, but it appears to be preserving the values and throwing the errors likes it is suppose to:

 

 

Could you try it without $form->error and without redirect after form submit

[CodingJungle]

i wasn't entirely sure what you meant "without redirect after form submit", so if you could clear that up, i'll see if i can duplicate it and test it out.

 

[Midnight Modding]

4 hours ago, CodingJungle said:

maybe i'm misunderstanding the problem, but it appears to be preserving the values and throwing the errors likes it is suppose to:

 

 

Thanks for testing. Maybe it got fixed some time after 4.4.2. I'll check it out later. I just wasn't wanting to update that particular install to 4.4.6 yet.

I thought maybe yours having the separate validation check for the specific element may have affected it, but I doubt that because it's not for your select input, so I've got to assume it's fixed.

I actually changed it to a radio now, for other reasons, so I will test both a Select and Radio and see what happens on 4.4.2 and 4.4.6 and if there is some issue still I'll post in here which situations (and code) still has an issue. All I did, though, was a typical Select, if $form->error is set then it doesn't do any work or a redirect.

[CodingJungle]

3 hours ago, Midnight Modding said:

Thanks for testing. Maybe it got fixed some time after 4.4.2. I'll check it out later. I just wasn't wanting to update that particular install to 4.4.6 yet.

I thought maybe yours having the separate validation check for the specific element may have affected it, but I doubt that because it's not for your select input, so I've got to assume it's fixed.

I actually changed it to a radio now, for other reasons, so I will test both a Select and Radio and see what happens on 4.4.2 and 4.4.6 and if there is some issue still I'll post in here which situations (and code) still has an issue. All I did, though, was a typical Select, if $form->error is set then it doesn't do any work or a redirect.

i compared system/Helpers/Form/Form.php, system/Helpers/Form/FormAbstract.php and system/Helpers/Form/Select.php from IPS 4.3 and IPS 4.4.6, which there is almost a year and a half of time between releases on and none of these classes have changed much. in Form.php it is mostly formatting and documentation changes, the methods and their bodies are virtually identical. same with Select.php, there is one place where they went with calling the parent method vs a call_user_func, but other than that it is mostly formatting changes, like the one file would have a space where the other wouldn't or vice versa. FormAbstract.php had 2 changes other than formatting, one was for the label, they were calling a different property (in 443 it was $this->name, in 446 they do a null operator $this->htmlId ?? $this->name instead) and on validation instead of doing a call to call_user_func, they assign the validation callback to a local variable and call it as a closure. so i don't think it is a bug that was fixed as there appears to be nothing changed in the code that is remotely relevant. 

so i don't know what was going on, other than if it was a form in a dialog, with a bunch of toggles and a tabs, that some time causes the form to stop functioning in my experience on a bad submit and requires a page refresh to correct. 

[Midnight Modding]

34 minutes ago, CodingJungle said:

i compared system/Helpers/Form/Form.php, system/Helpers/Form/FormAbstract.php and system/Helpers/Form/Select.php from IPS 4.3 and IPS 4.4.6, which there is almost a year and a half of time between releases on and none of these classes have changed much. in Form.php it is mostly formatting and documentation changes, the methods and their bodies are virtually identical. same with Select.php, there is one place where they went with calling the parent method vs a call_user_func, but other than that it is mostly formatting changes, like the one file would have a space where the other wouldn't or vice versa. FormAbstract.php had 2 changes other than formatting, one was for the label, they were calling a different property (in 443 it was $this->name, in 446 they do a null operator $this->htmlId ?? $this->name instead) and on validation instead of doing a call to call_user_func, they assign the validation callback to a local variable and call it as a closure. so i don't think it is a bug that was fixed as there appears to be nothing changed in the code that is remotely relevant. 

so i don't know what was going on, other than if it was a form in a dialog, with a bunch of toggles and a tabs, that some time causes the form to stop functioning in my experience on a bad submit and requires a page refresh to correct. 

It's a form with that lone input. Nothing else other than one Select. So I don't know, but it does it every time and in the past it was losing values as well (hadn't tested that aspect since probably 4.3.6, though). The one it keeps switching it back to is the first choice (with a key of 0). Speaking of page refreshing, though, I could see if that affects it, just where I'll know.

edit: it doesn't lose the value for the Radio. I am testing some things.

[CodingJungle]

 

[Midnight Modding]

I don't know, but it's happening, whatever the reason.

		$form->add( new \IPS\Helpers\Form\Select( 'pickemsx_winner_team', $done ? $winner : 0, TRUE, array( 'options' => array( 0 => \IPS\Member::loggedIn()->language()->addToStack('pickem_game_nobody'), -1 => \IPS\Member::loggedIn()->language()->addToStack('pickem_game_nobody_' . \IPS\Settings::i()->pickemsx_tie_type), $game->team_one_id => \IPS\Member::loggedIn()->language()->addToStack( 'pickemsx_team_' . $game->team_one_id ), $game->team_two_id => \IPS\Member::loggedIn()->language()->addToStack( 'pickemsx_team_' . $game->team_two_id ) ), 'multiple' => FALSE ) ) );

		if( $values = $form->values() )
		{
			// the winner
			$winningTeam = (int) $values['pickemsx_winner_team'];

			// if undoing, was it changed?
			if( $done && $winningTeam === $winner )
			{
				$form->error = \IPS\Member::loggedIn()->language()->addToStack( 'pickem_unchanged_error' );
			}

...

beyond that … there is no other code within the $values block, since it met the conditions to cause the $form->error. Beyond the end of that $values block is no redirect. Why the bug happens, I don't know, but all I did was a simple select with options in it, nothing fancy. For what it's worth $done is TRUE and $winner is one of those team ids. And once the form loads again with the error those are the same values as beforehand, but the default option in the select is then different.

And when keeping that exact same code, no changes except changing Seletc to Radio it does not lose the values anymore. So it's definitely an issue with Select.

It is on the front end, not acp, but I don't really think that would matter, since the form helpers are the same, although every once in a while IPS does something that could slightly change something based on location (usually just the template).

[CodingJungle]

i'm gonna say its prolly something in your ternary for the default value that is bucking it and btw for your options you don't have to do \IPS\Member::loggedIn()->language()->addToStack() to your options values, you can pass it lang strings it should parse them out for you.

[Midnight Modding]

1 hour ago, CodingJungle said:

i'm gonna say its prolly something in your ternary for the default value that is bucking it and btw for your options you don't have to do \IPS\Member::loggedIn()->language()->addToStack() to your options values, you can pass it lang strings it should parse them out for you.

thanks, I had a feeling the latter was the case, but didn't ever try it. As for the first part, like I said I already var dumped on the screen to confirm those were correct both on the first form load and the load with $form->error set and I also had this same issue happen in the past and someone else in here is apparently having the issue, as well. I can switch it to a definite default there and I guarantee it's still going to have the issue because I remember so many of these same things happening in the past when I tested. Not sure why you're not able to reproduce it, but that's how a lot of my 3.x bug reports went and eventually they were reproduced. I'd be very surprised if this one is on my end, with such simple code involved.

Also, shouldn't the default be irrelevant, anyway, since it's a select, so on the next page they will have their own selection made in it, overriding the default.

[newbie LAC]

 

1 hour ago, Midnight Modding said:

$form->add( new \IPS\Helpers\Form\Select( 'pickemsx_winner_team', $done ? $winner : 0, TRUE, array( 'options' => array( 0 => \IPS\Member::loggedIn()->language()->addToStack('pickem_game_nobody'), -1 => \IPS\Member::loggedIn()->language()->addToStack('pickem_game_nobody_' . \IPS\Settings::i()->pickemsx_tie_type), $game->team_one_id => \IPS\Member::loggedIn()->language()->addToStack( 'pickemsx_team_' . $game->team_one_id ), $game->team_two_id => \IPS\Member::loggedIn()->language()->addToStack( 'pickemsx_team_' . $game->team_two_id ) ), 'multiple' => FALSE ) ) );

You can't use integers values as keys options

1 hour ago, CodingJungle said:

 

Try on front side

On admin side works because uses the template applications\core\dev\html\admin\forms\select.phtml

On front side uses \applications\core\dev\html\front\forms\select.phtml

admin

{{if ( ( $value === 0 and $k === 0 ) or ( $value !== 0 and $value === $k ) or ( $value !== 0 and \is_numeric( $value ) and \is_numeric( $k ) and $value == $k ) ) or ( \is_array( $value ) and \in_array( $k, $value ) ) or ( !empty( $userSuppliedInput ) and !\in_array( $value, array_keys( $options ) ) and $k == $userSuppliedInput )}}selected{{endif}}

vs

front

{{if ( ( $value === 0 and $k === 0 ) or ( $value !== 0 and $value === $k ) ) or ( \is_array( $value ) and \in_array( $k, $value ) )}}selected{{endif}}

 

[Midnight Modding]

well, notice I did say at one point it could be a template difference between front end and admin end. Also, to me, it's a bug because of course people need to sometimes use integers as keys. (of course they probably had a reason for their own code to do that, so maybe not a bug technically, but it does affect a lot of situations).

edit: nvm, I thought I could change them to strings initially to work around, but that doesn't work. I didn't read the whole if statements there and it's not worth the time to figre it out at this moment, especially sicne I changed it to radio.

Thanks for confirming it, though. I always end up somehow running into issues that aren't easy to reproduce. I did that a lot in 3.x. lol.

[CodingJungle]

47 minutes ago, newbie LAC said:

Try on front side

On admin side works because uses the template applications\core\dev\html\admin\forms\select.phtml

On front side uses \applications\core\dev\html\front\forms\select.phtml

i'm in a bit of a stunned state atm, as i'm gonna have to agree with MM on this one, that this is a bug and not intended. if it was intended, then it is poorly implemented on IPS part and makes zero sense, as the video below will demonstrate, cause you can have numerical values for options values.

 

[Midnight Modding]

off topic, but look what fun fun fun I just had testing features. For each of these scenarios I had to check 5 database tables to be sure a ton of stats updated properly, ie no bug in my stats keeping. I always like keeping detailed stats. And every one of these worked properly the first time, so a ton of time I could have kept from spending, but I obviously had to be sure I didn't find issues...

*team to team change
*team to canceled
*canceled to team
*team to canceled via tie disabled
*canceled to team via tie disabled
*canceled via regular to canceled via tie
*canceled via tie to canceled via regular
*team to tie
*tie to team
*tie to canceled
*canceled to tie
*win
*loss via team
*no win when no pick for canceled
*skipped via no pick
*canceled canceled
*canceled tie
*win on tie
*loss on tie

note: I had those typed out as a checklist. I would have been really lame to type them only for a post. lol.

[Midnight Modding]

13 minutes ago, CodingJungle said:

i'm in a bit of a stunned state atm, as i'm gonna have to agree with MM on this one, that this is a bug and not intended. if it was intended, then it is poorly implemented on IPS part and makes zero sense, as the video below will demonstrate, cause you can have numerical values for options values.

 

lol. well I've always been good about finding bugs and usually ones where they were first marked cannot reproduce.

On this one I thought it "could" be working as intended, since I don't know every situation where IPS used selects, but I'd doubt it. Also, though, if they never use integers as keys in first party apps, they still could technically say it's not a bug. But either way at this moment I can't think of why it would be done that way.

[CodingJungle]

1 minute ago, Midnight Modding said:

lol. well I've always been good about finding bugs and usually ones where they were first marked cannot reproduce.

 

well a broke clock is right twice a day after all.

2 minutes ago, Midnight Modding said:

On this one I thought it "could" be working as intended, since I don't know every situation where IPS used selects, but I'd doubt it. Also, though, if they never use integers as keys in first party apps, they still could technically say it's not a bug. But either way at this moment I can't think of why it would be done that way.

the reason i think it is a bug, cause it will return the numerical value for a select, store it in the db, etc and it lets you set the default to be numerical at first load. this problem only presents itself on form error or if you don't do a redirect after the form is submitted.  if they intended it, then there are a lot better ways to throw an error to the developer to let them know int values aren't allowed on selects on the front end. 

[Midnight Modding]

1 minute ago, CodingJungle said:

well a broke clock is right twice a day after all.

the reason i think it is a bug, cause it will return the numerical value for a select, store it in the db, etc and it lets you set the default to be numerical at first load. this problem only presents itself on form error or if you don't do a redirect after the form is submitted.  if they intended it, then there are a lot better ways to throw an error to the developer to let them know int values aren't allowed on selects on the front end. 

yeah good point (on the latter... as for the former, I am usually right on bugs :p ). On the first load, it's always right. I "think" I recall Number being one where the value wouldn't carry over, but that may have been only when it's set to disabled.

So I guess if I did want to work around, I'd use 'something_1' instead of 1 and then pull it back apart after submitting. I thought I could just use '1', but it still had the issue (I haven't looked at that check closely yet, but would have thought it would be ok because it would still be a string at time of checking).

[CodingJungle]

15 minutes ago, Midnight Modding said:

So I guess if I did want to work around, I'd use 'something_1' instead of 1 and then pull it back apart after submitting. I thought I could just use '1', but it still had the issue (I haven't looked at that check closely yet, but would have thought it would be ok because it would still be a string at time of checking).

prolly the only way, i don't see IPS busting down doors to fix this one, as its very edge case imho and not a show stopper really. 

 

[Midnight Modding]

24 minutes ago, CodingJungle said:

prolly the only way, i don't see IPS busting down doors to fix this one, as its very edge case imho and not a show stopper really. 

 

well, I agree there. Would be a rare situation where it would really cause a problem.

[A Zayed]

3 hours ago, newbie LAC said:

 

You can't use integers values as keys options

Try on front side

On admin side works because uses the template applications\core\dev\html\admin\forms\select.phtml

On front side uses \applications\core\dev\html\front\forms\select.phtml

admin

{{if ( ( $value === 0 and $k === 0 ) or ( $value !== 0 and $value === $k ) or ( $value !== 0 and \is_numeric( $value ) and \is_numeric( $k ) and $value == $k ) ) or ( \is_array( $value ) and \in_array( $k, $value ) ) or ( !empty( $userSuppliedInput ) and !\in_array( $value, array_keys( $options ) ) and $k == $userSuppliedInput )}}selected{{endif}}

vs

front

{{if ( ( $value === 0 and $k === 0 ) or ( $value !== 0 and $value === $k ) ) or ( \is_array( $value ) and \in_array( $k, $value ) )}}selected{{endif}}

 

I'm really amazed! Why IPS did this difference?! I never expected this.

As a quick fix - for my very specific case that I have to use integer values as option keys - I made a hook to replace this template contents.

HOOK FILE:

public static function hookData() 
{
	return parent::hookData();
}

public function select( $name, $value, $required, $options, $multiple=FALSE, $class='', $disabled=FALSE, $toggles=array(), $id=NULL, $unlimited=NULL, $unlimitedLang='all', $unlimitedToggles=array(), $toggleOn=TRUE, $userSuppliedInput='', $sort=FALSE )
{
  	$output = \IPS\Theme::i()->getTemplate( 'misc', 'myApp', 'front' )->mcSelect( $name, $value, $required, $options, $multiple, $class, $disabled, $toggles, $id, $unlimited, $unlimitedLang, $unlimitedToggles, $toggleOn, $userSuppliedInput, $sort );			

  	return $output;
}

TEMPLATE FILE:

<ips:template parameters="$name, $value, $required, $options, $multiple=FALSE, $class='', $disabled=FALSE, $toggles=array(), $id=NULL, $unlimited=NULL, $unlimitedLang='all', $unlimitedToggles=array(), $toggleOn=TRUE, $userSuppliedInput='', $sort=FALSE" />
{{if $multiple}}
	<input type="hidden" name="{$name}" value="__EMPTY">
{{endif}}
<select name="{$name}" class="{$class}" {{if $multiple}}multiple{{endif}} {{if $required === TRUE}}required aria-required='true'{{endif}} {{if $disabled === TRUE}}disabled aria-disabled='true'{{endif}} {{if $id !== NULL}}id="elSelect_{$id}"{{endif}} {{if $sort}}data-sort{{endif}}>
	{{foreach $options as $k => $v}}
		{{if \is_array( $v )}}
			<optgroup label="{lang="$k"}">
				{{foreach $v as $_k => $_v}}
					<option value='{$_k}' {{if ( ( $value === 0 and $_k === 0 ) or ( $value !== 0 and $value === $_k ) ) or ( \is_array( $value ) and \in_array( $_k, $value ) )}}selected{{endif}} {{if isset( $toggles[ $_k ] )}}data-control="toggle" data-toggles="{expression="implode( ',', $toggles[ $_k ] )"}" aria-controls="{expression="implode( ',', $toggles[ $_k ] )"}"{{endif}}>{$_v|raw}</option>
				{{endforeach}}
			</optgroup>
		{{else}}
			<option value='{$k}' {{if ( ( $value === 0 and $k === 0 ) or ( $value !== 0 and $value === $k ) or ( $value !== 0 and \is_numeric( $value ) and \is_numeric( $k ) and $value == $k )) or ( \is_array( $value ) and \in_array( $k, $value ) )}}selected{{endif}} {{if \is_array( $disabled ) and \in_array( $k, $disabled )}}disabled{{endif}} {{if isset( $toggles[ $k ] )}}data-control="toggle" data-toggles="{expression="implode( ',', $toggles[ $k ] )"}"{{endif}}>{$v|raw}</option>
		{{endif}}
	{{endforeach}}
</select>
{{if $unlimited !== NULL}}
	<br><br>
	{lang="or"}
	&nbsp;
	<span class='ipsCustomInput'>
		<input type="checkbox" data-control="unlimited{{if \count($unlimitedToggles)}} toggle{{endif}}" name="{expression="trim( $name, '[]' )"}_unlimited" id='{expression="trim( $id ?: $name, '[]' )"}_unlimited' value="{$unlimited}" {{if $unlimited === $value}}checked aria-checked='true'{{endif}} {{if $disabled === TRUE}}disabled aria-disabled='true'{{endif}} {{if \count( $unlimitedToggles )}}{{if $toggleOn === FALSE}}data-togglesOff{{else}}data-togglesOn{{endif}}="{expression="implode( ',', $unlimitedToggles )"}" aria-controls="{expression="implode( ',', $unlimitedToggles )"}"{{endif}} aria-labelledby='{expression="trim( $id ?: $name, '[]' )"}_unlimited_label'>
		<span></span>
	</span> <label for='{expression="trim( $id ?: $name, '[]' )"}_unlimited' id='{expression="trim( $id ?: $name, '[]' )"}_unlimited_label' class='ipsField_unlimited'>{lang="$unlimitedLang"}</label>
{{endif}}

 

Hint: this is not a public app, I'm using it for my own dev. purposes.

[bfarber]

If you find anywhere in the core products that are affected by the issue being discussed let us know.

It's true that we likely won't be "busting down doors" to chase this niche issue down if we can't reproduce it without custom code in the first place.

[Midnight Modding]

7 hours ago, bfarber said:

If you find anywhere in the core products that are affected by the issue being discussed let us know.

It's true that we likely won't be "busting down doors" to chase this niche issue down if we can't reproduce it without custom code in the first place.

I don't care if it gets fixed or not, but that's a real reach calling it niche when it affects any front end Select that has numbers as keys if there is a form error. 🙂 I said myself that you may not consider it a bug if it doesn't affect a first party app, but if that's the standard there are a lot of reported "bugs" on here that got fixed which didn't need to be.