diff --git a/extension/background.js b/extension/background.js index 1b197bb..40d8ee6 100644 --- a/extension/background.js +++ b/extension/background.js @@ -11,7 +11,6 @@ import { createChatActivityStore } from './chat-activity.js'; import { HOST_ACCESS_REQUIRED_STATUS, normalizeTabId, inspectTabHostAccess, isHostAccessError, addTabHostAccessRequest, removeTabHostAccessRequest } from './host-access.js'; import { MEDIA_FRAME_ACCESS_REQUIRED, - MEDIA_FRAME_AMBIGUOUS, listMediaFrameScriptTargets, resolveMediaContentTarget } from './media-frame-target.js'; @@ -2404,7 +2403,8 @@ async function injectContentScript(tabId, { originPattern: error.originPattern }, requestAdded, error); } - if (error?.code === MEDIA_FRAME_AMBIGUOUS) throw error; + // MEDIA_FRAME_AMBIGUOUS is no longer fatal: the resolver falls back to + // the top frame and the monitor promotes the player that starts playing. addLog(`Media frame probe fell back to the top frame: ${error.message}`, 'warn'); } @@ -2933,20 +2933,25 @@ async function reactivateCurrentTarget(tabId, { expectedGeneration = targetActiv * like Drive and YouTube produce while simply playing. */ async function selectedMediaTargetMoved(tabId) { + let resolved; try { - const resolved = await resolveMediaContentTarget(chrome, tabId, { attempts: 1 }); - if (normalizeTabId(currentTabId) !== normalizeTabId(tabId)) return false; - const frameMoved = normalizeFrameId(resolved.frameId) !== normalizeFrameId(currentTargetFrameId); - const documentMoved = typeof resolved.documentId === 'string' - && typeof currentTargetDocumentId === 'string' - && resolved.documentId !== currentTargetDocumentId; - const gainedVideo = resolved.hasVideo === true && currentTargetHasVideo !== true; - return frameMoved || documentMoved || gainedVideo; + resolved = await resolveMediaContentTarget(chrome, tabId, { attempts: 1 }); } catch { - // Access-required and ambiguity errors must reach the full activation - // path so the popup can surface them. + // An access-required error must reach the full activation path so the + // popup can surface it. return true; } + if (normalizeTabId(currentTabId) !== normalizeTabId(tabId)) return false; + // An inconclusive probe is not a reason to move. A page whose players are + // still loading, or that offers several equally-ranked mirrors, resolves + // differently from one moment to the next; acting on that flips the target + // back and forth and leaves activation running forever. + if (resolved.hasVideo !== true) return false; + if (currentTargetHasVideo !== true) return true; + return normalizeFrameId(resolved.frameId) !== normalizeFrameId(currentTargetFrameId) + || (typeof resolved.documentId === 'string' + && typeof currentTargetDocumentId === 'string' + && resolved.documentId !== currentTargetDocumentId); } function refreshCurrentMediaTarget(tabId, { queueIfRunning = false, onlyIfTargetMoved = false } = {}) { @@ -3461,9 +3466,11 @@ async function handleAsyncMessage(message, sender, sendResponse) { ? 'activating' : pendingTarget?.tabId === publicTargetTabId ? 'access_required' - : normalizeTabId(userSelectionErrorTabId) === publicTargetTabId - ? 'error' - : 'activating'; + // Nothing is in flight and the target is not live, so + // this is a settled failure. Reporting it as + // "activating" is what left the popup spinning forever + // with no way to tell that it had already given up. + : 'error'; sendResponse({ status, peerId, diff --git a/extension/media-frame-target.js b/extension/media-frame-target.js index 0bac025..052998b 100644 --- a/extension/media-frame-target.js +++ b/extension/media-frame-target.js @@ -1,5 +1,4 @@ export const MEDIA_FRAME_ACCESS_REQUIRED = 'media_frame_access_required'; -export const MEDIA_FRAME_AMBIGUOUS = 'media_frame_ambiguous'; export const MEDIA_FRAME_PROBE_TIMEOUT = 'media_frame_probe_timeout'; const MIN_PLAYER_FRAME_AREA = 320 * 180; @@ -246,7 +245,12 @@ export function installParentFrameVisibilityProbe(token) { }; }; window.addEventListener('message', handler); - timeout = setTimeout(cleanup, 1000); + // The listener has to outlive the whole probe sequence: install, four + // dispatch passes and the final inspection, each a separate executeScript + // round trip. On a heavy page those add up well past a second, and a + // listener that expired first left every frame's visibility unknown — which + // is exactly the state that makes two players look equally ranked. + timeout = setTimeout(cleanup, 15000); window.__koalaFrameVisibilityCleanup = cleanup; } @@ -415,12 +419,6 @@ function accessRequiredError(access) { return error; } -function ambiguousFrameError() { - const error = new Error('The active embedded video frame could not be identified safely'); - error.code = MEDIA_FRAME_AMBIGUOUS; - return error; -} - function contentTarget(tabId, selected) { const frameId = normalizeFrameId(selected?.frameId); const documentId = typeof selected?.documentId === 'string' ? selected.documentId : null; @@ -619,6 +617,11 @@ export async function resolveMediaContentTarget(chromeApi, tabId, { // frame so the injected monitor can promote the real player once it loads, // instead of failing the activation or prompting for nothing. if (unresolvedGrantedHost) return contentTarget(tabId, null); - if (ambiguous) throw ambiguousFrameError(); + // Several equally-ranked players — anime mirrors, alternative dubs — are a + // normal page layout, not an error. Refusing to activate made those pages + // unusable, and flipping between candidates restarted the target forever. + // Hold the top frame and let the monitor promote the one that starts + // playing, which is the signal that breaks the tie. + if (ambiguous) return { ...contentTarget(tabId, null), ambiguous: true }; return contentTarget(tabId, null); } diff --git a/extension/media-frame-target.test.mjs b/extension/media-frame-target.test.mjs index d061c92..bcf0254 100644 --- a/extension/media-frame-target.test.mjs +++ b/extension/media-frame-target.test.mjs @@ -1,7 +1,6 @@ import { describe, expect, it, vi } from 'vitest'; import { MEDIA_FRAME_ACCESS_REQUIRED, - MEDIA_FRAME_AMBIGUOUS, inspectMediaFrame, resolveMediaContentTarget, selectMediaFrame @@ -432,17 +431,25 @@ describe('cross-origin media-frame targeting', () => { )).resolves.toMatchObject({ frameId: 0, scriptTarget: { tabId: 44 } }); }); - it('reports ambiguity rather than controlling an arbitrary equal player', async () => { + it('holds the top frame instead of guessing between equal players', async () => { const results = [ frame(3, { parentFrameVisible: null }), frame(4, { parentFrameVisible: null }) ]; const executeScript = vi.fn().mockResolvedValue(results); + // Equally-ranked mirrors are an ordinary anime-site layout. Refusing to + // activate made those pages unusable; the tab stays selected on its top + // frame until one of the players starts and breaks the tie. await expect(resolveMediaContentTarget( { scripting: { executeScript } }, 45, { attempts: 1, probeDelayMs: 0 } - )).rejects.toMatchObject({ code: MEDIA_FRAME_AMBIGUOUS }); + )).resolves.toMatchObject({ + frameId: 0, + hasVideo: false, + ambiguous: true, + scriptTarget: { tabId: 45 } + }); }); }); diff --git a/tests/e2e/extension.spec.mjs b/tests/e2e/extension.spec.mjs index 71a8b50..4406957 100644 --- a/tests/e2e/extension.spec.mjs +++ b/tests/e2e/extension.spec.mjs @@ -607,6 +607,73 @@ test('rejects a hidden cross-origin player after its iframe URL redirects', asyn expect(await redirectedFrame.locator('video').getAttribute('data-koala-attached')).toBeNull(); }); +test('selects the visible anime player nested behind a same-origin wrapper', async ({ context, extensionId, baseURL }) => { + const url = `${baseURL}/pages/yummy-style-player.html`; + const page = await context.newPage(); + await page.goto(url); + await page.waitForFunction(() => window.__fixtureReady === true); + + const { tabId, response } = await selectTargetTab(context, extensionId, url); + expect(response).toMatchObject({ status: 'ok', hasVideo: true }); + expect(response.frameId).not.toBe(0); + + // The playable element is the one inside the visible wrapper. The two + // zero-sized mirrors must be ignored, not treated as equal candidates. + const playerFrame = suffix => page.frames() + .find(frame => frame.url().endsWith(`/frames/${suffix}`)); + await expect + .poll(() => playerFrame('player-frame.html').locator('video').getAttribute('data-koala-attached')) + .toBe('true'); + expect(await playerFrame('player-frame-2.html').locator('video') + .getAttribute('data-koala-attached')).toBeNull(); + + // And the selection has to settle, not keep re-resolving. + await page.waitForTimeout(1500); + const status = await getExtensionState(context, extensionId, { type: 'GET_STATUS' }); + expect(status).toMatchObject({ + targetTabId: tabId, + targetReady: true, + targetActivationState: 'ready' + }); +}); + +test('selects an anime tab before playback and promotes the player once it appears', async ({ context, extensionId, baseURL }) => { + // The live case: at selection time the page has no video anywhere, because + // the host only builds the player when the viewer presses play. + const url = `${baseURL}/pages/yummy-deferred-player.html`; + const page = await context.newPage(); + await page.goto(url); + await page.waitForFunction(() => window.__fixtureReady === true); + + const { tabId, response } = await selectTargetTab(context, extensionId, url); + // Selecting must succeed and settle even with nothing to control yet. + expect(response).toMatchObject({ status: 'ok', frameId: 0, hasVideo: false }); + + await page.waitForTimeout(1200); + const idle = await getExtensionState(context, extensionId, { type: 'GET_STATUS' }); + expect(idle).toMatchObject({ + targetTabId: tabId, + targetReady: true, + targetActivationState: 'ready' + }); + + const deferred = page.frames().find(frame => frame.url().endsWith('/frames/deferred-player-frame.html')); + await deferred.locator('#poster').click(); + + // The monitor has to hand the target over to the frame that now owns the + // video, without the user touching the popup again. + await expect + .poll(() => deferred.locator('video').getAttribute('data-koala-attached'), { timeout: 15000 }) + .toBe('true'); + await expect + .poll(() => getExtensionState(context, extensionId, { type: 'GET_STATUS' }) + .then(state => ({ ready: state.targetReady, frame: state.targetFrameId })), { timeout: 15000 }) + .toMatchObject({ ready: true }); + const promoted = await getExtensionState(context, extensionId, { type: 'GET_STATUS' }); + expect(promoted.targetFrameId).not.toBe(0); + expect(promoted).toMatchObject({ targetTabId: tabId, targetActivationState: 'ready' }); +}); + test('keeps the tab selected when its activation fails', async ({ context, extensionId }) => { // A page the extension is not allowed to script stands in for any activation // failure the user can act on. Losing the selection here is what made the diff --git a/tests/e2e/fixtures/pages/frames/deferred-player-frame.html b/tests/e2e/fixtures/pages/frames/deferred-player-frame.html new file mode 100644 index 0000000..67beca9 --- /dev/null +++ b/tests/e2e/fixtures/pages/frames/deferred-player-frame.html @@ -0,0 +1,27 @@ + + +
Description block that pushes the player below the fold.
+Description block that pushes the player below the fold.
+