From bbdd383d6180d93cf45591164ea51ba7d404225a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 16:41:50 +0000 Subject: [PATCH] Fix double-prompt caused by www-redirect chain (v1.22) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The blocking UI was appearing twice consistently: once when navigating to a target site, and again immediately after pressing the allow button. Root cause was commit 50f838e's pendingPromptTabs guard clearing too eagerly on any changeInfo.url event, which fired for the browser's automatic site.com → www.site.com redirect before the tab had committed to prompt.html. The second event fell through to checkAccess and issued a duplicate prompt. Fix: introduce tabsCurrentlyAtPrompt to track when a tab has actually committed to prompt.html. Only clear pendingPromptTabs when the tab is genuinely navigating away from the prompt (i.e. tabsCurrentlyAtPrompt is set). Redirect-chain events arriving before the prompt commit are now suppressed correctly. Also clear processingTabs on prompt commit so back-button navigation is processed without waiting for the 1-second debounce to expire. Test infrastructure: add chrome.webNavigation mock to setup.js (was missing, causing all 62 tests to crash), add fireCommitted helper to helpers.js, and add two new flow tests covering the www-redirect chain and back-button without session scenarios. Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_018tjmes3dCNNYxB5n7LzYzL --- background.js | 28 ++++++++++++++++++---------- manifest-firefox.json | 2 +- manifest.json | 2 +- tests/flows.test.js | 42 +++++++++++++++++++++++++++++++++++------- tests/helpers.js | 7 +++++++ tests/setup.js | 5 ++++- 6 files changed, 66 insertions(+), 20 deletions(-) diff --git a/background.js b/background.js index 9243ed3..ee14af5 100644 --- a/background.js +++ b/background.js @@ -6,6 +6,7 @@ const DEFAULT_TARGETS = ['instagram.com', 'reddit.com', 'youtube.com']; const processingTabs = new Set(); // Tracks tabs currently being processed (short-lived lock) const pendingPromptTabs = new Set(); // Tracks tabs redirected to prompt.html, waiting for session start +const tabsCurrentlyAtPrompt = new Set(); // Tracks tabs that have committed to prompt.html // --- Time Range Helpers --- @@ -119,17 +120,23 @@ chrome.tabs.onUpdated.addListener(async (tabId, changeInfo, tab) => { // when status events fire after a redirect has already been issued. const currentUrl = changeInfo.url || tab.url; - // If the tab is heading to or already at the prompt, leave pendingPromptTabs intact and stop. - if (currentUrl.startsWith(chrome.runtime.getURL('prompt.html'))) return; + // If the tab has committed to the prompt page, mark it and clear the debounce lock + // (the redirect is complete; the lock is no longer needed). + if (currentUrl.startsWith(chrome.runtime.getURL('prompt.html'))) { + tabsCurrentlyAtPrompt.add(tabId); + processingTabs.delete(tabId); + return; + } - if (pendingPromptTabs.has(tabId)) { - if (changeInfo.url) { - // A fresh URL navigation away from the prompt — clear pending state and process normally. - pendingPromptTabs.delete(tabId); - } else { - // Stale status (loading/complete) event for a tab still waiting at the prompt — skip. - return; - } + // If the tab was genuinely at prompt.html and is now navigating away (e.g. after starting a session): + if (tabsCurrentlyAtPrompt.has(tabId)) { + tabsCurrentlyAtPrompt.delete(tabId); + pendingPromptTabs.delete(tabId); + // Fall through — process this navigation normally + } else if (pendingPromptTabs.has(tabId)) { + // Tab was redirected to prompt but hasn't committed there yet — this is a redirect-chain + // event (e.g. youtube.com → www.youtube.com) arriving before the tab reaches prompt.html. + return; } if (processingTabs.has(tabId)) return; @@ -455,6 +462,7 @@ chrome.alarms.onAlarm.addListener(async (alarm) => { // Handle Messages from Prompt or Content Script chrome.tabs.onRemoved.addListener((tabId) => { pendingPromptTabs.delete(tabId); + tabsCurrentlyAtPrompt.delete(tabId); processingTabs.delete(tabId); }); diff --git a/manifest-firefox.json b/manifest-firefox.json index beea726..e59c7d7 100644 --- a/manifest-firefox.json +++ b/manifest-firefox.json @@ -1,7 +1,7 @@ { "manifest_version": 3, "name": "Website Time Blocking", - "version": "1.21", + "version": "1.22", "description": "Control your website usage with blocking types.", "permissions": [ "storage", diff --git a/manifest.json b/manifest.json index 1a238e6..b3946cf 100644 --- a/manifest.json +++ b/manifest.json @@ -1,7 +1,7 @@ { "manifest_version": 3, "name": "Website Time Blocking", - "version": "1.21", + "version": "1.22", "description": "Control your website usage with blocking types.", "permissions": [ "storage", diff --git a/tests/flows.test.js b/tests/flows.test.js index ce8a645..88a5c46 100644 --- a/tests/flows.test.js +++ b/tests/flows.test.js @@ -1,6 +1,6 @@ // Cross-site and shared flows: time ranges, pendingPromptTabs, alarms, startSession message. const { - loadBackground, fireUpdated, fireRemoved, fireAlarm, setStorage, + loadBackground, fireUpdated, fireRemoved, fireCommitted, fireAlarm, setStorage, expectPromptRedirect, expectNoRedirect, flushPromises, NOW, } = require('./helpers'); @@ -125,12 +125,15 @@ test('pendingPromptTabs: stale status:complete after redirect does not double-pr }); test('pendingPromptTabs: fresh URL navigation away from prompt clears pending state', async () => { - // Redirect to prompt + // Step 1: navigate to YT — no session → redirect to prompt await nav(TAB, YT_HOME); expect(__mockFns__['tabs.update'].mock.calls.length).toBe(1); - // User starts a session — simulate what prompt.js does: sends startSession message, - // then calls window.location.replace(intendedUrl) which fires a changeInfo.url event + // Step 2: tab commits to prompt.html — this is required before any away-navigation is processed + const promptUrl = `chrome-extension://fakeid/prompt.html?url=${encodeURIComponent(YT_HOME)}`; + await fireUpdated(TAB, { url: promptUrl }, { url: promptUrl, status: 'loading' }); + + // Step 3: user starts a session via the prompt UI setStorage({ activeSessions: { 'youtube.com': { @@ -142,13 +145,38 @@ test('pendingPromptTabs: fresh URL navigation away from prompt clears pending st }, }); - // The replace fires a new URL navigation event — should clear pendingPromptTabs - // and then isTargetSite check should find an active session → no redirect + // Step 4: prompt.js calls window.location.replace(intendedUrl) → tab navigates back to YT + // Should clear pending state, find an active session, and allow access (no new redirect) await fireUpdated(TAB, { url: YT_HOME }, { url: YT_HOME }); - // Active session exists → should be allowed (no new redirect) expect(__mockFns__['tabs.update'].mock.calls.length).toBe(1); }); +test('pendingPromptTabs: www-redirect chain does not double-prompt', async () => { + // Step 1: navigate to bare domain (no www) — triggers redirect to prompt + await nav(TAB, 'https://youtube.com/'); + expect(__mockFns__['tabs.update'].mock.calls.length).toBe(1); + + // Step 2: browser fires www-redirect BEFORE tab commits to prompt.html + // This must be suppressed — tab hasn't reached the prompt page yet + await nav(TAB, 'https://www.youtube.com/'); + expect(__mockFns__['tabs.update'].mock.calls.length).toBe(1); // still only 1 +}); + +test('pendingPromptTabs: back button without session re-shows prompt', async () => { + // Step 1: navigate to YT — no session → redirect to prompt + await nav(TAB, YT_HOME); + expect(__mockFns__['tabs.update'].mock.calls.length).toBe(1); + + // Step 2: tab commits to prompt.html + const promptUrl = `chrome-extension://fakeid/prompt.html?url=${encodeURIComponent(YT_HOME)}`; + await fireUpdated(TAB, { url: promptUrl }, { url: promptUrl, status: 'loading' }); + + // Step 3: user presses back button without starting a session — tab returns to YT + await nav(TAB, YT_HOME); + // Should show prompt again — no valid session was started + expect(__mockFns__['tabs.update'].mock.calls.length).toBe(2); +}); + test('pendingPromptTabs: tab removed cleans up state', async () => { await nav(TAB, YT_HOME); expect(__mockFns__['tabs.update'].mock.calls.length).toBe(1); diff --git a/tests/helpers.js b/tests/helpers.js index b9f71ce..2e06136 100644 --- a/tests/helpers.js +++ b/tests/helpers.js @@ -34,6 +34,12 @@ function fireMessage(message, sender = {}) { }); } +// Fire a webNavigation.onCommitted event. +async function fireCommitted({ tabId, url, frameId = 0 }) { + const handlers = global.__listeners__.onCommitted; + await Promise.all(handlers.map(fn => fn({ tabId, url, frameId }))); +} + // Fire an alarm event. async function fireAlarm(alarm) { const handlers = global.__listeners__.onAlarm; @@ -85,6 +91,7 @@ module.exports = { loadBackground, fireUpdated, fireRemoved, + fireCommitted, fireMessage, fireAlarm, setStorage, diff --git a/tests/setup.js b/tests/setup.js index 965854c..ce78d0a 100644 --- a/tests/setup.js +++ b/tests/setup.js @@ -2,7 +2,7 @@ // Test files access storage via global.__store__ and listeners via global.__listeners__. const store = {}; -const listeners = { onUpdated: [], onAlarm: [], onMessage: [], onRemoved: [] }; +const listeners = { onUpdated: [], onAlarm: [], onMessage: [], onRemoved: [], onCommitted: [] }; const mockFns = {}; function makeMockFn(key) { @@ -74,4 +74,7 @@ global.chrome = { onAlarm: { addListener: (fn) => listeners.onAlarm.push(fn) }, create: (...args) => mockFns['alarms.create'](...args), }, + webNavigation: { + onCommitted: { addListener: (fn) => listeners.onCommitted.push(fn) }, + }, };