XWIKI-24800: The WYSIWYG editor sometimes doesn't join the realtime collaboration session

[vmassol]

Jira URL

https://jira.xwiki.org/browse/XWIKI-24800

This replaces #6298, which was opened on XWIKI-24287 — the flickering-test issue, in the
Development Issues only component — before we had a dedicated issue for the product bug.
@mflorea reviewed and approved that PR; the code here is identical, only the issue key and the
description changed. XWIKI-24287 is now closed as Solved By XWIKI-24800.

Changes

Description

• Wait for the CKEditor editing mode to be set before deciding whether the realtime collaboration
  session can be joined.

CKEDITOR.plugins.add('xwiki-realtime').init() runs at pluginsLoaded, but editor.mode is only
assigned much later, from within CKEDITOR.editor.prototype.setMode(), once the mode creator
(i.e. the WYSIWYG iframe) has finished loading. The initial join did:

editor._realtime.connect = async () => {
  if (editor.mode !== 'wysiwyg') {
    // Realtime collaboration is supported, but we can't join until the user switches to WYSIWYG mode.
    return true;
  }
  ...
};
return editor._realtime.connect();

The guard is meant to detect the Source mode, but it also matches "the editing mode is not set yet".
So whenever Loader.bootstrap() (two REST calls) completed before the editing mode was loaded, the
editor silently gave up on joining the realtime session: no exception, no notification,
realtimeSupported === true, and none of the mode change handlers registered afterwards can recover
because there is no Source -> WYSIWYG switch to react to.

Clarifications

• The guard was added by XWIKI-23985 ("Switching syntax in source mode wrongly joins the realtime
  session", f582505), first released in 18.1.0 — hence the Affects Version on XWIKI-24800 and
  the backport branches below. Before that commit the initial join had no mode check, so there was no
  silent skip.
• This is a race, so it can hit any editor load, but it is much more likely when the editor is
  recreated (e.g. after a document syntax change), because the RequireJS modules and the Netflux
  WebSocket are already warm and Loader.bootstrap() then only costs two REST round trips, while
  the new editor still has to load its iframe.
• It explains both failure modes seen on CI for RealtimeWYSIWYGEditorIT#syntaxChange:
  • assertTrue(firstEditPage.getToolbar().isCollaborating()) returning false right after the
    syntax conversion (e.g.
    master build 1361);
  • RealtimeWYSIWYGEditPage.gotoPage() timing out in RealtimeEditToolbar#waitUntilConnected() on
    the initial page load (master build 1350).
• Evidence from the CI artifacts of build 1361: the screenshot of the failure shows the reloaded
  editor with the plain Save & View / Save / Preview / Cancel bar and "Allow Realtime
  Collaboration" unchecked (the realtime toolbar was destroyed with the previous editor and never
  recreated), while the .flv recording shows the green Syntax converted and WYSIWYG editor updated notifications and no error. The browser console dump captured for another occurrence ends
  at Aborting the realtime session! with an empty REALTIME_DEBUG.logs afterwards and no
  SEVERE/WARNING entry, i.e. a silent early return rather than a thrown error.
• whenEditingModeIsSet() also resolves on destroy, so the promise passed to
  delayInstanceReady() can never hang if the editor is destroyed before its mode is loaded.
• No test change: isCollaborating() uses findElements(), which already waits for the implicit
  timeout, so the assertion is fine once the session is actually joined.

Screenshots & Video

Failure screenshot archived by CI for master build 1361, attached to XWIKI-24287 (the reloaded
editor is no longer in a realtime session — note the standard Save & View / Save / Preview / Cancel
button bar and the unchecked "Allow Realtime Collaboration" checkbox):

The VNC recording of the same failure is attached too:
https://jira.xwiki.org/secure/attachment/45330/syntaxChange-failure-master-1361.flv — around
01:24:22 it shows the realtime toolbar ("Done" + save status) being replaced by the standard button
bar while the green Syntax converted and WYSIWYG editor updated notifications are displayed, with
no error notification at any point.

Executed Tests

All realtime Docker ITs, on this change (requested by @mflorea when reviewing #6298) — Tomcat 11
containerised + PostgreSQL + Firefox, with xwiki.test.ui.offline=true so that the extension
installer could only resolve the locally built CKEditor webjar:

mvn clean verify -B -ntp -Pdocker,integration-tests \
  -Dxwiki.test.ui.servletEngine=tomcat -Dxwiki.test.ui.database=postgresql \
  -Dxwiki.test.ui.offline=true \
  -pl xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-wysiwyg/xwiki-platform-realtime-wysiwyg-test/xwiki-platform-realtime-wysiwyg-test-docker
# Tests run: 28, Failures: 0, Errors: 0, Skipped: 0 -- BUILD SUCCESS

mvn clean verify -B -ntp -Pdocker,integration-tests \
  -Dxwiki.test.ui.servletEngine=tomcat -Dxwiki.test.ui.database=postgresql \
  -Dxwiki.test.ui.offline=true \
  -pl xwiki-platform-core/xwiki-platform-realtime/xwiki-platform-realtime-wiki/xwiki-platform-realtime-wiki-test/xwiki-platform-realtime-wiki-test-docker
# Tests run: 2, Failures: 0, Errors: 0, Skipped: 0 -- BUILD SUCCESS

Module build with all checks on (Checkstyle, license, Revapi, Spoon):

mvn clean install -B -ntp -Plegacy \
  -pl xwiki-platform-core/xwiki-platform-ckeditor/xwiki-platform-ckeditor-plugins

Earlier, on #6298, syntaxChange alone was also run as a @RepeatedTest(10) on both browsers
(10/10 pass on Firefox, 10/10 on Chrome).

Since the race is rare (about 2 CI failures in the last 20 master builds of the environment-tests
job), it was also reproduced deterministically by temporarily wrapping editor.setMode() in the
plugin to delay the initial mode load by 3 seconds, so that the realtime bootstrap always wins the
race:

• with that delay and without this fix, syntaxChange fails every time, and
  Realtime debug info: {"logs":[]} confirms the session was never joined;
• with that delay and with this fix, syntaxChange passes (3/3).

That temporary instrumentation is not part of this PR.

Expected merging strategy

• Prefers squash: Yes
• Backport on branches: stable-18.7.x and stable-18.4.x — both contain f582505 and are
  therefore affected. stable-17.10.x and stable-16.10.x predate it and are not affected.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud