| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Findings1. Medium - a non-settling toBlob permanently disables the export button and leaks a full workflow DOM clone into the live documentinvokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:382-391 awaits toBlob with no timeout and no abort. invokeai/frontend/web/src/features/nodes/components/flow/panels/BottomLeftPanel/ViewportControls.tsx:62-74 relies entirely on that promise settling: .finally(() => setIsExportingWorkflow(false)). Chain:
To expose this issue, add a test that stubs toBlob with a promise that never settles, races exportWorkflowAsPng against a deadline, and asserts the call rejects and that the staging wrapper has been removed. That test can only pass once a timeout is added around toBlob. 2. Medium - one un-embeddable image anywhere in the workflow aborts the whole exportgetWorkflowExportOptions (invokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:178-187) sets neither imagePlaceholder nor onImageErrorHandler. Chain:
Workflows routinely carry image references (invokeai/frontend/web/src/features/nodes/components/flow/nodes/Invocation/fields/inputs/ImageFieldInputComponent.tsx, invokeai/frontend/web/src/features/nodes/components/flow/nodes/CurrentImage/CurrentImageNode.tsx). A single deleted or 404 image makes the camera button produce only nodes.downloadWorkflowImageError with no indication of the cause. html-to-image also caches the empty result module-globally, so a transient failure poisons every later export in the session. To expose this issue, add a test that asserts getWorkflowExportOptions returns an imagePlaceholder (or onImageErrorHandler) so a failed image degrades to a placeholder instead of failing the export. 3. Medium - skipFonts: true renders the PNG in a fallback typefaceinvokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:186. Invoke ships a webfont: import '@fontsource-variable/inter' at invokeai/frontend/web/src/app/components/ThemeLocaleProvider.tsx:1. html-to-image serializes the clone into an SVG foreignObject encoded as a data:image/svg+xml URL and loads it through an <img>. SVG in <img> is secure static mode: no external resource loading. The only mechanism that would make Inter available is html-to-image's @font-face inlining, and skipFonts: true disables exactly that. The fontsource CSS is same-origin and bundled by Vite, so cssRules is readable and the inlining path would in fact succeed here. font-family is copied (line 69), so the PNG asks for Inter Variable and gets the browser default instead. Different metrics also shift every label width, compounding finding 4. This contradicts the PR's "Invoke color scheme ... preserved" framing, which holds for color but not for type. This is primarily a rendered-text concern and there is no approved DOM testing framework in this repo (no jsdom/happy-dom in invokeai/frontend/web/package.json), so it needs manual verification: export a workflow and compare the label typeface against the editor. 4. Medium - forced one-line field titles can be clipped at the image edge or overlap neighbouring nodessetWorkflowExportInputFieldTitleStyles (invokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:278-285) forces white-space: nowrap, overflow: visible, text-overflow: clip on every [data-node-input-field-title="true"]. Chain:
This defeats the PR's stated goal ("Input labels remain one line with full text") in exactly the case the override exists for - labels too long to fit. To expose this issue, add a test that exports getWorkflowContentBounds and asserts it widens the returned rect to cover a measured overflowing label, not only node bounds and edge bounding boxes. 5. Low - the includeStyleProperties allowlist silently drops flex-wrap and directionEXPORT_STYLE_PROPERTIES (invokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:6-98) replaces html-to-image's default "all computed properties" with a fixed 90-entry list. Anything absent is dropped from the capture. Two confirmed consumers:
To expose this issue, add a test that asserts EXPORT_STYLE_PROPERTIES contains direction and flex-wrap. 6. Low - the grid geometry is duplicated from Flow.tsx with nothing binding the twoGRID_GAP = 25 at invokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:112 duplicates const snapGrid: [number, number] = [25, 25] at invokeai/frontend/web/src/features/nodes/components/flow/Flow.tsx:91, which is what <Background gap={snapGrid} offset={snapGrid} /> (Flow.tsx:525) actually uses. setBackgroundGridForExport (workflowImageExport.ts:241-244) additionally hardcodes cx/cy/r = 0.5, which is only correct because <Background> is left at its default size={1} and the export pins zoom to 1. Changing snapGrid, or passing size/color to <Background>, desyncs the exported grid from the editor with no failing test and no compile error. To expose this issue, add a test that imports snapGrid from the flow module (exporting it if necessary) and asserts it equals the export module's grid gap. 7. Low - the export error is discardedinvokeai/frontend/web/src/features/nodes/components/flow/panels/BottomLeftPanel/ViewportControls.tsx:73 is .catch(handleWorkflowImageExportError), and the handler at lines 47-49 takes no arguments. The thrown error is never logged. Combined with findings 1 and 2, a field failure produces one generic toast string and nothing actionable. 8. Low - the export puts a duplicate id="workflow-editor" (and every other id in the subtree) into the live documentworkflowImageExport.ts:374 does flowElement.cloneNode(true), which copies ids, and line 380 appends it to flowElement.parentElement. flowElement is #workflow-editor itself (Flow.tsx:490; @xyflow/react applies the id prop to the wrapper div that also carries .react-flow). This is currently benign: the only other consumer, invokeai/frontend/web/src/features/nodes/hooks/useBuildNode.ts:26, uses document.querySelector, which returns the earlier original. But it is invalid HTML for the duration of the export, duplicates every Chakra-generated aria-labelledby/aria-describedby target and every react-flow SVG marker id, and breaks the moment anyone reaches for getElementById or a nth-match query. The pattern already applied to the background pattern id (workflowImageExport.ts:230) is not applied to the root or to markers. 9. Low - the added tests do not test the exportinvokeai/frontend/web/src/features/nodes/util/workflowImageExport.test.ts - 11 tests, all passing (verified, see Verification), but none of them can fail for a real defect in this feature:
To expose the selector risk, add a test that asserts the exact selector strings the helpers query, pinned against the data-* attributes added in this PR, so removing data-node-status-indicator / data-node-info-icon / data-node-input-field-title from the components fails a test rather than silently degrading the image. |
Sorry, something went wrong.
|
@Pfannkuchensack Thanks for the review. In the current commit:
All of the others are valid and I'm working on fixes now. |
Sorry, something went wrong.
|
@Pfannkuchensack Ready for a re-review. |
Sorry, something went wrong.
Findings1. Medium-High: aspect-ratio is missing from the style allowlist, so the Current Image node collapses in the exported PNGinvokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:9-103 (EXPORT_STYLE_PROPERTIES), consumed at invokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:216. Evidence chain:
Trigger: export any workflow containing a Current Image node, or a node with an image-collection input. The node renders collapsed in the PNG, while getNodesBounds reserved its full on-screen size and the edge paths were baked from live coordinates - so the node is both wrong-sized and detached from its connectors. To expose this issue, add a test that asserts EXPORT_STYLE_PROPERTIES contains every CSS property the workflow node tree depends on for layout, seeded with aspect-ratio (the same shape as the existing invokeai/frontend/web/src/features/nodes/util/workflowImageExport.test.ts:68-72 assertion, which already guards text-overflow / -webkit-line-clamp / -webkit-box-orient). 2. Medium: input-label overflow is measured against the pre-export layout, so wide labels are clipped by the canvas edgeMeasurement at invokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:171-186; the conflicting mutation at invokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:311-318. getWorkflowContentBounds grows the bounds using Math.max(labelRect.width, label.scrollWidth) read from the live DOM. The live label (invokeai/frontend/web/src/features/nodes/components/flow/nodes/Invocation/fields/InputFieldTitle.tsx:99-111) is noOfLines={1} plus labelSx display: 'flex', i.e. overflow:hidden with wrapping text. Measured in Chrome for a 120px-wide box with the label text "Positive Prompt Conditioning Collection Field":
So the clone renders the label ~116px wider than anything the bounds calculation could see, and EXPORT_PADDING is only 100. Trigger: a node on the right edge of the workflow with a multi-word input label wider than its field column - its label is cut off at the PNG border. The label-bounds branch is effectively dead code as written. The test that claims to cover this, invokeai/frontend/web/src/features/nodes/util/workflowImageExport.edgeCases.test.ts:132-148 ("includes overflowing input labels in content bounds"), passes scrollWidth: 100 for a rect of width: 100 - zero overflow. It never exercises the branch it is named after. To expose this issue, add a test that feeds getWorkflowContentBounds a label whose scrollWidth exceeds its getBoundingClientRect().width and asserts the returned width grows by the overflow amount; then add a test that fixes the ordering contract - that the value fed into bounds is measured with the export label styles applied, not the live clamped ones. 3. Low: exports wider than 8192 logical px silently drop below the documented 2x resolutioninvokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:191-198 and :209-220; documented as 2x in the docs card added by this PR. canvasWidth = width * EXPORT_SCALE with pixelRatio: 1. In html-to-image@1.11.13 lib/index.js:89-95, canvas.width = canvasWidth * ratio, then checkCanvasDimensions runs because skipAutoScale is not set. lib/util.js:153-177 clamps any dimension above 16384 and rescales the other proportionally. So once padded content exceeds 8192 logical px in either axis (roughly 25 nodes laid out horizontally at NODE_WIDTH 320 plus spacing - routine for real workflows), the output silently falls below 2x, with no log line and no user-visible notice. The claim "2x resolution" in the PR description and in docs/src/content/docs/features/Workflows/editor-interface.mdx is unconditional. To expose this issue, add a test that asserts getWorkflowImageDimensions either clamps canvasWidth/canvasHeight to the 16384 limit itself or reports the effective scale, so the degradation is explicit rather than delegated to library-internal auto-scaling. 4. Low: hardcoded English 'My Workflow' filename fallback bypasses the existing translation keyinvokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:117 and :231-241. getInitialWorkflow() in invokeai/frontend/web/src/features/nodes/store/nodesSlice.ts:112-114 sets name: '', so every unsaved workflow exports as My Workflow.png. The filename is user-visible (download dialog, file system), and the repository already localizes this exact concept - workflows.unnamedWorkflow = "Unnamed Workflow" exists at invokeai/frontend/web/public/locales/en.json:2747. The new string is not in en.json at all. The two strings the PR did add (nodes.downloadWorkflowImage, nodes.downloadWorkflowImageError) are correctly keyed and consumed via t() at invokeai/frontend/web/src/features/nodes/components/flow/panels/BottomLeftPanel/ViewportControls.tsx:58,112-113. To expose this issue, add a test that asserts the blank-name fallback resolves through a translation key rather than the module-local DEFAULT_WORKFLOW_IMAGE_FILENAME constant (sanitizeWorkflowImageFilename would need to take the fallback as an argument). 5. Low: the click handler has no test coverage at allinvokeai/frontend/web/src/features/nodes/components/flow/panels/BottomLeftPanel/ViewportControls.tsx:64-88. Three behaviors the PR description calls out are untested: the duplicate-click guard (isExportingWorkflow), the missing-#workflow-editor path that fires the error toast with no logged error, and the .catch -> toast wiring. All export tests target invokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts helpers only. There is no approved DOM testing framework in this repo, so a rendered-component test is not available; the reachable coverage is to extract the guard/dispatch logic into a plain function under invokeai/frontend/web/src/features/nodes/util/ and unit-test it with vitest. Otherwise this needs an explicit manual-verification note: double-click the camera button during a slow export and confirm only one download fires, and delete #workflow-editor from the DOM and confirm the error toast appears. |
Sorry, something went wrong.
Findings1. Medium-High: aspect-ratio is missing from the style allowlist, so the Current Image node collapses in the exported PNGinvokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:9-103 (EXPORT_STYLE_PROPERTIES), consumed at invokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:216. Evidence chain:
Trigger: export any workflow containing a Current Image node, or a node with an image-collection input. The node renders collapsed in the PNG, while getNodesBounds reserved its full on-screen size and the edge paths were baked from live coordinates - so the node is both wrong-sized and detached from its connectors. To expose this issue, add a test that asserts EXPORT_STYLE_PROPERTIES contains every CSS property the workflow node tree depends on for layout, seeded with aspect-ratio (the same shape as the existing invokeai/frontend/web/src/features/nodes/util/workflowImageExport.test.ts:68-72 assertion, which already guards text-overflow / -webkit-line-clamp / -webkit-box-orient). 2. Medium: input-label overflow is measured against the pre-export layout, so wide labels are clipped by the canvas edgeMeasurement at invokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:171-186; the conflicting mutation at invokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:311-318. getWorkflowContentBounds grows the bounds using Math.max(labelRect.width, label.scrollWidth) read from the live DOM. The live label (invokeai/frontend/web/src/features/nodes/components/flow/nodes/Invocation/fields/InputFieldTitle.tsx:99-111) is noOfLines={1} plus labelSx display: 'flex', i.e. overflow:hidden with wrapping text. Measured in Chrome for a 120px-wide box with the label text "Positive Prompt Conditioning Collection Field":
So the clone renders the label ~116px wider than anything the bounds calculation could see, and EXPORT_PADDING is only 100. Trigger: a node on the right edge of the workflow with a multi-word input label wider than its field column - its label is cut off at the PNG border. The label-bounds branch is effectively dead code as written. The test that claims to cover this, invokeai/frontend/web/src/features/nodes/util/workflowImageExport.edgeCases.test.ts:132-148 ("includes overflowing input labels in content bounds"), passes scrollWidth: 100 for a rect of width: 100 - zero overflow. It never exercises the branch it is named after. To expose this issue, add a test that feeds getWorkflowContentBounds a label whose scrollWidth exceeds its getBoundingClientRect().width and asserts the returned width grows by the overflow amount; then add a test that fixes the ordering contract - that the value fed into bounds is measured with the export label styles applied, not the live clamped ones. 3. Low: exports wider than 8192 logical px silently drop below the documented 2x resolutioninvokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:191-198 and :209-220; documented as 2x in the docs card added by this PR. canvasWidth = width * EXPORT_SCALE with pixelRatio: 1. In html-to-image@1.11.13 lib/index.js:89-95, canvas.width = canvasWidth * ratio, then checkCanvasDimensions runs because skipAutoScale is not set. lib/util.js:153-177 clamps any dimension above 16384 and rescales the other proportionally. So once padded content exceeds 8192 logical px in either axis (roughly 25 nodes laid out horizontally at NODE_WIDTH 320 plus spacing - routine for real workflows), the output silently falls below 2x, with no log line and no user-visible notice. The claim "2x resolution" in the PR description and in docs/src/content/docs/features/Workflows/editor-interface.mdx is unconditional. To expose this issue, add a test that asserts getWorkflowImageDimensions either clamps canvasWidth/canvasHeight to the 16384 limit itself or reports the effective scale, so the degradation is explicit rather than delegated to library-internal auto-scaling. 4. Low: hardcoded English 'My Workflow' filename fallback bypasses the existing translation keyinvokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts:117 and :231-241. getInitialWorkflow() in invokeai/frontend/web/src/features/nodes/store/nodesSlice.ts:112-114 sets name: '', so every unsaved workflow exports as My Workflow.png. The filename is user-visible (download dialog, file system), and the repository already localizes this exact concept - workflows.unnamedWorkflow = "Unnamed Workflow" exists at invokeai/frontend/web/public/locales/en.json:2747. The new string is not in en.json at all. The two strings the PR did add (nodes.downloadWorkflowImage, nodes.downloadWorkflowImageError) are correctly keyed and consumed via t() at invokeai/frontend/web/src/features/nodes/components/flow/panels/BottomLeftPanel/ViewportControls.tsx:58,112-113. To expose this issue, add a test that asserts the blank-name fallback resolves through a translation key rather than the module-local DEFAULT_WORKFLOW_IMAGE_FILENAME constant (sanitizeWorkflowImageFilename would need to take the fallback as an argument). 5. Low: the click handler has no test coverage at allinvokeai/frontend/web/src/features/nodes/components/flow/panels/BottomLeftPanel/ViewportControls.tsx:64-88. Three behaviors the PR description calls out are untested: the duplicate-click guard (isExportingWorkflow), the missing-#workflow-editor path that fires the error toast with no logged error, and the .catch -> toast wiring. All export tests target invokeai/frontend/web/src/features/nodes/util/workflowImageExport.ts helpers only. There is no approved DOM testing framework in this repo, so a rendered-component test is not available; the reachable coverage is to extract the guard/dispatch logic into a plain function under invokeai/frontend/web/src/features/nodes/util/ and unit-test it with vitest. Otherwise this needs an explicit manual-verification note: double-click the camera button during a slow export and confirm only one download fires, and delete #workflow-editor from the DOM and confirm the error toast appears. |
Sorry, something went wrong.
|
@Pfannkuchensack Fixed:
I'm not making the click-handler DOM test. There's no approved DOM test framework, and this is simple guarded/error-handled logic. Utility and integration coverage are present. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Summary
Adds camera-button workflow PNG export.
QA Instructions
Build the frontend, click the camera icon when in Workflows. An empty workflow will render a small grid to the PNG, while a real workflow will render the entire workflow.
Related Issues / Discussions
Closes #5076
Merge Plan
Checklist