Skip to content

fix: initialize the writable Text editor only when needed - #2832

Open
arminfabritzek wants to merge 6 commits into
nextcloud:mainfrom
arminfabritzek:fix/lazy-text-editor-preview
Open

arminfabritzek wants to merge 6 commits into
nextcloud:mainfrom
arminfabritzek:fix/lazy-text-editor-preview

Conversation

@arminfabritzek

@arminfabritzek arminfabritzek commented Oct 1, 2026 •

Copy link
Copy Markdown

📝 Summary

Reading a Collectives page currently initializes a writable Text session and can lock the Markdown file against WebDAV updates. Defer that session until editing or an explicit attachment action, while keeping preview attachment metadata available through the reader.

Attachment rename/delete operations wait for editor readiness, verify saving before changing the file, update references, and verify the persisted Markdown afterwards. Page-bound initialization and serialized actions handle navigation and late callbacks. Partial failures retain an editor for retry and show an explicit warning. Transient Text save results are retried within a bounded loop; success requires a matching DAV readback, including checking that the editor has not changed during that readback.

Validation

Nextcloud 35.0.1 / Text 9.0.0 / Chromium 153:

  • 127 unit tests, lint, TypeScript and production build.
  • 30 attachment E2E checks (six cases repeated five times), including initialization failure, preflight failure and final-save recovery.
  • Nine lifecycle, page-content and anchor E2E checks.
  • Baseline countercheck reproduces HTTP423 for WebDAV writes while reading. The previous lazy-editor revision passes WebDAV but fails the three attachment regressions; the corrected implementation passes those cases.
  • Human checks completed for reading, attachment classification, rename, deletion outcome, editing, reload, navigation, empty pages, outline/anchors, focus and search. Saved files and Markdown were independently checked. The human tester could not confirm whether a transient message appeared during deletion; five automated deletion repetitions passed with success-message assertions. A subsequent focused diagnostic passed ten further repetitions, recording all toast messages from the delete click until five seconds after confirmed persistence: only the success message, with no error or partial-success message; DAV deletion, Markdown references and reload were independently verified. This does not retroactively establish the message shown in the human session.

Limitations: the existing public writable-share test also fails on the baseline and remains unresolved. The latest correction was not rerun on NC33 or other browser engines. Attachment file changes and Markdown persistence are not atomic; no automatic unlock or force-save is introduced. No complete E2E or version-matrix claim.

Development and review transparency

I developed this change with AI coding assistance and personally performed the manual UI acceptance checks listed above. AI coding agents ran the automated tests and reviewed the implementation in multiple rounds; those reviews identified attachment regressions, which were corrected and tested again. AI review and passing tests do not replace a human code review.

I am contributing as a user and cannot provide an expert code review myself. This implementation has not yet received a human code review. I would appreciate review by a maintainer or another experienced contributor, especially of editor lifecycle handling, attachment mutations and save confirmation. I am submitting it for review, not claiming it is ready to merge.

🚧 TODO

  • Human code review by a maintainer or experienced contributor, especially editor lifecycle, attachment mutations and save confirmation.
  • Assess whether broader browser and version coverage is needed before merging.

🏁 Checklist

  • JavaScript/TypeScript/Vue lint and type check pass; no style or PHP changes.
  • Sign-off message is added to all commits.
  • Unit and targeted end-to-end tests pass; changes are covered by regression and failure-path tests.
  • User documentation is not required for this internal initialization change.

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI tools.
  • The AI-generated content was reviewed, comprehended and tested by a human.

The AI checkbox remains unchecked because manual UI testing is complete, but a human review and comprehension of the implementation are still pending.

Signed-off-by: arminfabritzek <armin@starmin.de>
Signed-off-by: arminfabritzek <armin@starmin.de>
Verify attachment files and persisted Markdown after reload. Wait for completed Text initialization before the WebDAV probe, and exercise initialization rejection, navigation and save failures.

Signed-off-by: arminfabritzek <armin@starmin.de>
Await Text document readiness, serialize explicit attachment actions and keep their editor tied to the original page. Verify saved Markdown over DAV, report partial failures and allow initialization retries. Use reader attachment metadata in preview, including after editing.

Signed-off-by: arminfabritzek <armin@starmin.de>
Retry the explicit save when legacy Text resolves before the current Markdown reaches DAV. Keep the retry bounded and reject false or failed saves; cover stale-first-save recovery with a unit test.

Signed-off-by: arminfabritzek <armin@starmin.de>
Signed-off-by: arminfabritzek <armin@starmin.de>
@arminfabritzek arminfabritzek changed the title fix: avoid writable Text sessions when opening pages in preview fix: initialize the writable Text editor only when needed Oct 1, 2026
@arminfabritzek arminfabritzek reopened this Oct 1, 2026
@arminfabritzek
arminfabritzek marked this pull request as ready for review October 1, 2026 14:15

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant