To reproduce, say you have a form like this one:

    // switching to 'bar' item should display 'dependant' text field
    $form['principal'] = array(
        '#type' => 'radios',
        '#title' => 'Principal',
        '#options' => array('foo', 'bar'),
        '#default_value' => 0,
    );

    // should get displayed based on selection on 'principal'
    $form['dependant'] = array(
        '#type' => 'textfield',
        '#title' => 'Dependant',
        '#states' => array(
            'visible' => array(
                // (1) works with this, because a explicit string "1" is passed: ':input[name="principal"]' => array('value' => "1"),
                ':input[name="principal"]' => array('value' => 1), // doesn't work in IE8
            ),
        ),
    );

Switching 'principal' radio buttons it is expected that 'dependant' text field is displayed and hidden.

Instead, in IE8 (without uncommenting section (1)), 'dependant' visible state is never changed on 'principal' changes, the problem is because of the usage of 'reference.constructor.name' js which is not supported on IE8 and the posterior call to

function compare (a, b) {
  return (a === b) ? (typeof a === 'undefined' ? a : true) : (typeof a === 'undefined' || typeof b === 'undefined');
}

Where this method compares considering types (and the actual values compared are a Number vs a String, which gets false).

I'm attaching a module to reproduce this issue easily and a patch tested on Firefox, Chome and IE8.

Notes:

Bug only found in IE8, problem could exist in newer IE versions.

CommentFileSizeAuthor
#68 1962800-68.patch1.86 KBJeroenT
#60 1962800_60.patch1.86 KBmrinalini9
#60 1962800_60_bis.patch2.01 KBmrinalini9
#59 1962800_59_bis.patch2.02 KBAlexj12
#59 1962800_59.patch1.87 KBAlexj12
#56 1962800_56_bis.patch2.02 KBmpp
#56 1962800_56.patch1.87 KBmpp
#51 core-states-not-working-with-integers-ie11_1962800_51.patch1.83 KBMiroslavBanov
#46 core-states-not-working-with-integers-ie11_1962800_46.patch1.86 KBwesleydv
#39 core-states-not-working-with-integers-ie11-1962800.patch936 bytesfilsterjisah
#31 form_states_not-1962800-31.patch913 bytesgeertvd
#27 states-not-working-with-integers-in-ie-1962800.patch927 bytesAnonymous (not verified)
#25 states-not-working-with-integers-in-ie-1962800-24.patch936 bytespeterpoe
#21 states-not-working-with-integers-in-ie-1962800-21.patch882 bytesgeertvd
#21 interdiff-1962800-9-21.txt487 bytesgeertvd
#9 states-not-working-with-integers-in-ie-1962800-9.patch881 bytesmgifford
#2 states-not-working-with-integers-in-ie-1962800-2.patch861 bytesMatthijs
fix_problem_with_states_js.zip1.37 KBhablutzel1
form_states_not_working_with_literal_integers_as_values_in_IE8.patch1.04 KBhablutzel1
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hablutzel1’s picture

Here you can find that 'name' is not supported by IE

https://developer.mozilla.org/de/docs/JavaScript/Reference/Global_Object...

Matthijs’s picture

Status: Active » Needs review
FileSize
861 bytes

I've created a similar patch based on jQuery.type(). This looks a bit cleaner, no?

filsterjisah’s picture

Status: Needs review » Reviewed & tested by the community

Works great. Thx Matthijs!

David_Rothstein’s picture

Title: form #states not working with literal integers as values in IE8 » form #states not working with literal integers as values in IE8 (and higher?)
Version: 7.x-dev » 8.x-dev
Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs backport to D7

#879580-16: States fail when using integer values for select/radio dependencies and subsequent comments suggest this is an issue in newer version of Internet Explorer, in which case it would need to be fixed in Drupal 8 first. There's also a patch in that issue (before it was re-closed) that could be used for comparison.

DnaX’s picture

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

