theme-my-login / theme-my-login

Home of the Theme My Login plugin for WordPress.
61 stars 54 forks source link

Dropdown selection can be duplicated #204

Closed EatonZ closed 2 years ago

EatonZ commented 2 years ago

Observe the following code:

tml_add_form_field('register', 'gender', array(
    'type' => 'dropdown',
    'value' => '',
    'label' => 'Gender',
    'options' => array('' => 'Select...', 0 => 'Male', 1 => 'Female'),
    'priority' => 23
));

It results in two selections being made in the dropdown because when the plugin does its comparisons here, a PHP comparison of '' == 0 and '' == '' both return true.

gender

EatonZ commented 2 years ago

Temporary fix:

case 'dropdown' :
    $output .= $label;
    $output .= $error;
    $output .= $args['control_before'];
    $output .= '<select name="' . $this->get_name() . '"' . $attributes . ">\n";
    $selection_made = false;
    foreach ( $this->get_options() as $value => $option ) {
        $output .= '<option value="' . esc_attr( $value ) . '"';
        if ( $this->get_value() == $value && !$selection_made ) {
            $output .= ' selected="selected"';
            $selection_made = true;
        }
        $output .= '>' . esc_html( $option ) . "</option>\n";
    }
    $output .= "</select>\n";
    $output .= $args['control_after'];
    break;
jfarthing84 commented 2 years ago

In this case, I would just suggest using 1 for the "Male" option and 2 for the "Female" option. Or, -1 for the "Select..." option, and then check for and ignore -1.

EatonZ commented 2 years ago

@jfarthing84 Those values are tied to server-side validation and can't easily be changed right now, as those have been the decided values for a long time. I would appreciate a fix so I don't have to modify your plugin after every update.

jfarthing84 commented 2 years ago

Then, again, the fix would be to use the -1 for the "Select..." value.

EatonZ commented 2 years ago

@jfarthing84 Unfortunately, if you set the initial value of the field like this: 'value' => tml_get_request_value('gender', 'POST') That will be an empty string on form load (GET), which is why I am using an empty string in the first place, so it will select the option with the empty string value.

Of course I can check for -1, but that would be silly since a better fix would be to differentiate between 0 and an empty string in the plugin code itself, or you could use the temporary fix I added above to ensure only 1 option gets selected.

jfarthing84 commented 2 years ago

'value' => tml_get_request_value( 'gender', 'post' ) ?: -1

EatonZ commented 2 years ago

I guess that would work. You aren't interested in addressing this situation in the plugin code?