Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Part of #3270437: [meta] Tasks to deprecate the CKEditor 4 module. BigPipe has a regression test that uses CKEditor 4.
Proposed resolution
Since the regression does not occur with CKEditor 5 (see #8, #10, and #12), move the test to CKEditor 4.
Remaining tasks
NR.
User interface changes
None
API changes
None
Data model changes
None
Release notes snippet
N/A
Issue fork drupal-3270835
Show commands
Start 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 #3
xjmComment #4
xjmComment #6
xjmWim wrote this, so I can RTBC it. :)
Comment #7
wim leersOh, wow, I did not expect this to happen! 😄
Comment #8
lauriiiSince the test is a regression test, I'm wondering after changing to use CKEditor 5, if it's still testing what it's supposed to. I'm wondering if that's something we could confirm by reverting the fix from #2698811: BigPipe unnecessarily renders and sends multiple-occurrence placeholders multiple times when using JS — can cause JS errors and checking if it still fails. I'm wondering what should we do if it doesn't fail? I guess it would make sense to make sure that CKEditor works with Big Pipe, but even more valuable would be to ensure that the original bug is not reintroduced 🤔
Comment #9
wim leersThat's why I originally said I'm on the fence whether we truly want to keep this test — it may be fine to just remove it.
I'll let @xjm or somebody else go through the exercise proposed in #8 — I want to continue to be able to RTBC this.
Comment #10
wim leers@lauriii just pointed out that it is very unlikely that if we do #8, that tests would fail. Because CKEditor 4 has a highly custom "fetch plugin JS files via HTTP requests computed automatically via URL manipulation" logic, which interfered with CKEditor 4 initializing correctly.
CKEditor 5 on the other hand is being loaded using Drupal's default asset library loading strategy.
So it is likely that removing
\Drupal\Tests\big_pipe\FunctionalJavascript\BigPipeRegressionTest::testCommentForm_2698811()would be logical.Comment #11
xjmI pushed a commit to remove the handling of multiple occurrences in
sendNoJsPlaceholders().However,getPlaceholderOrder()has been significantly rewritten since the original fix in #2698811: BigPipe unnecessarily renders and sends multiple-occurrence placeholders multiple times when using JS — can cause JS errors, so there's no clean revert really.Comment #12
xjmEdit: Based on https://www.drupal.org/pift-ci-job/2346379 it looks like a different test fails if we revert the original fix, but not this one. OK, I'm convinced we can remove it. :)
Comment #13
xjmSo what I think we should do is:
@group legacy.Comment #14
xjmComment #17
xjmComment #18
xjmComment #19
xjmChecked the full console output on dispatcher to confirm both the old and new tests are running:
This is ready for review.
Comment #20
xjmComment #21
wim leersYay! Simplification 🤩 Nice detective work there, @xjm! 🕵️♀️
Comment #22
catchCommitted 650da65 and pushed to 10.0.x, and cherry-picked to 9.4.x. Thanks!
Comment #25
catchNice to get rid of this one eventually!