I can confirm this bug either with IE10. A workaround can be adding a empty string to the number to force a string cast.

dcam’s picture

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

Reverting version change. This issue must be fixed in Drupal 8 first.

nod_’s picture

Component: base system » javascript
Jalandhar’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

Patch needs to be updated.

mgifford’s picture

Status: Needs work » Needs review
FileSize
881 bytes

reroll for D8.

jhedstrom’s picture

Patch still applies. Adding the manual testing tag.

Anybody’s picture

Patch works good for me. How can we proceed here? The issue leads to follow-up problems for example in conditional_fields.module in IE, see #1373656: Dependent fields don't open in IE

mgifford’s picture

@Anybody You could mark it RTBC with manual testing.

Anybody’s picture

If we can get some more feedback I'd like to set this RTBC of course. My feedback is not enough I think. But it works great for me.

MathieuMa’s picture

We have been testing this patch (because of the reported issue) without any side effect as of now.

Anybody’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: states-not-working-with-integers-in-ie-1962800-9.patch, failed testing.

Status: Needs work » Needs review
spencer95@gmail.com’s picture

Status: Needs review » Reviewed & tested by the community

We're using this patch on D7 and D8. It's working for us in IE 8/9/10.

Anybody’s picture

Perfect. Let's get this into a new release as soon as possible :)

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +JavaScript
core/misc/states.js
  184:6  error    Expected indentation of 6 space characters but found 5  indent
  186:9  error    Expected indentation of 7 space characters but found 8  indent
  187:8  error    Expected indentation of 5 space characters but found 6  indent
  190:9  error    Expected indentation of 7 space characters but found 8  indent
  191:8  error    Expected indentation of 5 space characters but found 6  indent

Seems to introduce some javascript formatting errors.

geertvd’s picture

Bumped into this issue in a d7 project today. Would be nice to have this fixed in d8 so it can be backported to d7.

droplet’s picture

+++ b/core/misc/states.js
@@ -174,9 +174,16 @@
+        name = $.type(reference);

I wonder if we should use jQuery.type for all browsers.

peterpoe’s picture

Title: form #states not working with literal integers as values in IE8 (and higher?) » form #states not working with literal integers as values in IE
Status: Needs review » Reviewed & tested by the community
Issue tags: -ie8

Confirming that bug applies up to IE11, and that the patch works after some manual testing.
This breaks many Conditional Fields configurations in IE, please commit.

@droplet May be a good point, but let's get this fixed first

catch’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/misc/states.js
@@ -174,9 +174,16 @@
+        name = name.charAt(0).toUpperCase() + name.slice(1);

This really needs a comment.

peterpoe’s picture

Status: Needs work » Needs review
FileSize
936 bytes

Comment added.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Anonymous’s picture

Status: Needs review » Needs work

The last submitted patch, 27: states-not-working-with-integers-in-ie-1962800.patch, failed testing.

Anonymous’s picture

Status: Needs work » Needs review
Anonymous’s picture

Ran the tests locally and manually tested the one that failed in the above and it passed, re-queuing.

geertvd’s picture

ufku’s picture

Status: Needs review » Needs work

Jquery.type() always returns "object" for custom types.
Regex method seems more reliable.

