Comfy-Org / Comfy-Org/ComfyUI_frontend

Node shrinks by title height on move after a ResizeObserver flush (LayoutSource.DOM leaks)

Open
#14,840 1 comment 1 reaction 0 assignees View on GitHub
area:nodes Potential Bug Public API
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

> 🤖 Filed by an LLM agent using DrJKL's account, not by DrJKL.

### Summary

`layoutStore.batchUpdateNodeBounds` conditionally subtracts `NODE_TITLE_HEIGHT` from the bounds it is given, gated on `LayoutSource` — which is ambient global state that leaks. After any Vue-node `ResizeObserver` flush, subsequent *move* operations silently shrink the node by one title height per call, and report a size change that fires `LGraphNode.onResize`.

### Mechanism

1. `useVueNodeResizeTracking.ts:254` sets the source and never restores it:

```ts
if (updatesByType.size > 0) {
layoutStore.setSource(LayoutSource.DOM) // no restore
...
}
```

2. `batchUpdateNodeBounds` restores `currentSource` to `originalSource` (`layoutStore.ts:1328`) — i.e. back to `DOM`. So the source stays `DOM` until some other writer sets it.

3. While the source is `DOM`, `batchUpdateNodeBounds` rewrites the caller's height (`layoutStore.ts:1294-1306`):

```ts
const shouldNormalizeHeights = originalSource === LayoutSource.DOM
...
boundsRecord[nodeId] = shouldNormalizeHeights
? { ...bounds, height: removeNodeTitleHeight(bounds.height) }
: bounds
```

This is correct for exactly one caller — the ResizeObserver handler, which passes raw border-box height. The other callers (`layoutMutations.batchMoveNodes`, `useNodeDrag` snap-on-release, `LGraphCanvas` paste) pass an already-normalized store height, so the subtraction is pure corruption.

4. `handleBatchUpdateBounds` then sees `rect[3] !== bounds.height` and pushes to `sizeChangedNodeIds` (`layoutStore.ts:1100-1106`), so `notifyLayoutChanges` fires `node.onResize(node.size)` for what the user experienced as a drag.

The `ResizeObserver` is a single module-level instance observing every Vue node, so a resize on one node poisons a drag of another. `useNodeDrag.ts:98` sets `Vue` at `startDrag`, which only protects the drag until the first mid-drag RO flush (preview image load, progress badge, widget hydration).

### Impact

- Node loses `NODE_TITLE_HEIGHT` per affected move.
- Spurious `onResize` callbacks — visible to custom nodes, so extension-facing.

### Suggested fix

Normalize at the call site. Have `useVueNodeResizeTracking.ts:73` subtract the title height before calling, so `batchUpdateNodeBounds` takes its `bounds` argument literally and `shouldNormalizeHeights` / the `originalSource` read can be deleted. `LayoutSource` should describe provenance for listeners, not gate a data transform.

### Related

Surfaced while reviewing #14133 (https://github.com/Comfy-Org/ComfyUI_frontend/pull/14133#discussion_r3725623457). Pre-existing — that PR does not touch `batchUpdateNodeBounds`, and the predecessor `useLayoutSync` fired `onResize` on the same corrupted write via its own size diff.

Contributor guide

Open the contributing guide

Research direction

Start with useVueNodeResizeTracking.ts around lines 73 and 254, then trace batchUpdateNodeBounds and handleBatchUpdateBounds in layoutStore.ts and the callers in layoutMutations, useNodeDrag.ts, and LGraphCanvas. Reproduce a ResizeObserver flush followed by a move, checking that the node height is not reduced and that the move does not trigger onResize; done when bounds are passed literally and LayoutSource no longer controls normalization.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
frontend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.