lhumina_code/hero_whiteboard

Fork 0

Web frame: delete leaves the iframe DOM overlay orphaned in the page #104

New issue

Open

opened 2026-04-29 10:33:04 +00:00 by AhmedHanafy725 · 3 comments

AhmedHanafy725 commented

2026-04-29 10:33:04 +00:00

Member

Summary

Deleting a webframe leaves the iframe DOM overlay (<div id="wf-overlay-<id>">) orphaned in the page. On the receiver side, the Konva placeholder disappears but the iframe stays floating on the canvas. On the originator, the same orphaning happens but the visible effect depends on whether the wrapper happens to be hidden at the moment of delete.

Steps to reproduce

Local

Place a webframe and let it load.
Select it and press Delete (or right-click → Delete).
The Konva placeholder vanishes; the iframe DOM element remains in the DOM (visible in DevTools as #wf-overlay-<id>).

Remote

Open the board in two tabs (A and B). Place a webframe in A.
Wait for the iframe to load in both.
Delete the webframe in tab A.
In tab B the Konva placeholder disappears but the iframe overlay stays visible at its old screen position.

Expected

The iframe overlay is removed from the DOM whenever the webframe is deleted — locally or via a remote object.deleted broadcast.

Actual

The Konva group is destroyed but the iframe <div> is leaked.

Root cause

WhiteboardWebframe.destroyOverlay(id) exists and is exported, but neither call site invokes it on delete:

Local delete: WhiteboardObjects.deleteObject(id) (in objects.js) destroys obj.group but doesn't dispatch type-specific cleanup.
Remote delete: the msg.type === 'object.deleted' handler in sync.js destroys obj.group but doesn't dispatch either.

Suggested fix

In both call sites, when obj.type === 'webframe', call WhiteboardWebframe.destroyOverlay(id) BEFORE obj.group.destroy(). Guard with typeof WhiteboardWebframe !== 'undefined' && WhiteboardWebframe.destroyOverlay for defensive consistency with the rest of the codebase.

If other types acquire DOM overlays in the future (e.g. video frames), consider a small _typeCleanup map in objects.js keyed by type → cleanup fn, called from a single _destroyTyped(obj, id) helper. Out of scope for this issue — direct call is fine for now.

Acceptance criteria

Local delete of a webframe removes both the Konva placeholder and the iframe <div>.
Remote delete of a webframe (via object.deleted broadcast) removes both on the receiver.
Undo of a deleted webframe restores both pieces (verify the existing undo path still works after the fix).
No regression to other object types' delete paths.

## Summary Deleting a webframe leaves the iframe DOM overlay (`<div id="wf-overlay-<id>">`) orphaned in the page. On the receiver side, the Konva placeholder disappears but the iframe stays floating on the canvas. On the originator, the same orphaning happens but the visible effect depends on whether the wrapper happens to be hidden at the moment of delete. ## Steps to reproduce ### Local 1. Place a webframe and let it load. 2. Select it and press Delete (or right-click → Delete). 3. The Konva placeholder vanishes; the iframe DOM element remains in the DOM (visible in DevTools as `#wf-overlay-<id>`). ### Remote 1. Open the board in two tabs (A and B). Place a webframe in A. 2. Wait for the iframe to load in both. 3. Delete the webframe in tab A. 4. In tab B the Konva placeholder disappears but the iframe overlay stays visible at its old screen position. ## Expected The iframe overlay is removed from the DOM whenever the webframe is deleted — locally or via a remote `object.deleted` broadcast. ## Actual The Konva group is destroyed but the iframe `<div>` is leaked. ## Root cause `WhiteboardWebframe.destroyOverlay(id)` exists and is exported, but neither call site invokes it on delete: - Local delete: `WhiteboardObjects.deleteObject(id)` (in `objects.js`) destroys `obj.group` but doesn't dispatch type-specific cleanup. - Remote delete: the `msg.type === 'object.deleted'` handler in `sync.js` destroys `obj.group` but doesn't dispatch either. ## Suggested fix In both call sites, when `obj.type === 'webframe'`, call `WhiteboardWebframe.destroyOverlay(id)` BEFORE `obj.group.destroy()`. Guard with `typeof WhiteboardWebframe !== 'undefined' && WhiteboardWebframe.destroyOverlay` for defensive consistency with the rest of the codebase. If other types acquire DOM overlays in the future (e.g. video frames), consider a small `_typeCleanup` map in `objects.js` keyed by type → cleanup fn, called from a single `_destroyTyped(obj, id)` helper. Out of scope for this issue — direct call is fine for now. ## Acceptance criteria - [ ] Local delete of a webframe removes both the Konva placeholder and the iframe `<div>`. - [ ] Remote delete of a webframe (via `object.deleted` broadcast) removes both on the receiver. - [ ] Undo of a deleted webframe restores both pieces (verify the existing undo path still works after the fix). - [ ] No regression to other object types' delete paths.

AhmedHanafy725 commented

2026-04-29 10:34:08 +00:00

Author

Member

Spec — Issue #104: Webframe delete must also remove the iframe DOM overlay

Objective

On every delete path (local user delete and remote object.deleted broadcast), destroy the iframe DOM overlay along with the Konva group. Currently both paths leak the <div id="wf-overlay-<id>">.

Root cause

WhiteboardWebframe.destroyOverlay(id) is exported but never called from the delete paths:

WhiteboardObjects.deleteObject(id) in objects.js destroys obj.group and skips type-specific cleanup.
sync.js msg.type === 'object.deleted' handler does the same on the receiver.

Files to Modify

crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js — add a webframe-specific cleanup in deleteObject.
crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js — add the same cleanup in the object.deleted handler.

Step-by-Step Plan

Step 1 — `objects.js::deleteObject`

Before obj.group.destroy(), add:

if (obj.type === 'webframe' && typeof WhiteboardWebframe !== 'undefined' && WhiteboardWebframe.destroyOverlay) {
    WhiteboardWebframe.destroyOverlay(id);
}

Step 2 — `sync.js` `object.deleted` handler

Before the existing obj.group.destroy() call (which currently runs after the frame-cleanup loop added in #96), add the symmetric block:

if (obj.type === 'webframe' && typeof WhiteboardWebframe !== 'undefined' && WhiteboardWebframe.destroyOverlay) {
    WhiteboardWebframe.destroyOverlay(delId);
}

Acceptance Criteria

Local delete: pressing Delete on a selected webframe removes both the Konva placeholder and the <div id="wf-overlay-<id>"> from the DOM.
Remote delete: deleting a webframe in tab A removes both pieces in tab B.
Undo of a deleted webframe restores both pieces (existing undo path).
No regression to other object types' delete paths.

Notes

destroyOverlay is idempotent on missing ids — already guards on overlays[id] lookup.
The temporary-id remap path (remapOverlay(tempId, serverId)) is unaffected; remap moves the entry to a new key but destroyOverlay looks up the current id, so deletes after remap still hit it.

# Spec — Issue #104: Webframe delete must also remove the iframe DOM overlay ## Objective On every delete path (local user delete and remote `object.deleted` broadcast), destroy the iframe DOM overlay along with the Konva group. Currently both paths leak the `<div id="wf-overlay-<id>">`. ## Root cause `WhiteboardWebframe.destroyOverlay(id)` is exported but never called from the delete paths: - `WhiteboardObjects.deleteObject(id)` in `objects.js` destroys `obj.group` and skips type-specific cleanup. - `sync.js` `msg.type === 'object.deleted'` handler does the same on the receiver. ## Files to Modify - `crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js` — add a webframe-specific cleanup in `deleteObject`. - `crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js` — add the same cleanup in the `object.deleted` handler. ## Step-by-Step Plan ### Step 1 — `objects.js::deleteObject` Before `obj.group.destroy()`, add: ```js if (obj.type === 'webframe' && typeof WhiteboardWebframe !== 'undefined' && WhiteboardWebframe.destroyOverlay) { WhiteboardWebframe.destroyOverlay(id); } ``` ### Step 2 — `sync.js` `object.deleted` handler Before the existing `obj.group.destroy()` call (which currently runs after the frame-cleanup loop added in #96), add the symmetric block: ```js if (obj.type === 'webframe' && typeof WhiteboardWebframe !== 'undefined' && WhiteboardWebframe.destroyOverlay) { WhiteboardWebframe.destroyOverlay(delId); } ``` ## Acceptance Criteria - [ ] Local delete: pressing Delete on a selected webframe removes both the Konva placeholder and the `<div id="wf-overlay-<id>">` from the DOM. - [ ] Remote delete: deleting a webframe in tab A removes both pieces in tab B. - [ ] Undo of a deleted webframe restores both pieces (existing undo path). - [ ] No regression to other object types' delete paths. ## Notes - `destroyOverlay` is idempotent on missing ids — already guards on `overlays[id]` lookup. - The temporary-id remap path (`remapOverlay(tempId, serverId)`) is unaffected; remap moves the entry to a new key but `destroyOverlay` looks up the current id, so deletes after remap still hit it.

AhmedHanafy725 commented

2026-04-29 10:35:08 +00:00

Author

Member

Test Results

cargo fmt --all -- --check: pass
cargo check --workspace: pass
cargo clippy --workspace -- -D warnings: pass
node --check objects.js: pass
node --check sync.js: pass

## Test Results - cargo fmt --all -- --check: pass - cargo check --workspace: pass - cargo clippy --workspace -- -D warnings: pass - node --check objects.js: pass - node --check sync.js: pass

AhmedHanafy725 commented

2026-04-29 10:35:12 +00:00

Author

Member

Implementation Summary

Two files changed, +6/-0. Adds WhiteboardWebframe.destroyOverlay(id) to both delete paths so the iframe <div> no longer leaks.

`crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js`

In deleteObject(id), before obj.group.destroy():

if (obj.type === 'webframe' && typeof WhiteboardWebframe !== 'undefined' && WhiteboardWebframe.destroyOverlay) {
    WhiteboardWebframe.destroyOverlay(id);
}

`crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js`

In the object.deleted handler, mirrored: before obj.group.destroy() on the receiver:

if (obj.type === 'webframe' && typeof WhiteboardWebframe !== 'undefined' && WhiteboardWebframe.destroyOverlay) {
    WhiteboardWebframe.destroyOverlay(delId);
}

Verification

cargo fmt --all -- --check: clean
cargo check --workspace: clean
cargo clippy --workspace -- -D warnings: clean
node --check on objects.js and sync.js: clean

Manual smoke test

Place a webframe locally, then delete it (Delete key) — both the Konva placeholder and the iframe <div> are removed (verify via DevTools that wf-overlay-<id> is gone).
In two windows, place a webframe in tab A, delete it in tab A — tab B drops both pieces.
Place a webframe and Ctrl+Z after deleting — the existing undo path still restores the webframe correctly.

Notes

destroyOverlay is already idempotent (guards on overlays[id]).
The temporary-id remap path is unaffected — remapOverlay moves the entry to the new server id, and destroyOverlay looks up the current id, so deletes after remap still hit it.

## Implementation Summary Two files changed, +6/-0. Adds `WhiteboardWebframe.destroyOverlay(id)` to both delete paths so the iframe `<div>` no longer leaks. ### `crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js` In `deleteObject(id)`, before `obj.group.destroy()`: ```js if (obj.type === 'webframe' && typeof WhiteboardWebframe !== 'undefined' && WhiteboardWebframe.destroyOverlay) { WhiteboardWebframe.destroyOverlay(id); } ``` ### `crates/hero_whiteboard_ui/static/web/js/whiteboard/sync.js` In the `object.deleted` handler, mirrored: before `obj.group.destroy()` on the receiver: ```js if (obj.type === 'webframe' && typeof WhiteboardWebframe !== 'undefined' && WhiteboardWebframe.destroyOverlay) { WhiteboardWebframe.destroyOverlay(delId); } ``` ### Verification - `cargo fmt --all -- --check`: clean - `cargo check --workspace`: clean - `cargo clippy --workspace -- -D warnings`: clean - `node --check` on `objects.js` and `sync.js`: clean ### Manual smoke test 1. Place a webframe locally, then delete it (Delete key) — both the Konva placeholder and the iframe `<div>` are removed (verify via DevTools that `wf-overlay-<id>` is gone). 2. In two windows, place a webframe in tab A, delete it in tab A — tab B drops both pieces. 3. Place a webframe and Ctrl+Z after deleting — the existing undo path still restores the webframe correctly. ### Notes - `destroyOverlay` is already idempotent (guards on `overlays[id]`). - The temporary-id remap path is unaffected — `remapOverlay` moves the entry to the new server id, and `destroyOverlay` looks up the current id, so deletes after remap still hit it.