#states doesn't work correctly with type text_format

The final version of D7.0 didn't took care of this... as for few other things considering the #states functionnality (#735528: FAPI #states: Fix conditionals to allow OR and XOR constructions is a very cool fix i think).

Anyway, this function changed a bit since the rc2 and the closest() function is used now, so i went along with your idea of changing the states.js, but in the final version of it.
I changed the parents() function a contionnal with hasClass() verification since parents() or parent() doesn't return the current element if nothing was found.

Line 481 :

      var elem = $(e.target).closest('.form-item, .form-submit, .form-wrapper');
	  if(elem.parent().hasClass('text-format-wrapper')){
		  elem = elem.parent();
	  }
	  elem[e.value ? 'show' : 'hide']();

I still think that it's not the good way to fix it as it's not really a states.js misfunctionning but as you said the text_format/text-area form fields generation that should be adjusted. At least i see it that way.

This bug is still present in 7.2 release and the 7.x-1.x branch. Attaching a screenshot to further illustrate the bug.

Confirmed : the bug is still alive in 7.15 release.

Happens in D8 as well.

Confirming this bug exists in 7.16.

I worked around the following way (could possibly be the general fix):

Copy the filter_process_format() function to your module and rename it to mymodule_filter_process_format().

Change the new function like this (adding 3 lines of code):

// snip

// Setup child container for the text format widget.
  $element['format'] = array(
    '#type' => 'fieldset',
    '#attributes' => array('class' => array('filter-wrapper')),
  );

  // The #states-bug fix:
  if (isset($element['value']['#states'])) {
    $element['format']['#states'] = $element['value']['#states'];
  }

  // Prepare text format guidelines.
  $element['format']['guidelines'] = array(
    '#type' => 'container',
    '#attributes' => array('class' => array('filter-guidelines')),
    '#weight' => 20,
  );

// snap

Finally, set the #process attribute of your text_format element to array('mymodule_filter_process_format').

Simple work around !

 $form['container'] = array(
   '#type' => 'container',
   '#states' => array('visible' => array(':input[name="xyz"]' => array('value' => true),),),
 );

 $form['container']['mytext'] = array(
   '#type' => 'text_format',
   '#format' => 'full_html',
   '#title' => t('My text format'),
   //'#states' => array('visible' => array(':input[name="xyz"]' => array('value' => 'true'),),),
   );

I achieved #states results for my "mytext" field by putting "mytext" field inside "container" field and applying #states to "container" field rather than "mytext".

