Embed-create: postMessage host on succeeded / failed (closes #95) by Jaggob · Pull Request #96 · Jaggob/etherpad-integration-for-nextcloud

Jaggob · 2026-05-22T20:59:34Z

Closes #95.

Problem

The /embed/create-by-parent flow used to handle errors (including the common 409 on duplicate filename) only via an inline error in the iframe. From the host page's perspective the iframe loaded successfully — there was no programmatic signal whether the create actually happened, failed on validation, or hit a conflict. Most visible symptom: when the target name already existed, the host UI behaved as if a new pad had opened silently.

What this PR adds

Two structured postMessage events from src/embed-create-main.js to window.parent, matching the convention the open-flow already has (epnc:sync-flush-*):

Event	When	Payload
`epnc:create-succeeded`	Just before `window.location.replace(embed_url)` on the happy path	`{ embed_url, file_id, pad_id, access_mode }`
`epnc:create-failed`	On any error path	`{ reason, status, message }`

reason buckets the cause so hosts can branch without parsing HTTP status codes:

invalid — client-side validation (missing name, invalid access mode, missing CSRF, incomplete config)
conflict — backend returned 409 (e.g. duplicate filename)
server — any other 4xx / 5xx
network — fetch itself failed (offline, CORS, timeout)

Target-origin is * because the create page doesn't know the host's origin up-front; the route's frame-ancestors CSP (driven by trusted_embed_origins) already constrains who can be the parent, so by construction the receiver is allowlisted.

Inline error rendering is unchanged — postMessage is purely additive for hosts that want to react programmatically.

What this PR does NOT change

Backend behavior: PadFileAlreadyExistsException already maps to HTTP 409 via PadControllerErrorMapper; the bug was always frontend-only.
Existing inline error UX: users who see the iframe still get the same error message text.
The open-flow embed: this is a one-way event from the create launcher; the open-flow handshake stays as-is.

Tests

6 new vitest cases in tests/js/embed-create-main.test.js covering: success path, conflict (409), server (5xx), network error, client-side validation (missing name), and the not-actually-embedded no-op (window.parent === window).
JS suite: 98 → 104.
PHP suite unchanged at 395.

Docs

docs/architecture.md § "Create (trusted embed integration)" extended with the new event contract.

Caller-side follow-up (out of scope here)

The wechange frontend needs to add a window.addEventListener('message', …) filtering for these two events and trusting the iframe's origin. We'll coordinate that on their side.

Closes #95. The /embed/create-by-parent flow used to handle 409 (and any other error) only via an inline error message in the iframe. From the host page's perspective the iframe loaded successfully — there was no signal whether the create actually happened, failed on a duplicate name, hit a server error, or was rejected at validation time. The most visible symptom: when the target name already existed, the host UI behaved as if a new pad opened silently. Add the same kind of structured postMessage events the open-flow already has (epnc:sync-flush-*): - epnc:create-succeeded {embed_url, file_id, pad_id, access_mode} fires once on the happy path, immediately before the iframe self-redirects to embed_url. - epnc:create-failed {reason, status, message} fires on any error path. reason buckets the cause so hosts can branch without parsing HTTP status: invalid — client-side validation (missing name, etc.) conflict — backend returned 409 (duplicate filename) server — any other 4xx/5xx network — fetch itself failed (offline, CORS, timeout) Target-origin is '*' because the create page doesn't know the host's origin up-front (the host hasn't talked to us yet). The actual access control already happens at iframe-load time via the route's frame-ancestors CSP, which only lists admin-configured trusted_embed_origins — anyone receiving these messages is by construction in that allowlist. Inline error rendering is unchanged. postMessage is purely additive for hosts that want to act on the outcome programmatically. Backend is already correct — PadFileAlreadyExistsException maps to HTTP 409 via PadControllerErrorMapper; no PHP change in this PR. Tests: 6 new vitest cases in tests/js/embed-create-main.test.js covering success, conflict (409), server (5xx), network error, client-side validation (missing name), and the not-actually-embedded no-op case (window.parent === window). Docs: extended the architecture.md embed-create flow with the event contract. JS suite: 98 -> 104 passing. PHP suite unchanged at 395.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8cd919c0df

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copilot

Pull request overview

This PR adds a structured postMessage contract to the embed-create flow so host pages embedding /embed/create-by-parent/... can programmatically detect create success/failure (matching the existing embed-open messaging style), with accompanying Vitest coverage and documentation updates.

Changes:

Emit epnc:create-succeeded (pre-redirect) and epnc:create-failed (all error paths) from src/embed-create-main.js.
Add a new Vitest suite covering success, conflict (409), server (5xx), network error, client-side validation, and “not embedded” no-op behavior.
Extend architecture docs and update generated JS bundle artifacts to reflect the new runtime behavior.

Reviewed changes

Copilot reviewed 11 out of 20 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`src/embed-create-main.js`	Adds host `postMessage` helpers and emits create success/failure events.
`tests/js/embed-create-main.test.js`	New Vitest coverage for embed-create event emission + redirect/error behavior.
`docs/architecture.md`	Documents the new embed-create event contract and payloads.
`js/etherpad_nextcloud-embed-create-main.mjs`	Rebuilt production bundle for embed-create with the new messaging behavior.
`js/etherpad_nextcloud-embed-create-main.mjs.map`	Source map update for rebuilt embed-create bundle.
`js/fetch-helpers-C4MxuNvt.chunk.mjs`	Rebuilt chunk artifact (hash changed).
`js/fetch-helpers-C4MxuNvt.chunk.mjs.map`	Source map for rebuilt fetch-helpers chunk.
`js/fetch-helpers-C4MxuNvt.chunk.mjs.license`	Generated license file for rebuilt fetch-helpers chunk.
`js/oc-compat-hVqZy-MX.chunk.mjs`	Rebuilt chunk artifact (hash changed).
`js/oc-compat-hVqZy-MX.chunk.mjs.map`	Source map for rebuilt oc-compat chunk.
`js/oc-compat-hVqZy-MX.chunk.mjs.license`	Generated license file for rebuilt oc-compat chunk.
`js/api-client-BXEMiUh7.chunk.mjs`	Rebuilt api-client chunk artifact (hash changed).
`js/api-client-BXEMiUh7.chunk.mjs.map`	Source map for rebuilt api-client chunk.
`js/api-client-BXEMiUh7.chunk.mjs.license`	Generated license file for rebuilt api-client chunk.
`js/api-client-CT0tqdIR.chunk.mjs`	Removed old api-client chunk artifact (replaced by new hashed chunk).
`js/etherpad_nextcloud-embed-main.mjs`	Rebuilt embed-open bundle referencing new chunk hashes.
`js/etherpad_nextcloud-embed-main.mjs.map`	Source map update for rebuilt embed-open bundle.
`js/etherpad_nextcloud-files-main.mjs`	Rebuilt files integration bundle referencing new chunk hashes.
`js/etherpad_nextcloud-viewer-main.mjs`	Rebuilt viewer bundle referencing new chunk hashes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

… reason classification Addresses three codex / Copilot review findings on #96: 1. (P2) succeeded was sent *before* normalizeEmbedRedirectUrl ran, so a malformed or cross-origin embed_url from the server would first trigger 'epnc:create-succeeded' and then 'epnc:create-failed' from the catch path — leaving the host with contradictory signals for the same operation. Restructure into three explicit steps: (a) fetch -> network/server error on throw (b) validate response shape + redirect target -> server error (c) only now emit succeeded and replace location The success event is reached only when every prerequisite is definitively OK. 2. reason='network' was overly broad in the old catch — it fired for any error with status===null, including local exceptions thrown after a successful fetch (e.g. URL validation). The new split keeps 'network' for the actual fetch-level throw (status===null, nothing came back at all), and reclassifies post-fetch problems (bad response shape, cross-origin embed_url) as 'server', which matches what they actually are: the server gave us something unusable. 3. Test harness was redefining window.location without configurable: true, which would have made the property non-configurable after the first test and broken subsequent runs. Add configurable: true and explicitly restore the original location descriptor in afterEach, plus the same hygiene comment in beforeEach. One new regression test ('does not emit succeeded then failed when the server returns a cross-origin embed_url') locks in the sequencing fix. JS suite 104 -> 105.

Copilot

Pull request overview

Copilot reviewed 11 out of 20 changed files in this pull request and generated 2 comments.

…RF tests Two follow-up review points on #96: 1. failCreate() showed 'Unknown error.' inline when the message was empty/undefined but posted an empty-string message to the host. Normalise once and reuse the same value for both UI and postMessage so a host script never sees an empty message when the iframe is showing 'Unknown error.'. 2. The two 'invalid' branches that fire before fetch (incomplete embed config + missing CSRF token) weren't covered by the test suite. Add two targeted tests that verify the host receives a structured epnc:create-failed event in each case and that no fetch / redirect happens. JS suite 105 -> 107.

chatgpt-codex-connector Bot reviewed May 22, 2026

View reviewed changes

Comment thread src/embed-create-main.js Outdated

Jaggob requested a review from Copilot May 22, 2026 21:57

Copilot started reviewing on behalf of Jaggob May 22, 2026 21:57 View session

Copilot AI reviewed May 22, 2026

View reviewed changes

Comment thread tests/js/embed-create-main.test.js

Comment thread src/embed-create-main.js

Jaggob requested a review from Copilot May 23, 2026 09:05

Copilot started reviewing on behalf of Jaggob May 23, 2026 09:05 View session

Copilot AI reviewed May 23, 2026

View reviewed changes

Comment thread src/embed-create-main.js Outdated

Comment thread src/embed-create-main.js

Jaggob merged commit eb86c0b into main May 23, 2026
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Embed-create: postMessage host on succeeded / failed (closes #95)#96

Embed-create: postMessage host on succeeded / failed (closes #95)#96
Jaggob merged 3 commits into
mainfrom
feat/embed-create-postmessage

Jaggob commented May 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Jaggob commented May 22, 2026

Problem

What this PR adds

What this PR does NOT change

Tests

Docs

Caller-side follow-up (out of scope here)

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants