From dac3396de8cab4356550448d1fdd178f8aa9e335 Mon Sep 17 00:00:00 2001 From: Alexander Brown Date: Mon, 4 May 2026 13:30:42 -0700 Subject: [PATCH 001/148] test: add selection paste, rename, and batch rename browser tests (#11367) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *PR Created by the Glary-Bot Agent* --- ## Summary Adds `browser_tests/tests/selectionPasteRename.spec.ts` covering the untested `pasteSelection()` and `renameSelection()` paths in `useSelectionOperations.ts`. ### Coverage gaps filled - `pasteSelection()` — copy → paste creates new nodes - `renameSelection()` single node path — opens title editor - `renameSelection()` batch path — prompt dialog with sequential naming - Empty selection → toast warning ### References - Follows patterns from `selectionToolboxMoreActions.spec.ts` (More Options menu, `selectNodeWithPan`) - Follows `browser_tests/AGENTS.md` directory structure - Follows `browser_tests/FLAKE_PREVENTION_RULES.md` assertion patterns ### Verification - TypeScript: clean - ESLint: clean - oxlint: clean - oxfmt: formatted ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11367-test-add-selection-paste-rename-and-batch-rename-browser-tests-3466d73d3650812194a4d8bfbed3dee7) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot Co-authored-by: Amp --- browser_tests/fixtures/utils/groupHelpers.ts | 34 +++++++ .../fixtures/utils/selectionToolbox.ts | 20 ++++ browser_tests/tests/groupCopyPaste.spec.ts | 11 +-- .../tests/groupSelectChildren.spec.ts | 24 +---- .../tests/selectionToolboxMoreActions.spec.ts | 40 +++----- .../tests/selectionToolboxRename.spec.ts | 96 +++++++++++++++++++ 6 files changed, 166 insertions(+), 59 deletions(-) create mode 100644 browser_tests/fixtures/utils/groupHelpers.ts create mode 100644 browser_tests/fixtures/utils/selectionToolbox.ts create mode 100644 browser_tests/tests/selectionToolboxRename.spec.ts diff --git a/browser_tests/fixtures/utils/groupHelpers.ts b/browser_tests/fixtures/utils/groupHelpers.ts new file mode 100644 index 0000000000..37d1de232b --- /dev/null +++ b/browser_tests/fixtures/utils/groupHelpers.ts @@ -0,0 +1,34 @@ +import type { ComfyPage } from '@e2e/fixtures/ComfyPage' + +const GROUP_TITLE_CLICK_OFFSET_X = 50 +const GROUP_TITLE_CLICK_OFFSET_Y = 15 + +/** + * Returns the client-space position of a group's title bar (for clicking). + */ +export async function getGroupTitlePosition( + comfyPage: ComfyPage, + title: string +): Promise<{ x: number; y: number }> { + const pos = await comfyPage.page.evaluate( + ({ title, offsetX, offsetY }) => { + const app = window.app! + const group = app.graph.groups.find( + (g: { title: string }) => g.title === title + ) + if (!group) return null + const clientPos = app.canvasPosToClientPos([ + group.pos[0] + offsetX, + group.pos[1] + offsetY + ]) + return { x: clientPos[0], y: clientPos[1] } + }, + { + title, + offsetX: GROUP_TITLE_CLICK_OFFSET_X, + offsetY: GROUP_TITLE_CLICK_OFFSET_Y + } + ) + if (!pos) throw new Error(`Group "${title}" not found`) + return pos +} diff --git a/browser_tests/fixtures/utils/selectionToolbox.ts b/browser_tests/fixtures/utils/selectionToolbox.ts new file mode 100644 index 0000000000..d135df4a4d --- /dev/null +++ b/browser_tests/fixtures/utils/selectionToolbox.ts @@ -0,0 +1,20 @@ +import type { Locator } from '@playwright/test' + +import { comfyExpect as expect } from '@e2e/fixtures/ComfyPage' +import type { ComfyPage } from '@e2e/fixtures/ComfyPage' + +/** + * Opens the selection toolbox "More Options" menu and returns the menu + * locator so callers can scope follow-up queries to it. + */ +export async function openMoreOptions(comfyPage: ComfyPage): Promise { + await expect(comfyPage.selectionToolbox).toBeVisible() + + const moreOptionsBtn = comfyPage.page.getByTestId('more-options-button') + await expect(moreOptionsBtn).toBeVisible() + await moreOptionsBtn.click() + + const menu = comfyPage.page.locator('.p-contextmenu') + await expect(menu.getByText('Copy', { exact: true })).toBeVisible() + return menu +} diff --git a/browser_tests/tests/groupCopyPaste.spec.ts b/browser_tests/tests/groupCopyPaste.spec.ts index 8263b4aa4b..de6e514663 100644 --- a/browser_tests/tests/groupCopyPaste.spec.ts +++ b/browser_tests/tests/groupCopyPaste.spec.ts @@ -1,6 +1,7 @@ import { expect } from '@playwright/test' import { comfyPageFixture as test } from '@e2e/fixtures/ComfyPage' +import { getGroupTitlePosition } from '@e2e/fixtures/utils/groupHelpers' test.describe('Group Copy Paste', { tag: ['@canvas'] }, () => { test.afterEach(async ({ comfyPage }) => { @@ -12,15 +13,7 @@ test.describe('Group Copy Paste', { tag: ['@canvas'] }, () => { }) => { await comfyPage.workflow.loadWorkflow('groups/single_group_only') - const titlePos = await comfyPage.page.evaluate(() => { - const app = window.app! - const group = app.graph.groups[0] - const clientPos = app.canvasPosToClientPos([ - group.pos[0] + 50, - group.pos[1] + 15 - ]) - return { x: clientPos[0], y: clientPos[1] } - }) + const titlePos = await getGroupTitlePosition(comfyPage, 'Group') await comfyPage.canvas.click({ position: titlePos }) await comfyPage.nextFrame() diff --git a/browser_tests/tests/groupSelectChildren.spec.ts b/browser_tests/tests/groupSelectChildren.spec.ts index 931bbf8d94..263e488f47 100644 --- a/browser_tests/tests/groupSelectChildren.spec.ts +++ b/browser_tests/tests/groupSelectChildren.spec.ts @@ -2,29 +2,7 @@ import { expect } from '@playwright/test' import { comfyPageFixture as test } from '@e2e/fixtures/ComfyPage' import type { ComfyPage } from '@e2e/fixtures/ComfyPage' - -/** - * Returns the client-space position of a group's title bar (for clicking). - */ -async function getGroupTitlePosition( - comfyPage: ComfyPage, - title: string -): Promise<{ x: number; y: number }> { - const pos = await comfyPage.page.evaluate((title) => { - const app = window.app! - const group = app.graph.groups.find( - (g: { title: string }) => g.title === title - ) - if (!group) return null - const clientPos = app.canvasPosToClientPos([ - group.pos[0] + 50, - group.pos[1] + 15 - ]) - return { x: clientPos[0], y: clientPos[1] } - }, title) - if (!pos) throw new Error(`Group "${title}" not found`) - return pos -} +import { getGroupTitlePosition } from '@e2e/fixtures/utils/groupHelpers' /** * Returns {selectedNodeCount, selectedGroupCount, selectedItemCount} diff --git a/browser_tests/tests/selectionToolboxMoreActions.spec.ts b/browser_tests/tests/selectionToolboxMoreActions.spec.ts index ee700e2081..6c2d7f0991 100644 --- a/browser_tests/tests/selectionToolboxMoreActions.spec.ts +++ b/browser_tests/tests/selectionToolboxMoreActions.spec.ts @@ -2,21 +2,7 @@ import { comfyExpect as expect, comfyPageFixture as test } from '@e2e/fixtures/ComfyPage' -import type { ComfyPage } from '@e2e/fixtures/ComfyPage' - -async function openMoreOptions(comfyPage: ComfyPage) { - await expect(comfyPage.selectionToolbox).toBeVisible() - - const moreOptionsBtn = comfyPage.page.getByTestId('more-options-button') - await expect(moreOptionsBtn).toBeVisible() - await moreOptionsBtn.click() - await comfyPage.nextFrame() - - // Wait for the context menu to appear by checking for 'Copy', which is - // always present regardless of single or multi-node selection. - const menu = comfyPage.page.locator('.p-contextmenu') - await expect(menu.getByText('Copy', { exact: true })).toBeVisible() -} +import { openMoreOptions } from '@e2e/fixtures/utils/selectionToolbox' test.describe('Selection Toolbox - More Options', { tag: '@ui' }, () => { test.describe('Single node actions', () => { @@ -34,14 +20,14 @@ test.describe('Selection Toolbox - More Options', { tag: '@ui' }, () => { await expect(nodeRef).not.toBePinned() - await openMoreOptions(comfyPage) - await comfyPage.page.getByText('Pin', { exact: true }).click() + let menu = await openMoreOptions(comfyPage) + await menu.getByText('Pin', { exact: true }).click() await comfyPage.nextFrame() await expect(nodeRef).toBePinned() - await openMoreOptions(comfyPage) - await comfyPage.page.getByText('Unpin', { exact: true }).click() + menu = await openMoreOptions(comfyPage) + await menu.getByText('Unpin', { exact: true }).click() await comfyPage.nextFrame() await expect(nodeRef).not.toBePinned() @@ -57,14 +43,14 @@ test.describe('Selection Toolbox - More Options', { tag: '@ui' }, () => { await expect(nodeRef).not.toBeCollapsed() - await openMoreOptions(comfyPage) - await comfyPage.page.getByText('Minimize Node', { exact: true }).click() + let menu = await openMoreOptions(comfyPage) + await menu.getByText('Minimize Node', { exact: true }).click() await comfyPage.nextFrame() await expect(nodeRef).toBeCollapsed() - await openMoreOptions(comfyPage) - await comfyPage.page.getByText('Expand Node', { exact: true }).click() + menu = await openMoreOptions(comfyPage) + await menu.getByText('Expand Node', { exact: true }).click() await comfyPage.nextFrame() await expect(nodeRef).not.toBeCollapsed() @@ -78,8 +64,8 @@ test.describe('Selection Toolbox - More Options', { tag: '@ui' }, () => { const initialCount = await comfyPage.nodeOps.getGraphNodesCount() - await openMoreOptions(comfyPage) - await comfyPage.page.getByText('Copy', { exact: true }).click() + const menu = await openMoreOptions(comfyPage) + await menu.getByText('Copy', { exact: true }).click() await comfyPage.nextFrame() // Paste the copied node @@ -99,8 +85,8 @@ test.describe('Selection Toolbox - More Options', { tag: '@ui' }, () => { const initialCount = await comfyPage.nodeOps.getGraphNodesCount() - await openMoreOptions(comfyPage) - await comfyPage.page.getByText('Duplicate', { exact: true }).click() + const menu = await openMoreOptions(comfyPage) + await menu.getByText('Duplicate', { exact: true }).click() await comfyPage.nextFrame() await expect diff --git a/browser_tests/tests/selectionToolboxRename.spec.ts b/browser_tests/tests/selectionToolboxRename.spec.ts new file mode 100644 index 0000000000..0bbd77fe9b --- /dev/null +++ b/browser_tests/tests/selectionToolboxRename.spec.ts @@ -0,0 +1,96 @@ +import { + comfyExpect as expect, + comfyPageFixture as test +} from '@e2e/fixtures/ComfyPage' +import { getGroupTitlePosition } from '@e2e/fixtures/utils/groupHelpers' +import { openMoreOptions } from '@e2e/fixtures/utils/selectionToolbox' + +test.describe('Selection toolbox rename', { tag: '@ui' }, () => { + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.settings.setSetting('Comfy.Canvas.SelectionToolbox', true) + await comfyPage.workflow.loadWorkflow('default') + await comfyPage.nextFrame() + }) + + test.describe('Single rename', () => { + test('Rename via More Options opens title editor for single node', async ({ + comfyPage + }) => { + const nodeRef = ( + await comfyPage.nodeOps.getNodeRefsByTitle('KSampler') + )[0] + await comfyPage.nodeOps.selectNodeWithPan(nodeRef) + + const menu = await openMoreOptions(comfyPage) + await menu.getByText('Rename', { exact: true }).click() + + await expect(comfyPage.page.getByTestId('node-title-input')).toHaveValue( + 'KSampler' + ) + }) + + test('Rename shows prompt dialog for group', async ({ comfyPage }) => { + await comfyPage.settings.setSetting( + 'LiteGraph.Group.SelectChildrenOnClick', + false + ) + await comfyPage.workflow.loadWorkflow('groups/nested-groups-1-inner-node') + await comfyPage.nextFrame() + + const outerGroupPos = await getGroupTitlePosition( + comfyPage, + 'Outer Group' + ) + await comfyPage.canvas.click({ position: outerGroupPos }) + + const menu = await openMoreOptions(comfyPage) + await menu.getByText('Rename', { exact: true }).click() + + await expect(comfyPage.nodeOps.promptDialogInput).toBeVisible() + await comfyPage.nodeOps.promptDialogInput.fill('Renamed Group') + await comfyPage.page.keyboard.press('Enter') + await expect(comfyPage.nodeOps.promptDialogInput).toBeHidden() + + await expect + .poll(() => + comfyPage.page.evaluate(() => { + return window.app!.graph.groups.some( + (g) => g.title === 'Renamed Group' + ) + }) + ) + .toBe(true) + }) + }) + + test.describe('Batch rename', () => { + test('Batch rename multiple selected nodes', async ({ comfyPage }) => { + const ksampler = ( + await comfyPage.nodeOps.getNodeRefsByTitle('KSampler') + )[0] + const emptyLatent = ( + await comfyPage.nodeOps.getNodeRefsByTitle('Empty Latent Image') + )[0] + + await comfyPage.nodeOps.selectNodes(['KSampler', 'Empty Latent Image']) + + const menu = await openMoreOptions(comfyPage) + await menu.getByText('Rename', { exact: true }).click() + + await expect(comfyPage.nodeOps.promptDialogInput).toBeVisible() + await comfyPage.nodeOps.promptDialogInput.fill('TestNode') + await comfyPage.page.keyboard.press('Enter') + await expect(comfyPage.nodeOps.promptDialogInput).toBeHidden() + + await expect + .poll(async () => { + const titles = await Promise.all([ + ksampler.getProperty('title'), + emptyLatent.getProperty('title') + ]) + return [...titles].sort() + }) + .toEqual(['TestNode 1', 'TestNode 2']) + }) + }) +}) From 2a3b692c0bca7b494215d189ee95829ba9445152 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Mon, 4 May 2026 13:51:29 -0700 Subject: [PATCH 002/148] =?UTF-8?q?Repair:=20re-add=20bug-dump-ingest=20sk?= =?UTF-8?q?ill=20(#11460)=20=E2=80=94=20GitHub=20squash-commit=20incident?= =?UTF-8?q?=20recovery,=20step=202=20of=202=20(#11630)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *PR Created by the Glary-Bot Agent* --- ## Step 2 of 2 — GitHub squash-commit incident recovery for #11460 This is the companion to #11629. After #11629 (the revert) merges, this PR re-applies the original PR #11460 contents on top of the post-revert `main`, restoring the intended state of the codebase. ### ⚠️ Sequencing — must merge after #11629 Until #11629 is merged, this PR's diff against `main` is empty (because `main` currently still contains the squashed bug-dump-ingest commit that #11629 will revert). After #11629 is merged into `main`, GitHub will recompute the diff and this PR will show a clean re-add of the same 5 `.claude/skills/bug-dump-ingest/` files. ### Verification The branch was constructed by: 1. Branching from `glary/revert-pr-11460` (the revert branch in #11629). 2. `git checkout FETCH_HEAD -- .claude/skills/bug-dump-ingest/` from `refs/pull/11460/head` to restore the exact files. 3. Committing them as a single squash-style commit. I verified that all 5 file blobs are byte-for-byte identical to the original squash commit `559922eaa5c129767c22275c206c6877931ac15c` by comparing git object SHAs: | File | Object SHA | |---|---| | `.claude/skills/bug-dump-ingest/SKILL.md` | `413737835fa1c996291019483effdd39e6da33e5` | | `.claude/skills/bug-dump-ingest/reference/examples.md` | `4fc54a4f14b1359de63f558acca0de48c1b65c57` | | `.claude/skills/bug-dump-ingest/reference/linear-api.md` | `57986740df2ee02c19b81059f0f1e00e54c2a042` | | `.claude/skills/bug-dump-ingest/reference/schema.md` | `84db1a5818c04ee53a94167092ec76dd814992d4` | | `.claude/skills/bug-dump-ingest/reference/verify-commands.md` | `a2c99a43a030ccc3769692d3c09be74132645bb4` | ### Notes - One commit on `refs/pull/11460/head` (an automated lint commit `76ca1598e`) modified `src/renderer/extensions/vueNodes/widgets/components/WidgetChart.test.ts` to remove an `eslint-disable` directive. That change was **not** in the original squash commit on `main` (verified via `git show --stat`), and the directive has separately been removed from `main` by an unrelated commit (#11550), so re-applying that change would now be a no-op. This repair PR therefore intentionally restores **only the 5 bug-dump-ingest files**, matching the original squash commit exactly. - Branch name is `glary/repair-pr-11460` (the `glary/` prefix is required by the tooling; otherwise equivalent to GitHub's suggested `repair-pr-11460`). Refs: #11460, #11629 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11630-Repair-re-add-bug-dump-ingest-skill-11460-GitHub-squash-commit-incident-recovery--34d6d73d365081acbd54c19316561fa9) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot From aee2e6e6ddd632176bc6f674f49bb96671a76397 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Mon, 4 May 2026 13:53:16 -0700 Subject: [PATCH 003/148] test: add e2e tests for nested SubgraphNode input target resolution (#11405) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *PR Created by the Glary-Bot Agent* --- ## Summary - Adds four Playwright tests targeting `resolveSubgraphInputTarget` lines 20-31 — the `inputNode.isSubgraphNode()` branch where the target widget is a PromotedWidgetView - These lines had 0% e2e coverage because no existing test loaded a multi-level nested subgraph with VueNodes enabled - Tests use the existing `subgraph-nested-promotion.json` workflow (node 5 → Sub 0 → node 6 → Sub 1), which has outer SubgraphNode inputs connecting to an inner SubgraphNode ## Test cases | Test | Coverage target | Mechanism | |---|---|---| | Nested SubgraphNode promoted widgets render without resolution failures | Lines 20-31 (via VueNodes rendering) | Console warning collection + widget count assertion | | Subgraph input resolves through inner SubgraphNode with PromotedWidgetView | Lines 20-31 (graph structure verification) | `page.evaluate` walks link chain, asserts `isSubgraphNode() === true` and `isPromotedWidgetView === true` | | Promoted widgets from inner SubgraphNode carry correct source identity | Lines 24-31 (source identity) | Asserts widgets with `sourceNodeId === '6'` have correct `sourceWidgetName` | | Serialize and reload preserves nested promoted widget resolution | Lines 20-31 (persistence) | `serializeAndReload()` + polled widget count comparison | ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11405-test-add-e2e-tests-for-nested-SubgraphNode-input-target-resolution-3476d73d365081ab932edc8a01c55c40) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot Co-authored-by: Alexander Brown Co-authored-by: Connor Byrne --- .../tests/subgraph/subgraphNested.spec.ts | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/browser_tests/tests/subgraph/subgraphNested.spec.ts b/browser_tests/tests/subgraph/subgraphNested.spec.ts index 41323f3577..550534f8f1 100644 --- a/browser_tests/tests/subgraph/subgraphNested.spec.ts +++ b/browser_tests/tests/subgraph/subgraphNested.spec.ts @@ -3,6 +3,7 @@ import { expect } from '@playwright/test' import { comfyPageFixture as test, comfyExpect } from '@e2e/fixtures/ComfyPage' import { SubgraphHelper } from '@e2e/fixtures/helpers/SubgraphHelper' import { TestIds } from '@e2e/fixtures/selectors' +import { getPromotedWidgets } from '@e2e/fixtures/utils/promotedWidgets' test.describe('Nested Subgraphs', { tag: ['@subgraph'] }, () => { test.describe('Nested subgraph configure order', () => { @@ -190,4 +191,106 @@ test.describe('Nested Subgraphs', { tag: ['@subgraph'] }, () => { }) } ) + + test.describe( + 'Nested subgraph input target resolution', + { tag: ['@widget', '@vue-nodes'] }, + () => { + const WORKFLOW = 'subgraphs/subgraph-nested-promotion' + const OUTER_NODE_ID = '5' + const INNER_SUBGRAPH_NODE_ID = '6' + + test('Nested SubgraphNode promoted widgets render without resolution failures', async ({ + comfyPage + }) => { + const { warnings, dispose } = SubgraphHelper.collectConsoleWarnings( + comfyPage.page, + ['No link found', 'Failed to resolve legacy -1'] + ) + + try { + await comfyPage.workflow.loadWorkflow(WORKFLOW) + await comfyPage.vueNodes.waitForNodes() + + const outerNode = comfyPage.vueNodes.getNodeLocator(OUTER_NODE_ID) + await comfyExpect(outerNode).toBeVisible() + + const widgets = outerNode.getByTestId(TestIds.widgets.widget) + await comfyExpect( + widgets, + 'asset has 4 promoted widgets on outer subgraph node' + ).toHaveCount(4) + + expect(warnings).toEqual([]) + } finally { + dispose() + } + }) + + test('Promoted widgets from inner SubgraphNode are visible with correct values', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow(WORKFLOW) + await comfyPage.vueNodes.waitForNodes() + + const outerNode = comfyPage.vueNodes.getNodeLocator(OUTER_NODE_ID) + await comfyExpect(outerNode).toBeVisible() + + const widgets = outerNode.getByTestId(TestIds.widgets.widget) + await comfyExpect(widgets).toHaveCount(4) + + const valueWidget = outerNode + .getByRole('textbox', { name: 'value' }) + .first() + await comfyExpect(valueWidget).toBeVisible() + await comfyExpect(valueWidget).toHaveValue(/Inner 1/) + }) + + test('Promoted widgets from inner SubgraphNode carry correct source identity', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow(WORKFLOW) + await comfyPage.vueNodes.waitForNodes() + + await expect + .poll(async () => { + const widgets = await getPromotedWidgets(comfyPage, OUTER_NODE_ID) + return widgets + .filter( + ([sourceNodeId]) => sourceNodeId === INNER_SUBGRAPH_NODE_ID + ) + .map(([, sourceWidgetName]) => sourceWidgetName) + }) + .toContain('value') + }) + + test('Serialize and reload preserves nested promoted widget visibility', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow(WORKFLOW) + await comfyPage.vueNodes.waitForNodes() + + const outerNode = comfyPage.vueNodes.getNodeLocator(OUTER_NODE_ID) + const widgets = outerNode.getByTestId(TestIds.widgets.widget) + await comfyExpect( + widgets, + 'asset has 4 promoted widgets on outer subgraph node' + ).toHaveCount(4) + const initialCount = await widgets.count() + + await comfyPage.subgraph.serializeAndReload() + await comfyPage.vueNodes.waitForNodes() + + const outerNodeAfter = comfyPage.vueNodes.getNodeLocator(OUTER_NODE_ID) + const widgetsAfter = outerNodeAfter.getByTestId(TestIds.widgets.widget) + await comfyExpect(widgetsAfter).toHaveCount(initialCount) + + const valueWidget = outerNodeAfter + .getByRole('textbox', { name: 'value' }) + .first() + await comfyExpect(valueWidget).toBeVisible() + await comfyExpect(valueWidget).toHaveValue(/Inner 1/) + }) + } + ) }) From b83602fd23f3dd7f60c9df198c71ec6ce3c8b1bb Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Mon, 4 May 2026 14:08:06 -0700 Subject: [PATCH 004/148] feat: hide Google SSO button in embedded webviews (#10699) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hide the Google SSO login/signup button when the app runs inside an embedded webview (Android WebView, iOS WKWebView, social app in-app browsers), where Google blocks OAuth with a `403 disallowed_useragent` error. Fixes #7017 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10699-feat-hide-Google-SSO-button-in-embedded-webviews-3326d73d365081048e35d9d678fe1a2f) by [Unito](https://www.unito.io) --------- Co-authored-by: Benjamin Lu --- src/base/webviewDetection.test.ts | 119 ++++++++++++++++++ src/base/webviewDetection.ts | 72 +++++++++++ .../dialog/content/SignInContent.vue | 3 + .../cloud/onboarding/CloudLoginView.vue | 9 +- .../cloud/onboarding/CloudSignupView.vue | 4 +- 5 files changed, 205 insertions(+), 2 deletions(-) create mode 100644 src/base/webviewDetection.test.ts create mode 100644 src/base/webviewDetection.ts diff --git a/src/base/webviewDetection.test.ts b/src/base/webviewDetection.test.ts new file mode 100644 index 0000000000..c7faab6854 --- /dev/null +++ b/src/base/webviewDetection.test.ts @@ -0,0 +1,119 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { isEmbeddedWebView } from '@/base/webviewDetection' + +describe('isEmbeddedWebView', () => { + afterEach(() => { + vi.unstubAllGlobals() + }) + + describe('Android WebView', () => { + it('detects Android WebView with wv token', () => { + const ua = + 'Mozilla/5.0 (Linux; Android 13; SM-G991B; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/120.0.0.0 Mobile Safari/537.36' + expect(isEmbeddedWebView(ua)).toBe(true) + }) + + it('does not flag regular Chrome on Android', () => { + const ua = + 'Mozilla/5.0 (Linux; Android 13; SM-G991B) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Mobile Safari/537.36' + expect(isEmbeddedWebView(ua)).toBe(false) + }) + }) + + describe('iOS WKWebView', () => { + it('detects iOS WKWebView (AppleWebKit without Safari/)', () => { + const ua = + 'Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148' + expect(isEmbeddedWebView(ua)).toBe(true) + }) + + it('does not flag regular Safari on iOS', () => { + const ua = + 'Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Mobile/15E148 Safari/604.1' + expect(isEmbeddedWebView(ua)).toBe(false) + }) + + it('does not flag Chrome on iOS (CriOS)', () => { + const ua = + 'Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) CriOS/120.0.0.0 Mobile/15E148' + expect(isEmbeddedWebView(ua)).toBe(false) + }) + + it('does not flag Firefox on iOS (FxiOS)', () => { + const ua = + 'Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) FxiOS/120.0 Mobile/15E148' + expect(isEmbeddedWebView(ua)).toBe(false) + }) + }) + + describe('social app in-app browsers', () => { + it('detects Facebook (FBAN)', () => { + const ua = + 'Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148 [FBAN/FBIOS;FBAV/400.0]' + expect(isEmbeddedWebView(ua)).toBe(true) + }) + + it('detects Instagram', () => { + const ua = + 'Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148 Instagram 300.0' + expect(isEmbeddedWebView(ua)).toBe(true) + }) + + it('detects TikTok', () => { + const ua = + 'Mozilla/5.0 (Linux; Android 13) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Mobile Safari/537.36 TikTok/30.0' + expect(isEmbeddedWebView(ua)).toBe(true) + }) + + it('detects Line', () => { + const ua = + 'Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148 Line/13.0' + expect(isEmbeddedWebView(ua)).toBe(true) + }) + + it('detects Snapchat', () => { + const ua = + 'Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148 Snapchat/12.0' + expect(isEmbeddedWebView(ua)).toBe(true) + }) + }) + + describe('regular desktop browsers', () => { + it('does not flag Chrome desktop', () => { + const ua = + 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36' + expect(isEmbeddedWebView(ua)).toBe(false) + }) + + it('does not flag Firefox desktop', () => { + const ua = + 'Mozilla/5.0 (X11; Linux x86_64; rv:120.0) Gecko/20100101 Firefox/120.0' + expect(isEmbeddedWebView(ua)).toBe(false) + }) + + it('does not flag Safari desktop', () => { + const ua = + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 14_0) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Safari/605.1.15' + expect(isEmbeddedWebView(ua)).toBe(false) + }) + }) + + describe('edge cases', () => { + it('handles empty string', () => { + expect(isEmbeddedWebView('')).toBe(false) + }) + }) + + describe('JS bridge detection', () => { + it('detects webkit.messageHandlers bridge', () => { + vi.stubGlobal('webkit', { messageHandlers: {} }) + expect(isEmbeddedWebView('')).toBe(true) + }) + + it('detects ReactNativeWebView bridge', () => { + vi.stubGlobal('ReactNativeWebView', { postMessage: vi.fn() }) + expect(isEmbeddedWebView('')).toBe(true) + }) + }) +}) diff --git a/src/base/webviewDetection.ts b/src/base/webviewDetection.ts new file mode 100644 index 0000000000..4d56118409 --- /dev/null +++ b/src/base/webviewDetection.ts @@ -0,0 +1,72 @@ +/** + * Detects whether the app is running inside an embedded webview. + * + * Google blocks OAuth via `signInWithPopup` in embedded webviews, + * returning a 403 `disallowed_useragent` error (policy since 2021). + * This utility is used to hide the Google SSO button in those contexts. + * + * Detection covers: + * • Android WebView (`wv` token in UA) + * • iOS WKWebView (has `AppleWebKit` but lacks `Safari/`) + * • Social app in-app browsers (Facebook, Instagram, TikTok, etc.) + * • JS bridge objects (`window.webkit.messageHandlers`, `ReactNativeWebView`) + */ + +const SOCIAL_APP_PATTERNS = + /FBAN|FBAV|Instagram|Line\/|Snapchat|TikTok|musical_ly/i + +function isAndroidWebView(ua: string): boolean { + return /\bwv\b/.test(ua) && /Android/.test(ua) +} + +function isIOSWebView(ua: string): boolean { + if (!/AppleWebKit/i.test(ua)) return false + if (/Safari\//i.test(ua)) return false + if (/CriOS|FxiOS|OPiOS|EdgiOS/i.test(ua)) return false + return true +} + +function isSocialAppBrowser(ua: string): boolean { + return SOCIAL_APP_PATTERNS.test(ua) +} + +function hasWebViewBridge(): boolean { + try { + const win = globalThis as Record + if ( + typeof win.webkit === 'object' && + win.webkit !== null && + typeof (win.webkit as Record).messageHandlers === + 'object' + ) { + return true + } + if (win.ReactNativeWebView != null) return true + } catch { + // Access to bridge objects may throw in sandboxed contexts + } + return false +} + +export function isEmbeddedWebView(ua: string = navigator.userAgent): boolean { + if (isSocialAppBrowser(ua)) return true + if (isAndroidWebView(ua)) return true + if (isIOSWebView(ua)) return true + if (hasWebViewBridge()) return true + return false +} + +/** + * Reason why Google SSO is blocked in the current environment, or `null` if it + * is available. Modeled as a discriminated string so call sites read as + * "if blocked, here's why" rather than an opaque boolean. Extend this union + * (e.g. `'unauthorized-host'`) as new blocking conditions are detected. + */ +type GoogleSsoBlockedReason = 'embedded-webview' | null + +export function getGoogleSsoBlockedReason( + ua: string = navigator.userAgent +): GoogleSsoBlockedReason { + if (isEmbeddedWebView(ua)) return 'embedded-webview' + return null +} diff --git a/src/components/dialog/content/SignInContent.vue b/src/components/dialog/content/SignInContent.vue index 2592f34077..a6ac047e92 100644 --- a/src/components/dialog/content/SignInContent.vue +++ b/src/components/dialog/content/SignInContent.vue @@ -49,6 +49,7 @@