I noticed this issue while developing a relatively complex form, but I have since refined the problem down to a very simple form and have tested on a vanilla instance of Drupal 9.5.10.
The issue occurs when an element's state is built from a logical combination of multiple other fields.
For example:
'visible' => [
':input[name="select1"]' => ['value' => 0],
':input[name="select2"]' => ['value' => 1],
],
Also note the same behaviour occur if written:
'visible' => [
':input[name="select1"]' => ['value' => 0],
'and',
':input[name="select2"]' => ['value' => 1],
],
- BigPipe module has been installed.
- Use the following form:
$form['select1'] = [
'#type' => 'select',
'#title' => 'Select 1',
'#options' => [0 => 0, 1 => 1],
'#default_value' => 0,
];
$form['select2'] = [
'#type' => 'select',
'#title' => 'Select 2',
'#options' => [0 => 0, 1 => 1],
'#default_value' => 0,
];
$form['select3'] = [
'#type' => 'select',
'#title' => 'Select 3',
'#options' => [0 => 0, 1 => 1],
'#states' => [
'visible' => [
':input[name="select1"]' => ['value' => 0],
'and',
':input[name="select2"]' => ['value' => 1],
],
],
];
After the form loads, you see the select elements 1 and 2. You don't see select 3 because the default values aren't as defined in #states.
If I select a value of 1 in select element 2, the values *should* be correct to trigger the appearance of select element 3. However, this doesn't happen.
Instead you have to set element 2 back to a value of 0, then again to a value of 1, before select element 3 appears.
Modules like big_pipe invoke attachBehaviors that results in multiple attachment of behaviors for a given context. If the attachBehavior is called multiple times, then in states multiple states.Dependent objects are created for the same elements. However the initialise is not called in Trigger function for all the states.Dependent object since it checks if it was already initialised for an element. Thus resulting in the bug. Proposed solution is to ensure that the states are attached only once.
- Approach 1: 3386191-states-not-working
Modify core/misc/states.js to use the library once to avoid duplicate attach calls multiple times.
Drupal.behaviors.states = {
attach(context, settings) {
// Uses once to avoid duplicates if attach is called multiple times.
const elements = once('states', '[data-drupal-states]', context);
| Comment | File | Size | Author |
|---|---|---|---|
| #46 | fix-core-states-issues.patch | 476 bytes | Ozle |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
ashley george commentedComment #3
shiv_yadavHi Ashley George,
Please use this type of coding after working for me visible functionality .
shared coding format:
$form['select1'] = [
'#type' => 'select',
'#title' => 'Select 1',
'#options' => [0 => 0, 1 => 1],
'#default_value' => 0,
];
$form['select2'] = [
'#type' => 'select',
'#title' => 'Select 2',
'#options' => [0 => 0, 1 => 1],
'#default_value' => 0,
];
$form['select3'] = [
'#type' => 'select',
'#title' => 'Select 3',
'#options' => [0 => 0, 1 => 1],
'#states' => [
'visible' => [
[':input[name="select1"]' => ['value' => 0]],
'and',
[':input[name="select2"]' => ['value' => 1]],
],
],
];
Comment #4
shiv_yadavComment #5
smustgrave commented@Ashley George does that answer your question? If so this can be closed.
Comment #6
ashley george commented@smustgrave unfortunately this doesn't seem to resolve things. After implementing the change suggested by @shiv_yadav the visibility on "select3" is now only linked to "select1". So I beleive there is still a valid issue here.
Comment #7
ashley george commentedComment #8
ashley george commentedComment #9
ashley george commentedActually I'm not quite right with my description of how it's behaving now, but it's still not correct. If someone could see if they can replicate that would be awesome.
Comment #10
smustgrave commentedI can confirm the issue you are seeing. I'm not entirely sure the cause.
At first I thought maybe states wasn't picking up the default value from select1 but that kinda is disproved by switching select2 to 0 and 1 again. Which triggered select3
Believe this is a valid bug, moving to Active for a patch/tests.
Comment #14
sukr_s commentedModules like big_pipe invoke attachBehaviors that results in multiple attachment of behaviors for a given context. So maintain semaphore to ensure that the behavior is attached only once.
If the attachBehavior is called multiple times, then in states multiple states.Dependent objects are created for the same elements. However the initialise is not called in Trigger function for all the states.Dependent object since it checks if it was already initialised for an element. Therefore it's best to avoid multiple invocation of attachBehaviors to ensure that the behviors work as intended.
This fix ensures that attachBehavior is called only once for a given context
Comment #15
smustgrave commentedWill need a test to show the bug.
Also issue summary appears incomplete, recommend using standard issue template
Comment #20
phthlaap commentedComment #21
phthlaap commentedComment #22
phthlaap commentedComment #23
phthlaap commentedComment #24
phthlaap commentedComment #25
phthlaap commentedComment #26
sukr_s commented@phthlaap the fix that you have provided is very specific to #states. The duplicates loading of attachBehavior can cause problems for any other module or custom module. So it's best to fix the root cause rather than in the affected module. Enclosed in a patch with tests for the same using #states for testing.
Comment #27
smustgrave commentedIf a different approach is being proposed can a separate MR be opened for reviewing. Issue summary will eventually have to be updated to match.
Comment #29
phthlaap commentedComment #30
phthlaap commentedThe approach in patch #26 will affect some tests because I believe Ajax should reattach on form submission once again.
Comment #31
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #32
phthlaap commentedComment #33
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #35
phthlaap commentedComment #36
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #38
phthlaap commentedComment #39
phthlaap commentedComment #40
phthlaap commentedAs suggestion of @nod_ , I use Once library to fix the issue. I think it is better.
I also changed the variable name $states because it is no longer a jQuery instance. but, it will be identical to the state variable defined at the top of the file, so I named it element.
Comment #41
phthlaap commentedComment #42
phthlaap commentedComment #43
brunoalmeida commentedI have tested the last changes and I can confirm it's working.
I'm also adding a patch for the 10.1.6 core version.
Comment #44
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #45
phthlaap commentedComment #46
Ozle commentedI fixed this issue modifying only the $states variable to
const $states = once('states', '[data-drupal-states]', context);Comment #47
smustgrave commented@ozie can you try with tests though. Would have to be an MR
Comment #48
phthlaap commented@smustgrave
The merge request above are already using same solution once library and also have a tests https://git.drupalcode.org/project/drupal/-/merge_requests/7045/diffs
Comment #49
smustgrave commentedMR also has additional changes besides that one line
Comment #50
phthlaap commentedAs comment #40 I has explained, the variable name $states is no longer a jQuery instance. but, it will be identical to the state variable defined at the top of the file, so I named it element.
Comment #51
smustgrave commentedRemoving the 2nd approach to avoid confusion
Shows test coverage
Believe the change makes sense using once()
Comment #52
larowlanHi folks
Can we get an issue summary update that details why using once is the solution here.
Its not evident to me why that makes things different or why bigpipe is part of the issue.
Fine to self RTBC afterwards.
Thanks
Comment #53
sukr_s commentedIS updated as per #14 analysis
Comment #54
sukr_s commentedComment #55
smustgrave commentedIS is updated.
Comment #60
nod_Committed and pushed 5e2806f1b5 to 11.x and e3b6f2fd67 to 11.0.x and b02bce4af0 to 10.4.x. Thanks!
Comment #61
eddylbsHello,
Patch from #46 works for me on 10.2.4
Thanks
Comment #63
nod_Comment #65
cilefen commentedWe have a report of a regression caused by this. I don't know whether this should actually be fixed in Webform. Perhaps the people on this issue can look into it.
Comment #66
gaurav_manerkar commentedWe are facing issue because of this change. Conditional #states JS are not getting applied when element is refreshed/rendered by JS
Comment #67
sukr_s commentedsee related issue #3468860: JS #states behavior does not have a detach method
Comment #68
nod_a proper fix is in the works for core, could you test #3468860: JS #states behavior does not have a detach method. It should fix this but introduces another change in that #states about a missing element are now applied, they used to be ignored.