From 95e616b89404fe2bda5be4dc6194aeac07a65a0b Mon Sep 17 00:00:00 2001 From: jaeone94 <89377375+jaeone94@users.noreply.github.com> Date: Thu, 14 May 2026 10:28:41 +0900 Subject: [PATCH] fix: clear media upload errors via widget change (#12212) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Clear missing media validation errors after paste/drop media uploads by emitting the existing widget-change event path. ## Changes - **What**: Emit `node.onWidgetChanged` after image/video upload completion updates the file combo widget. - **What**: Emit the same widget-change path after Load Audio upload completion. - **What**: Add unit coverage for upload completion emitting `onWidgetChanged` and for missing media clearing through that existing hook path. - **What**: Add E2E coverage for Load Image drag/drop and paste clearing validation rings, with red/green verified from a fresh `main` base. - **Dependencies**: None. ## Review Focus Please check that paste/drop upload paths now reuse the existing widget-change error-clearing path instead of expanding `widget.callback` patching. Also check the Load Image E2E helper path for synthetic paste/drop behavior. Supersedes #12207. Ref: FE-687 ## Screenshots Before https://github.com/user-attachments/assets/2cee52bc-b1c8-4dff-8a02-5b18a69ae639 After https://github.com/user-attachments/assets/e1ecd147-1d8a-470e-b77d-13345d473ef3 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12212-fix-clear-media-upload-errors-via-widget-change-35f6d73d365081bcb1a0dfc042d417eb) by [Unito](https://www.unito.io) --- .../fixtures/helpers/ClipboardHelper.ts | 114 +++++++++++----- .../tests/vueNodes/nodeStates/error.spec.ts | 126 ++++++++++++++++++ .../graph/useErrorClearingHooks.test.ts | 42 ++++++ src/extensions/core/uploadAudio.test.ts | 9 +- src/extensions/core/uploadAudio.ts | 4 + .../composables/useImageUploadWidget.test.ts | 113 ++++++++++++++++ .../composables/useImageUploadWidget.ts | 7 + 7 files changed, 378 insertions(+), 37 deletions(-) create mode 100644 src/renderer/extensions/vueNodes/widgets/composables/useImageUploadWidget.test.ts diff --git a/browser_tests/fixtures/helpers/ClipboardHelper.ts b/browser_tests/fixtures/helpers/ClipboardHelper.ts index 36acb5b0b9..48012f81a3 100644 --- a/browser_tests/fixtures/helpers/ClipboardHelper.ts +++ b/browser_tests/fixtures/helpers/ClipboardHelper.ts @@ -6,6 +6,71 @@ import type { Locator, Page } from '@playwright/test' import type { KeyboardHelper } from '@e2e/fixtures/helpers/KeyboardHelper' import { getMimeType } from '@e2e/fixtures/utils/mimeTypeUtil' +function readFilePayload(filePath: string) { + const buffer = readFileSync(filePath) + const bufferArray = [...new Uint8Array(buffer)] + const fileName = basename(filePath) + const fileType = getMimeType(fileName) + + return { bufferArray, fileName, fileType } +} + +async function dispatchFilePaste( + page: Page, + payload: ReturnType +): Promise { + await page.evaluate(({ bufferArray, fileName, fileType }) => { + const file = new File([new Uint8Array(bufferArray)], fileName, { + type: fileType + }) + const dataTransfer = new DataTransfer() + dataTransfer.items.add(file) + + const target = document.activeElement ?? document + target.dispatchEvent( + new ClipboardEvent('paste', { + clipboardData: dataTransfer, + bubbles: true, + cancelable: true + }) + ) + }, payload) +} + +async function interceptNextFilePaste( + page: Page, + payload: ReturnType +): Promise { + await page.evaluate(({ bufferArray, fileName, fileType }) => { + document.addEventListener( + 'paste', + (e: ClipboardEvent) => { + e.preventDefault() + e.stopImmediatePropagation() + + const file = new File([new Uint8Array(bufferArray)], fileName, { + type: fileType + }) + const dataTransfer = new DataTransfer() + dataTransfer.items.add(file) + + document.dispatchEvent( + new ClipboardEvent('paste', { + clipboardData: dataTransfer, + bubbles: true, + cancelable: true + }) + ) + }, + { capture: true, once: true } + ) + }, payload) +} + +type PasteFileOptions = { + mode?: 'keyboard' | 'direct' +} + export class ClipboardHelper { constructor( private readonly keyboard: KeyboardHelper, @@ -20,43 +85,20 @@ export class ClipboardHelper { await this.keyboard.ctrlSend('KeyV', locator ?? null) } - async pasteFile(filePath: string): Promise { - const buffer = readFileSync(filePath) - const bufferArray = [...new Uint8Array(buffer)] - const fileName = basename(filePath) - const fileType = getMimeType(fileName) + async pasteFile( + filePath: string, + { mode = 'keyboard' }: PasteFileOptions = {} + ): Promise { + const payload = readFilePayload(filePath) - // Register a one-time capturing-phase listener that intercepts the next - // paste event and injects file data onto clipboardData. - await this.page.evaluate( - ({ bufferArray, fileName, fileType }) => { - document.addEventListener( - 'paste', - (e: ClipboardEvent) => { - e.preventDefault() - e.stopImmediatePropagation() + if (mode === 'keyboard') { + await interceptNextFilePaste(this.page, payload) + await this.paste() + return + } - const file = new File([new Uint8Array(bufferArray)], fileName, { - type: fileType - }) - const dataTransfer = new DataTransfer() - dataTransfer.items.add(file) - - const syntheticEvent = new ClipboardEvent('paste', { - clipboardData: dataTransfer, - bubbles: true, - cancelable: true - }) - document.dispatchEvent(syntheticEvent) - }, - { capture: true, once: true } - ) - }, - { bufferArray, fileName, fileType } - ) - - // Trigger a real Ctrl+V keystroke — the capturing listener above will - // intercept it and re-dispatch with file data attached. - await this.paste() + // Browser clipboard APIs cannot reliably seed arbitrary files in tests. + // Dispatch the app-level paste event with file clipboardData directly. + await dispatchFilePaste(this.page, payload) } } diff --git a/browser_tests/tests/vueNodes/nodeStates/error.spec.ts b/browser_tests/tests/vueNodes/nodeStates/error.spec.ts index f64d1e1376..85354560c8 100644 --- a/browser_tests/tests/vueNodes/nodeStates/error.spec.ts +++ b/browser_tests/tests/vueNodes/nodeStates/error.spec.ts @@ -4,6 +4,7 @@ import { comfyExpect as expect, comfyPageFixture } from '@e2e/fixtures/ComfyPage' +import type { ComfyPage } from '@e2e/fixtures/ComfyPage' import { cleanupFakeModel, dismissErrorOverlay, @@ -13,7 +14,9 @@ import { ExecutionHelper, buildKSamplerError } from '@e2e/fixtures/helpers/ExecutionHelper' +import type { NodeError } from '@/schemas/apiSchema' import { fitToViewInstant } from '@e2e/fixtures/utils/fitToView' +import { assetPath } from '@e2e/fixtures/utils/paths' import { webSocketFixture } from '@e2e/fixtures/ws' const test = mergeTests(comfyPageFixture, webSocketFixture) @@ -22,6 +25,61 @@ const ERROR_CLASS = /ring-destructive-background/ const UNKNOWN_NODE_ID = '1' const INNER_EXECUTION_ID = '2:1' const KSAMPLER_MODEL_INPUT_NAME = 'model' +const LOAD_IMAGE_INPUT_NAME = 'image' +const LOAD_IMAGE_UPLOAD_FILE = 'test_upload_image.png' + +function buildLoadImageRequiredInputError(): NodeError { + return { + class_type: 'LoadImage', + dependent_outputs: [], + errors: [ + { + type: 'required_input_missing', + message: `Required input is missing: ${LOAD_IMAGE_INPUT_NAME}`, + details: '', + extra_info: { input_name: LOAD_IMAGE_INPUT_NAME } + } + ] + } +} + +async function surfaceLoadImageMissingInputError( + comfyPage: ComfyPage, + loadImageId: string +): Promise { + const exec = new ExecutionHelper(comfyPage) + await exec.mockValidationFailure({ + [loadImageId]: buildLoadImageRequiredInputError() + }) + await comfyPage.runButton.click() + await dismissErrorOverlay(comfyPage) +} + +async function selectLoadImageNodeForPaste( + comfyPage: ComfyPage, + loadImageId: string +): Promise { + await comfyPage.page.evaluate((nodeId) => { + const node = window.app!.graph.getNodeById(Number(nodeId)) + if (!node) throw new Error(`Load Image node ${nodeId} not found`) + window.app!.canvas.selectNode(node) + window.app!.canvas.current_node = node + }, loadImageId) +} + +async function setupLoadImageErrorScenario(comfyPage: ComfyPage) { + await comfyPage.workflow.loadWorkflow('widgets/load_image_widget') + const loadImageNode = ( + await comfyPage.nodeOps.getNodeRefsByType('LoadImage') + )[0] + const loadImageId = String(loadImageNode.id) + + return { + loadImageId, + innerWrapper: comfyPage.vueNodes.getNodeInnerWrapper(loadImageId), + imageWidget: await loadImageNode.getWidgetByName(LOAD_IMAGE_INPUT_NAME) + } +} test.describe('Vue Node Error', { tag: '@vue-nodes' }, () => { test('should display error state when node is missing (node from workflow is not installed)', async ({ @@ -191,6 +249,74 @@ test.describe('Vue Node Error', { tag: '@vue-nodes' }, () => { await expect(innerWrapper).not.toHaveClass(ERROR_CLASS) }) + + test('clears error ring when user drops an image file onto Load Image', async ({ + comfyPage + }) => { + const { loadImageId, innerWrapper, imageWidget } = + await setupLoadImageErrorScenario(comfyPage) + + await test.step('queue with missing image input to surface the error', async () => { + await surfaceLoadImageMissingInputError(comfyPage, loadImageId) + await expect(innerWrapper).toHaveClass(ERROR_CLASS) + }) + + await test.step('drop an image onto the Load Image node', async () => { + const dropPosition = + await comfyPage.canvasOps.getNodeCenterByTitle('Load Image') + if (!dropPosition) { + throw new Error('Load Image node center must be available for drop') + } + + await comfyPage.dragDrop.dragAndDropFile(LOAD_IMAGE_UPLOAD_FILE, { + dropPosition, + waitForUpload: true + }) + await expect + .poll(() => imageWidget.getValue()) + .toContain(LOAD_IMAGE_UPLOAD_FILE) + }) + + await expect(innerWrapper).not.toHaveClass(ERROR_CLASS) + }) + + test('clears error ring when user pastes an image file onto Load Image', async ({ + comfyPage + }) => { + const { loadImageId, innerWrapper, imageWidget } = + await setupLoadImageErrorScenario(comfyPage) + + await test.step('queue with missing image input to surface the error', async () => { + await surfaceLoadImageMissingInputError(comfyPage, loadImageId) + await expect(innerWrapper).toHaveClass(ERROR_CLASS) + }) + + await test.step('paste an image while Load Image is selected', async () => { + await comfyPage.canvas.focus() + await selectLoadImageNodeForPaste(comfyPage, loadImageId) + await expect + .poll(() => + comfyPage.page.evaluate(() => window.app!.canvas.current_node?.type) + ) + .toBe('LoadImage') + + const uploadResponse = comfyPage.page.waitForResponse( + (resp) => resp.url().includes('/upload/') && resp.status() === 200, + { timeout: 10_000 } + ) + // File clipboard contents cannot be seeded reliably in Playwright; + // use the direct document paste mode to exercise usePaste. + await comfyPage.clipboard.pasteFile(assetPath(LOAD_IMAGE_UPLOAD_FILE), { + mode: 'direct' + }) + await uploadResponse + await expect + .poll(() => imageWidget.getValue()) + .toContain(LOAD_IMAGE_UPLOAD_FILE) + }) + + await expect(innerWrapper).not.toHaveClass(ERROR_CLASS) + }) }) test.describe('subgraph propagation', { tag: '@subgraph' }, () => { diff --git a/src/composables/graph/useErrorClearingHooks.test.ts b/src/composables/graph/useErrorClearingHooks.test.ts index 0a4b351333..8b6b7564d8 100644 --- a/src/composables/graph/useErrorClearingHooks.test.ts +++ b/src/composables/graph/useErrorClearingHooks.test.ts @@ -21,6 +21,7 @@ import { useMissingNodesErrorStore } from '@/platform/nodeReplacement/missingNod import { app } from '@/scripts/app' import { useExecutionErrorStore } from '@/stores/executionErrorStore' import { seedRequiredInputMissingNodeError } from '@/utils/__tests__/executionErrorTestUtils' +import type { MissingMediaCandidate } from '@/platform/missingMedia/types' import type { MissingModelCandidate } from '@/platform/missingModel/types' beforeEach(() => { @@ -210,6 +211,47 @@ describe('Widget change error clearing via onWidgetChanged', () => { expect(store.lastNodeErrors).not.toBeNull() }) + it('clears missing media when an upload emits onWidgetChanged', () => { + const graph = new LGraph() + const node = new LGraphNode('LoadImage') + node.type = 'LoadImage' + const widget = node.addWidget( + 'combo', + 'image', + 'missing.png', + () => undefined, + { values: [] } + ) + graph.add(node) + installErrorClearingHooks(graph) + + const store = useExecutionErrorStore() + const mediaStore = useMissingMediaStore() + vi.spyOn(app, 'rootGraph', 'get').mockReturnValue(graph) + seedRequiredInputMissingNodeError(store, String(node.id), 'image') + mediaStore.setMissingMedia([ + { + nodeId: String(node.id), + nodeType: 'LoadImage', + widgetName: 'image', + mediaType: 'image', + name: 'missing.png', + isMissing: true + } satisfies MissingMediaCandidate + ]) + + node.onWidgetChanged!.call( + node, + 'image', + 'uploaded.png', + 'missing.png', + widget + ) + + expect(store.lastNodeErrors).toBeNull() + expect(mediaStore.missingMediaCandidates).toBeNull() + }) + it('uses interior node execution ID for promoted widget error clearing', () => { const subgraph = createTestSubgraph({ inputs: [{ name: 'ckpt_input', type: '*' }] diff --git a/src/extensions/core/uploadAudio.test.ts b/src/extensions/core/uploadAudio.test.ts index d123ae1ea3..4672dba189 100644 --- a/src/extensions/core/uploadAudio.test.ts +++ b/src/extensions/core/uploadAudio.test.ts @@ -130,7 +130,8 @@ function createAudioNode() { widgets: [audioWidget, audioUIWidget], isUploading: false, graph: { setDirtyCanvas: vi.fn() }, - addWidget: vi.fn(() => uploadWidget) + addWidget: vi.fn(() => uploadWidget), + onWidgetChanged: vi.fn() }) return { audioUIWidget, audioWidget, node, uploadWidget } @@ -180,6 +181,12 @@ describe('Comfy.UploadAudio AUDIOUPLOAD widget', () => { expect(node.isUploading).toBe(false) expect(audioWidget.value).toBe('pasted/uploaded.mp3') expect(audioWidget.options.values).toContain('pasted/uploaded.mp3') + expect(node.onWidgetChanged).toHaveBeenCalledWith( + 'audio', + 'pasted/uploaded.mp3', + 'clip.mp3', + audioWidget + ) expect(node.graph?.setDirtyCanvas).toHaveBeenCalledWith(true) }) diff --git a/src/extensions/core/uploadAudio.ts b/src/extensions/core/uploadAudio.ts index 0d680e3250..fb7eb5ddb1 100644 --- a/src/extensions/core/uploadAudio.ts +++ b/src/extensions/core/uploadAudio.ts @@ -38,6 +38,7 @@ function updateUIWidget( } async function uploadFile( + node: LGraphNode, audioWidget: IStringWidget, audioUIWidget: DOMWidget, file: File, @@ -67,6 +68,7 @@ async function uploadFile( } if (updateNode) { + const oldValue = audioWidget.value updateUIWidget( audioUIWidget, api.apiURL(getResourceURL(...splitFilePath(path))) @@ -75,6 +77,7 @@ async function uploadFile( audioWidget.value = path // Manually trigger the callback to update VueNodes audioWidget.callback?.(path) + node.onWidgetChanged?.(audioWidget.name, path, oldValue, audioWidget) } return true } else { @@ -247,6 +250,7 @@ app.registerExtension({ audioWidget.value = files[0].name try { const success = await uploadFile( + node, audioWidget, audioUIWidget, files[0], diff --git a/src/renderer/extensions/vueNodes/widgets/composables/useImageUploadWidget.test.ts b/src/renderer/extensions/vueNodes/widgets/composables/useImageUploadWidget.test.ts new file mode 100644 index 0000000000..225a0a59f2 --- /dev/null +++ b/src/renderer/extensions/vueNodes/widgets/composables/useImageUploadWidget.test.ts @@ -0,0 +1,113 @@ +import { fromPartial } from '@total-typescript/shoehorn' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { useImageUploadWidget } from '@/renderer/extensions/vueNodes/widgets/composables/useImageUploadWidget' +import { LGraphNode } from '@/lib/litegraph/src/litegraph' +import type { IComboWidget } from '@/lib/litegraph/src/types/widgets' +import type { ResultItem, ResultItemType } from '@/schemas/apiSchema' +import type { InputSpec } from '@/schemas/nodeDefSchema' + +type CapturedImageUploadOptions = { + onUploadComplete: (paths: (string | ResultItem)[]) => void + allow_batch?: boolean + folder?: ResultItemType + onUploadStart?: (files: File[]) => void + onUploadError?: () => void +} + +const mocks = vi.hoisted(() => ({ + capturedUploadOptions: undefined as CapturedImageUploadOptions | undefined, + openFileSelection: vi.fn(), + setNodeOutputs: vi.fn(), + showPreview: vi.fn() +})) + +vi.mock('@/composables/node/useNodeImage', () => ({ + useNodeImage: () => ({ showPreview: mocks.showPreview }), + useNodeVideo: () => ({ showPreview: mocks.showPreview }) +})) + +vi.mock('@/composables/node/useNodeImageUpload', () => ({ + useNodeImageUpload: ( + _node: LGraphNode, + options: CapturedImageUploadOptions + ) => { + mocks.capturedUploadOptions = options + return { openFileSelection: mocks.openFileSelection } + } +})) + +vi.mock('@/i18n', () => ({ + t: (key: string) => key +})) + +vi.mock('@/stores/nodeOutputStore', () => ({ + useNodeOutputStore: () => ({ + setNodeOutputs: mocks.setNodeOutputs + }) +})) + +vi.mock('@/utils/litegraphUtil', () => ({ + addToComboValues: (widget: IComboWidget, value: string) => { + const values = widget.options?.values + if (Array.isArray(values) && !values.includes(value)) { + values.push(value) + } + } +})) + +function createUploadNode() { + const onWidgetChanged = vi.fn() + const node = new LGraphNode('LoadImage') + node.type = 'LoadImage' + node.onWidgetChanged = onWidgetChanged + const fileComboWidget = node.addWidget( + 'combo', + 'image', + 'missing.png', + () => undefined, + { values: ['missing.png'] } + ) as IComboWidget + + return { fileComboWidget, node, onWidgetChanged } +} + +describe('useImageUploadWidget', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.capturedUploadOptions = undefined + vi.stubGlobal('requestAnimationFrame', vi.fn()) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('emits onWidgetChanged after upload changes the combo widget value', () => { + const { fileComboWidget, node, onWidgetChanged } = createUploadNode() + const constructor = useImageUploadWidget() + + constructor( + node, + 'upload', + [ + 'IMAGEUPLOAD', + { imageInputName: 'image', image_upload: true } + ] as InputSpec, + fromPartial({}) + ) + + mocks.capturedUploadOptions?.onUploadComplete(['uploaded.png']) + + expect(fileComboWidget.value).toBe('uploaded.png') + expect(mocks.setNodeOutputs).toHaveBeenCalledWith(node, 'uploaded.png', { + isAnimated: false + }) + expect(onWidgetChanged).toHaveBeenCalledWith( + 'image', + 'uploaded.png', + 'missing.png', + fileComboWidget + ) + }) +}) diff --git a/src/renderer/extensions/vueNodes/widgets/composables/useImageUploadWidget.ts b/src/renderer/extensions/vueNodes/widgets/composables/useImageUploadWidget.ts index 3c117007ce..da4e45f89c 100644 --- a/src/renderer/extensions/vueNodes/widgets/composables/useImageUploadWidget.ts +++ b/src/renderer/extensions/vueNodes/widgets/composables/useImageUploadWidget.ts @@ -83,10 +83,17 @@ export const useImageUploadWidget = () => { }) const newValue = allow_batch ? annotated : annotated[0] + const oldValue = fileComboWidget.value // @ts-expect-error litegraph combo value type does not support arrays yet fileComboWidget.value = newValue fileComboWidget.callback?.(newValue) + node.onWidgetChanged?.( + fileComboWidget.name, + newValue, + oldValue, + fileComboWidget + ) } })