From 32739f322a1ca92fdd2bf622f56d7617f2e2d2d7 Mon Sep 17 00:00:00 2001 From: Openclaw Date: Wed, 23 Sep 2026 07:05:19 +0200 Subject: [PATCH] fix: cancel stale channel picker focus --- public/channel-color-picker.js | 21 +++++++++++++++- test-channel-color-picker-e2e.js | 41 ++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/public/channel-color-picker.js b/public/channel-color-picker.js index cd7ddfffc..997e778b0 100644 --- a/public/channel-color-picker.js +++ b/public/channel-color-picker.js @@ -28,6 +28,9 @@ // overwriting body.style.overflow directly. Without this, two cooperating // surfaces (this picker + SlideOver) corrupt overflow last-writer-wins. var scrollLockToken = null; + // #1943: handle for the deferred focus of the first swatch, so it can be + // cancelled. It used to be fire-and-forget. + var focusTimer = null; function createPopover() { if (popoverEl) return popoverEl; @@ -143,7 +146,20 @@ // Focus first swatch for keyboard accessibility var firstSwatch = el.querySelector('.cc-swatch'); - if (firstSwatch) setTimeout(function() { firstSwatch.focus(); }, 0); + // #1943: this used to be an uncancellable setTimeout(0). It could land + // AFTER the user had already moved focus with an arrow key, snapping the + // selection back to the first swatch, and Enter then assigned the wrong + // colour. Cancel any pending one, and do not steal focus that already sits + // inside the popover or that belongs to a popover since hidden. + if (focusTimer) { clearTimeout(focusTimer); focusTimer = null; } + if (firstSwatch) { + focusTimer = setTimeout(function () { + focusTimer = null; + if (el.style.display === 'none') return; + if (el.contains(document.activeElement)) return; + firstSwatch.focus(); + }, 0); + } // Listen for outside click / Escape setTimeout(function() { @@ -153,6 +169,9 @@ } function hidePopover() { + // #1943: a pending focus timer from this show() must not fire into the + // next one. + if (focusTimer) { clearTimeout(focusTimer); focusTimer = null; } if (popoverEl) popoverEl.style.display = 'none'; currentChannel = null; if (window.__scrollLock && scrollLockToken != null) { diff --git a/test-channel-color-picker-e2e.js b/test-channel-color-picker-e2e.js index c8cc335eb..bcf78b389 100644 --- a/test-channel-color-picker-e2e.js +++ b/test-channel-color-picker-e2e.js @@ -161,6 +161,47 @@ function assert(c, m) { if (!c) throw new Error(m || 'assertion failed'); } 'Enter should assign focused color (' + nextColor + '), got ' + stored); }); + await step('a late focus timer does not snap the selection back (#1943)', async () => { + // Deterministic reproduction of #1943. showPopover() defers focusing the + // first swatch with setTimeout(0). Reopening while a swatch still has focus + // means the test's usual "wait for a focused swatch" is satisfied by the + // OLD focus, so ArrowRight runs before the new timer lands. The timer then + // fired into the popover and pulled focus back to the first swatch, and + // Enter assigned that colour instead of the navigated-to one. + await page.evaluate(() => window.ChannelColorPicker.show('#lateA', 100, 100)); + await page.waitForFunction(() => { + const el = document.activeElement; + return el && el.classList && el.classList.contains('cc-swatch'); + }, { timeout: 2000 }); + await page.keyboard.press('Escape'); + // Reopen and move immediately, without waiting for the new focus timer. + await page.evaluate(() => window.ChannelColorPicker.show('#lateB', 100, 100)); + const before = await page.evaluate(() => + document.activeElement && document.activeElement.getAttribute + ? document.activeElement.getAttribute('data-color') : null); + await page.keyboard.press('ArrowRight'); + const moved = await page.evaluate(() => + document.activeElement.getAttribute('data-color')); + // Give any pending setTimeout(0) more than enough time to land. + await page.waitForTimeout(150); + const settled = await page.evaluate(() => + document.activeElement.getAttribute('data-color')); + assert(settled === moved, + 'a late focus timer must not move focus after the user did (was ' + before + + ', moved to ' + moved + ', settled on ' + settled + ')'); + await page.keyboard.press('Enter'); + await page.waitForFunction(() => { + const el = document.querySelector('.cc-picker-popover'); + return el && el.style.display === 'none'; + }, { timeout: 3000 }); + const stored = await page.evaluate(() => + window.ChannelColors && window.ChannelColors.get('#lateB')); + assert(stored === moved, + 'Enter must assign the swatch the user navigated to (expected ' + moved + + ', got ' + stored + ')'); + await page.evaluate(() => window.ChannelColors.remove('#lateB')); + }); + await step('outside click closes popover', async () => { // De-flake history: #1317 (62a81776) tried `mouse.click(700,500)` + a // `rect.width > 0` "listener installed" proxy. That proxy is FALSE — it