When using integer options for select and radio form fields, the states system fails for dependent form fields operating with integer values.

For example:

define('OPTION_1', 1);
define('OPTION_2', 2);

function example_form(&$form, $form_state) {
  $form['select'] = array(
    '#type' => 'select',
    '#options' => array(
      OPTION_1 => 'Option 1',
      OPTION_2 => 'Option 2',
    ),
  );

  $form['dependent_field'] = array(
    '#type' => 'text',
    '#states' => array(
      'visible' => array(
        ':input[select]' => array('value' => OPTION_1)
      ),
    ),
  );
}

Fails because $(':input[select]').val() == '1' not 1, and the states system checks to make sure the values are the same type. The way around it is to type cast the dependent_field state value as a string:

  $form['dependent_field'] = array(
    '#type' => 'text',
    '#states' => array(
      'visible' => array(
        ':input[select]' => array('value' => (string) OPTION_1)
      ),
    ),
  );

but is there a better way to approach this? It's not exactly intuitive to make the value a string.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

Issue tags: -FAPI #states
bleen’s picture

sun’s picture

Probably not - this bug is most likely caused by PHP integer to JSON conversion (i.e., when #states get converted into Drupal.settings).

amitaibu’s picture

Subscribe

arithmetric’s picture

Version: 7.x-dev » 8.x-dev
Status: Active » Needs review
FileSize
475 bytes

Attached is a patch against the 8.x branch to resolve this issue by altering the value comparison code in states.js.

With this patch, if the reference value (from the #states array) is numeric but the value from jQuery is a string, then the reference variable is cast as a string before the strict comparison in compare().

Niklas Fiekas’s picture

Status: Needs review » Reviewed & tested by the community

Looks clean and solves the problem for me. Thank you arithmetric.

sun’s picture

Status: Reviewed & tested by the community » Needs review

Looks good. Wondering whether we could capture this non-obvious edge-case in an inline comment, to prevent people from interpreting this data type casting from number to string as a bug, and clarify what the casting is actually fixing, in case someone wants to rewrite/revamp this code at some point in the future.

arithmetric’s picture

@sun: Thanks for the feedback. I've updated the patch to include a comment on why the type casting should happen in this case.

sun’s picture

Status: Needs review » Reviewed & tested by the community

Thanks!

Niklas Fiekas’s picture

Will this also be fixed for D7? The patch applies and works for 7.x.

xjm’s picture

Issue tags: +Needs backport to D7

Changing to the "official" form of the backport tag.

catch’s picture

Version: 8.x-dev » 7.x-dev

Committed to 8.x, moving to 7.x for webchick.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 7.x. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -Needs backport to D7

Automatically closed -- issue fixed for 2 weeks with no activity.

anon’s picture

Issue summary: View changes
Status: Closed (fixed) » Needs work

This still doesn't work in IE 10+ in D7.

define('OPTION_1', 1);
define('OPTION_2', 2);
  $form['select'] = array(
    '#type' => 'select',
    '#options' => array(
      OPTION_1 => 'Option 1',
      OPTION_2 => 'Option 2',
    ),
  );
  $form['dependent_field'] = array(
    '#type' => 'textfield',
    '#states' => array(
      'visible' => array(
        'select' => array('value' => OPTION_1)
      ),
    ),
  );

It works in firefox and chrome tho.

anon’s picture

Seems like an IE issue.
https://groups.google.com/forum/#!topic/coffeescript/lIgad5BWWEk

if (reference.constructor.name in states.Dependent.comparisons) {
In IE, the constructor.name is nonstandard, and it is not suported by IE.

That make the solution provided by #8 to not work at all in IE. In the article I linked to, are a nasty hack to fix this,but I', note sure of that should go into core.

anon’s picture

Status: Needs work » Needs review
FileSize
847 bytes

Submitted patch for Drupal 7.

I'm still unfamiliar width Drupal 8 and I don'y know how to make the test code in that version.

xjm’s picture

Thanks @anon. This was patched in Drupal 8 and Drupal 7, so please open a new issue instead for the current bug and upload your patch there. Also, to test Drupal 8 and see if the bug exists there, you can try simplytest.me http://simplytest.me/project/drupal/8.x

xjm’s picture

Status: Needs review » Closed (fixed)

Restoring status -- you can add a link to the new issue in the comments here so others find it. :)

David_Rothstein’s picture

Not sure if another issue was filed, but there's an older issue (#1962800: form #states not working with literal integers as values in IE) that was filed about this as well. I left a comment on that issue mentioning that there's a patch to look at here too.