Ideally by considering above(#7) work around we can update some basics designing in filter module.
Istead of wrapping text_format field in simple div we should have container field as parent field.
and attributes like #states should be applied to it. I am not sure about pros and cons of adding parent container to text_format field internally.

#7 worked perfectly here (D7). Thanks!

While #7 works just fine, IMHO it's clunky and not very Drupal-y.

Submitting a core patch with the suggestion in comment #6

Patch at #16 works perfectly for me!

By the way, I have been having trouble getting #states applied to a container to properly work in a situation where the form (or subform) is loaded via AJAX. At least with Paragraphs. So I'm not sure if that is a viable workaround for every situation. For some reason, the #states don't get properly activated when the form elements first load from the AJAX request. They work, in that if I change the element that the container is dependent on, the state changes get applied.

I think patch at #16 is elegant and in the right place. Thanks @sukanyaramakrishnan

#16 solved my problem

Patch #16 works quite well for me using Drupal 8.8

#16 working in 8.9.x now, as well.

Thanks for filing this bug report and for fixing the issue. Bug fixing is very valuable. In order to commit a bug fix, we need an automated to test to prove that we've fixed the bug and ensure that we don't break it again in the future. For more information about writing tests in Drupal, see the following links:

1. https://www.drupal.org/docs/testing/phpunit-in-drupal/phpunit-javascript...
2. https://api.drupal.org/api/drupal/core%21core.api.php/group/testing/9.1.x

Here is a test for this, test only patch is also the interdiff

Patch #28 works as expected and includes test coverage. Setting to RTBC.

+++ b/core/tests/Drupal/FunctionalJavascriptTests/Core/Form/JavascriptStatesTest.php
@@ -80,6 +112,8 @@ protected function doCheckboxTriggerTests() {
+    $this->assertFalse($text_format_invisible_when_checkbox_trigger_checked_value->isVisible());
+    $this->assertFalse($text_format_invisible_when_checkbox_trigger_checked_format->isVisible());

We've got two negative asserts here, but not the corresponding assertTrue(...->isVisible()) like we have for the other elements being tested in this class.

I think we should add those.

Yay, glad to see this getting fixed! And that it's easy to expand core/modules/system/tests/modules/form_test/src/Form/JavascriptStatesForm.php from #2702233: [backport] Add JavaScript tests for Form API #states: required, visible, invisible, expanded, checked, unchecked. Thanks for the effort so far. In addition to #32 (+1 to all that), a few other nits/concerns:

1. +++ b/core/modules/filter/src/Element/TextFormat.php
   @@ -165,6 +165,12 @@ public static function processFormat(&$element, FormStateInterface $form_state,
   +    // Set the states property for the filter format element from the value
   +    // of the element if it exists.
   

   Maybe:

   "Set the #states property for the filter format element from the value element if it exists." ?

   a. "states" on its own is maybe confusing? Is "#states" more clear?

   b. "format element" vs. "value of the element" is a little confusing. Let's be consistent about if it's a value/format *of* the element, or if we're talking about the value and format (sub)elements *of* the (parent) element.

2. +++ b/core/modules/filter/src/Element/TextFormat.php
   @@ -165,6 +165,12 @@ public static function processFormat(&$element, FormStateInterface $form_state,
   +    if (isset($element['value']['#states'])) {
   

   Should we also check if $element['#states'] itself is set?

3. +++ b/core/modules/system/tests/modules/form_test/src/Form/JavascriptStatesForm.php
   @@ -121,6 +121,15 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   +    $form['text_format_invisible_when_checkbox_trigger_checked'] = [
   +      '#type' => 'text_format',
   +      '#title' => 'Text format invisible when checkbox trigger checked',
   +      '#states' => [
   +        'visible' => [
   +          ':input[name="checkbox_trigger"]' => ['checked' => FALSE],
   

   The rest of the test names the things exactly what they are. E.g.:

      $form['textfield_invisible_when_checkbox_trigger_checked'] = [
         '#type' => 'textfield',
         '#title' => 'Textfield invisible when checkbox trigger checked',
         '#states' => [
           'invisible' => [
             ':input[name="checkbox_trigger"]' => ['checked' => TRUE],
           ],
         ],
       ];
   

   In this case, the name doesn't match the logic due to inverted values. Either we should use 'invisible' and 'checked' => TRUE, or we should rename this element to "text_format_visible_when_checkbox_trigger_unchecked" or something.

Thanks again,
-Derek

Thanks for the reviews!

#32.1 done
#33.1 Updated the comment, I think it's clearer now
#33.2 Don't think we need that since $element['value'] is set to the value of $element about 50 lines earlier, so there is nothing there that isn't in 'value'
#33.3 Agree, changed to invisible and TRUE

Thanks, @Lendude! Mostly looking good. A few final points:

1. +++ b/core/modules/filter/src/Element/TextFormat.php
   @@ -165,6 +165,12 @@ public static function processFormat(&$element, FormStateInterface $form_state,
   +    // Set the #states property of the filter format element to the same value
   +    // as the value element, if the value element has #states set.
   

   This still seems clumsy. How about?

   "If the value element has #states set, copy it to the format element."

   That even fits in 80 chars. ;)

2. +++ b/core/tests/Drupal/FunctionalJavascriptTests/Core/Form/JavascriptStatesTest.php
   @@ -60,6 +88,10 @@ protected function doCheckboxTriggerTests() {
   +    $text_format_invisible_when_checkbox_trigger_checked_value = $page->findField('text_format_invisible_when_checkbox_trigger_checked[value]');
   +    $text_format_invisible_when_checkbox_trigger_checked_format = $page->findField('text_format_invisible_when_checkbox_trigger_checked[format]');
   

   While I'm all for self-documenting variable names, this seems to take it a bit far. ;) All of the other local vars in this method assume the "when_checkbox_trigger_checked" part. It seems cleaner to leave that out for these, too.

3. +++ b/core/tests/Drupal/FunctionalJavascriptTests/Core/Form/JavascriptStatesTest.php
   @@ -60,6 +88,10 @@ protected function doCheckboxTriggerTests() {
   +    $this->assertNotEmpty($text_format_invisible_when_checkbox_trigger_checked_value);
   +    $this->assertNotEmpty($text_format_invisible_when_checkbox_trigger_checked_format);
   

   All the existing code puts the assertNotEmpty() immediately after trying to find each thing, so for consistency, let's do the same.

Attached fixes these 3. I also wondered about this:

4. +++ b/core/tests/Drupal/FunctionalJavascriptTests/Core/Form/JavascriptStatesTest.php
   @@ -21,6 +22,33 @@ class JavascriptStatesTest extends WebDriverTestBase {
   +    $filtered_html_format->save();
   

   Why the local variable at all? Why not just:

   FilterFormat::create([...])->save();

   ?

Wasn't sure that was worth fixing, so attaching it as a separate patch.

Also refreshing the test-only patch for good measure. It should fail with:

1) Drupal\FunctionalJavascriptTests\Core\Form\JavascriptStatesTest::testJavascriptStates
Failed asserting that true is false.
...
/.../core/tests/Drupal/FunctionalJavascriptTests/Core/Form/JavascriptStatesTest.php:116

Line 116 is:

    $this->assertFalse($text_format_invisible_format->isVisible());

I believe 1 of these 2 patches is now RTBC, but now I can't say. 😉

Thanks,
-Derek

Re: #37 - The failed test result for #35b was unexpected. #3192260: [random test failure] Random fail in media_library CKEditorIntegrationTest strikes again. Re-queued.

I have hidden the b patch let's go with the original patch.

Committed/pushed to 9.2.x and cherry-picked to 9.1.x, thanks! Nice to see an eleven-year-old bug fixed.