var name = reference.constructor.name || (reference.constructor.toString().match(/function\s+([^\(\s]+)\(/)||['', ''])[1];

Anonymous’s picture

Status: Needs work » Reviewed & tested by the community

That doesn't matter, does it? Isn't the block where type is used just trying to find a valid fallback if the name doesn't exist regardless? Introducing regex here seems messy and the last patch seems to work fine.

Can you determine a use case where something is broken/bad? I'd like to have this RTBC and fixed so I don't have to patch my core every time.

smaz’s picture

+1 for patch in #31 working for me in D7.

Cheers!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 31: form_states_not-1962800-31.patch, failed testing.

The last submitted patch, 31: form_states_not-1962800-31.patch, failed testing.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

filsterjisah’s picture

Jeroen_005’s picture

Status: Needs work » Needs review
maacl’s picture

Had this Problem on IE11, #39 is working for me. Thanks @filsterjisah

Anybody’s picture

Status: Needs review » Reviewed & tested by the community

Confirming #39 for D8. Did someone already check if the backport to D7 can work the same way?

star-szr’s picture

Status: Reviewed & tested by the community » Needs review

A couple thoughts:

  1. Can we add a comment above the type-casting code explaining that this workaround is for IE11 and lower, so the code can hopefully be removed in the future?
  2. On that note, is this workaround needed for Edge, or only IE11-?

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

droplet’s picture

Status: Needs review » Needs work

It depends on how we passing the function. Use jQuery workaround would be safer.

https://kangax.github.io/compat-table/es6/#test-function_name_property

Let's add some comments (and use jQuery workaround only IMO)

wesleydv’s picture

Adding a new patch against 8.5.x (both .es6.js and the transpiled .js)
Only added a comment, code is the same as #39

wesleydv’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

MiroslavBanov’s picture

Somehow git apply fails to apply the patch #46 for me, but patch -p1 works. Adding reroll here against core 8.6.1.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

aleevas’s picture

Status: Needs work » Needs review
mpp’s picture

Status: Needs review » Reviewed & tested by the community
GrandmaGlassesRopeMan’s picture

Status: Reviewed & tested by the community » Needs work
  1. +++ b/core/misc/states.es6.js
    @@ -203,6 +203,21 @@
    +        name = name.charAt(0).toUpperCase() + name.slice(1);
    

    Use template string here.

  2. +++ b/core/misc/states.es6.js
    @@ -203,6 +203,21 @@
    +      if (name in states.Dependent.comparisons) {
    

    Don't use in, you could accidently match against a prototype. Use hasOwnProperty instead.

mpp’s picture

Status: Needs work » Needs review
FileSize
1.87 KB
2.02 KB

Thank you @alwaysworking for your feedback.

Personally I find this to be more readable:
name = name.charAt(0).toUpperCase() + name.slice(1);
Than this:
name = `${name.charAt(0).toUpperCase()}${name.slice(1)}`;

Note that this:

        let firstChar = name.charAt(0).toUpperCase();
        let remainder = name.slice(1);
        name = `${firstChar}${remainder}`;

would be turned into:

        var firstChar = name.charAt(0).toUpperCase();
        var remainder = name.slice(1);
        name = '' + firstChar + remainder;

I fixed your second remark in 1962800_56.patch, in 1962800_56_bis.patch a template string is used.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Alexj12’s picture

I'm updating the patches here to use

states.Dependent.comparisons.hasOwnProperty(name)
over
states.Dependent.comparisons.hasOwnProperty('name')

as the patches from #56 did not pass this check.

mrinalini9’s picture

I have rerolled patch #59 as it failed to apply, please review.

nod_’s picture

Status: Needs review » Needs work

Not sure why we have 2 patches here. If the one with the concatenation is better just keep that one (i agree it's more readable and we're not concatenating that with a static string, so all good for me).

As long as eslint doesn't complain it's valid :), can we make it clear which patch should be reviewed? Thanks.

Alexj12’s picture

I agree too. I've been using the concatenating patch for a while with no issues and also believe it is more readable in this case. I'll hide the other patches and the argument can be made for the the template string if required.

Alexj12’s picture

Status: Needs work » Needs review

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

JeroenT’s picture

Issue tags: +Bug Smash Initiative

Let's see if the patch still applies for Drupal 9.4.

JeroenT’s picture

FileSize
1.86 KB

Patch no longer applied, so I created a reroll.

Also note that this patch is only necessary for Drupal 9. Since Drupal 10 drops support for Internet Explorer.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

nod_’s picture

Status: Needs review » Closed (won't fix)

IE is not supported in 10+ so this is not a concern anymore :)