- Issue created by @Evaldas Užkuras
- Status changed to Needs review
over 1 year ago 7:48am 10 March 2023 - 🇱🇹Lithuania Evaldas Užkuras Kaunas, LT
Here's a patch adding use of `once` as I proposed.
- Status changed to Needs work
over 1 year ago 1:25pm 10 March 2023 - 🇪🇸Spain rodrigoaguilera Barcelona
Thank you for creating the issue and providing a patch.
Bugs should be fixed in the latest dev version of Drupal and then backported to older versions like 9.5.x when applicable.
Since D10 doesn't use es6.js files anymore the patch needs to be different for D10.Also we should provide one of this scenarios as a js test to be able ensure the behaviour stays consistent in the future
I can confirm this issue atleast in 9.5 and 9.2, the patch at #3 resolves it for me, atleast in my situation BigPipe is not used when anonymous/not-logged-in, but on logged in, bigpipe is activated and then I get missing form states
- Merge request !4044Ensure states loop uses `.once()`, Attempt 11.x re-roll of... → (Open) created by dgtlmoon
- last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed 5:01 4:32 Running- last update
over 1 year ago 29,398 pass - last update
over 1 year ago 29,398 pass There is this one which is sort of related https://www.drupal.org/project/drupal/issues/3337995 🐛 Big Pipe calls attachBehaviors twice Closed: works as designed but was closed
- 🇺🇦Ukraine abramm Lutsk
I'm a bit surprised that
#states
wasn't usingonce()
, it's even declared as dependency to thedrupal.states
library.
This is definitely a bug and the patch looks correct (didn't test it though). - Status changed to Needs review
over 1 year ago 8:15am 25 May 2023 Just throwing this out there, maybe `states` doesnt bind with the right 'context' ? https://www.drupal.org/project/drupal/issues/3337995#comment-15060748 🐛 Big Pipe calls attachBehaviors twice Closed: works as designed
I already have trouble convincing front-end developers to use the context parameter in our custom modules (most of them just ignore it since don't understand its logic). Now I have to convince them to use jQuery once. When the rest of the javascript world is moving away from jQuery we are forcing people to use it. It doesn't make any sense.
Edit: I just realized that Drupal has its own version of once that is compatible with vanilla javascript but I still think we need a javascript API easier to understand for frontend developers that aren't specialized in Drupal.
- Status changed to Needs work
over 1 year ago 2:57pm 29 May 2023 - 🇺🇸United States smustgrave
Did not test.
But was previously tagged for tests which still need to happen. May be worth posting in #testing channel for suggestions where to add.
- 🇺🇸United States bkosborne New Jersey, USA
This was a tricky one to debug, but doing so eventually led to me here today. Here's an example form that is affected by this bug:
$form['select'] = [ '#type' => 'select', '#title' => 'Select', '#options' => [ 'one' => 'one', 'two' => 'two', 'three' => 'three', ], '#default_value' => 'one', ]; $form['checkbox'] = [ '#type' => 'checkbox', '#title' => 'Checkbox One', ]; $form['textfield'] = [ '#type' => 'textfield', '#title' => 'Textfield', '#states' => [ 'visible' => [ ':input[name="select"]' => ['value' => 'one'], ':input[name="checkbox"]' => ['checked' => TRUE], ], ] ];
In this example, if the select box has the first option selected AND the checkbox is checked, the textfield should be shown. However, on page load w/ Big Pipe enabled, checking the checkbox (which meets conditions) does NOT show the textfield. You have to uncheck and then recheck it before it finally works.
Big Pipe causes two more invocations of Drupal behaviors, resulting in a total of 3 invocations with the form items in the context. That's enough to cause issues with the logic with the 3 duplicate states behaviors defined.
- last update
over 1 year ago 29,553 pass - 🇩🇪Germany FeyP
I just closed 🐛 States JavaScript should use the once library, causes problems with Big Pipe Closed: duplicate as a duplicate of this issue. There is some information on how to reproduce with the contributed Webform module in the IS of that issue and MR with a similar but slightly different approach to fixing this.
- First commit to issue fork.
- Status changed to Needs review
11 months ago 12:06am 12 December 2023 - 🇦🇺Australia mstrelan
Enabled
big_pipe
module on existingJavascriptStatesTest
. Test-only job should show it failing. - 🇦🇺Australia mstrelan
Well GitLab CI made a liar out of me. For some reason it decided to skip the test instead of running it and letting it fail. Attaching a patch to see what Drupal CI thinks.
FWIW this is the result when running the test-only changes locally:
$phpunit --filter=JavascriptStatesTest PHPUnit 9.6.15 by Sebastian Bergmann and contributors. Testing F 1 / 1 (100%) Time: 00:28.863, Memory: 357.00 MB There was 1 failure: 1) Drupal\FunctionalJavascriptTests\Core\Form\JavascriptStatesTest::testJavascriptStates Failed asserting that true is false. /data/app/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:121 /data/app/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55 /data/app/core/tests/Drupal/FunctionalJavascriptTests/Core/Form/JavascriptStatesTest.php:462 /data/app/core/tests/Drupal/FunctionalJavascriptTests/Core/Form/JavascriptStatesTest.php:70 /data/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:728 FAILURES! Tests: 1, Assertions: 205, Failures: 1.
- last update
11 months ago 30,711 pass, 1 fail - Status changed to RTBC
11 months ago 2:33pm 12 December 2023 - 🇺🇸United States smustgrave
Hiding #20 so bot doesn't pick it up.
Going to post in gitlab channel but I see this test-only job failed correctly https://git.drupalcode.org/issue/drupal-3347144/-/jobs/471592
But change seems good, had to look up the param changes but looks good.
- last update
11 months ago 30,712 pass - last update
11 months ago 30,726 pass - last update
11 months ago 30,764 pass - last update
11 months ago 25,915 pass, 1,788 fail - last update
11 months ago 25,865 pass, 1,791 fail - last update
11 months ago 25,951 pass, 1,789 fail - last update
11 months ago 25,948 pass, 1,820 fail - last update
11 months ago 25,939 pass, 1,804 fail - last update
11 months ago 25,890 pass, 1,843 fail - 🇳🇿New Zealand quietone
I'm triaging RTBC issues → . I read the IS and the comments. I didn't find any unanswered questions or other work to do.
Leaving at RTBC.
- last update
11 months ago 25,910 pass, 1,843 fail - 🇬🇧United Kingdom catch
Committed/pushed to 11.x and cherry-picked to 10.2.x, thanks!
- Status changed to Fixed
11 months ago 3:30pm 1 January 2024 Automatically closed - issue fixed for 2 weeks with no activity.
- 🇺🇸United States attheshow
Related regression ticket is here: https://www.drupal.org/project/drupal/issues/3416398 🐛 Regression on entity reference field states Fixed
- Status changed to Active
10 months ago 9:46am 26 January 2024 - 🇬🇧United Kingdom catch
The regression in 🐛 Regression on entity reference field states Fixed seems like it might be worse than the bug with big pipe that this fixed - I'm considering rolling this back in 11.x and 10.2.x, so we can try again.
- 🇺🇸United States bkosborne New Jersey, USA
Hmm, tricky. So it seems the problem is that states sometimes needs to be applied again because the dependents may have changed, right?
In that case, I guess we need a way for states behavior to replace the existing state that was there before so it doesn't create duplicate state definitions.
- 🇬🇧United Kingdom catch
Discussed this briefly with nod_ in slack and we both think reverting is probably the best approach here - more sites are still on 10.1.x than 10.2.x, so reverting the bug fixed here won't be a regression for them, but the regression introduced with be a new bug when those sites update. Also doesn't seem like there's an easy fix for this in either direction, we probably can't use once, and might need to make the states javascript re-entrant instead, no idea what that means in practice.
- Status changed to Needs work
8 months ago 2:12am 2 April 2024 - 🇦🇺Australia acbramley
Rerolling and marking as needs tests to cover the bug found in 🐛 Regression on entity reference field states Fixed
- 🇮🇳India sukr_s
This issue is a duplicate of 🐛 #states not working correctly when built from a logical combination of multliple fields Fixed
- Status changed to Postponed: needs info
4 months ago 10:26pm 17 July 2024 - 🇦🇺Australia mstrelan
It looks like the fix in 🐛 #states not working correctly when built from a logical combination of multliple fields Fixed was the exact same fix that had already been committed and reverted in this issue. So I'm wondering if we are still concerned about the issues in #34? I'm guessing not since @node_ was the one who committed it. Perhaps credit should be transferred from this issue as well. Setting to PMNMI to see if #34 still needs addressing.
- Status changed to Fixed
3 months ago 9:17am 19 August 2024 - 🇫🇷France nod_ Lille
let's take that on in 🐛 JS #states behavior does not have a detach method Needs review ,
Automatically closed - issue fixed for 2 weeks with no activity.