Skip to content

content.js lifecycle defects: soft-to-hard check runs before the player links, windowed-fullscreen styling survives removePlayers, REMOVE_PLAYERS reaches frame 0 only, miniplayer throws on a detached or body-filling iframe #229

Closed as not planned

Description

@Nawid3333

Severity: low

  1. Soft-to-hard conversion check fires before the player has linked (content.js: updateReplacedPlayers(i == 4 ? pobj : null) on the 1,000 ms timer; the convert check only visits iframeMap): a replaced player is in replacedPlayerQueue until the FRAME_LINK round trip moves it there. On a slow machine (CLAUDE.md records the Windows runner taking seconds for a player start) all eight timers run while pobj is still queued and do nothing, so the one chance to turn a collapsed soft replace into a hard one is lost; the link handler's own update passes convert = null. Fix: schedule updateReplacedPlayers(replacedData) about 1 s after the link, or make the timers also walk replacedPlayerQueue.

  2. removePlayers leaves a chain iframe in its windowed-fullscreen styling: it calls unmakeMiniPlayer and restoreReplaced, then undoFillScreenIframe(), but the only place the iframe's own style is put back is windowedFullscreenToggle. A player in a site's cross-origin embed iframe with windowed fullscreen on: the top frame's removePlayers() (same-site link click, or REMOVE_PLAYERS on a tab URL change) restores the hidden page elements but neither resets the iframe's position: fixed; 100%; z-index: 2147483647 nor windowedFullscreenState.active; an SPA link or pushState change leaves the page under a fixed full-screen iframe. Fix: in removePlayers, call windowedFullscreenToggle(iframeObj) for every active one before undoFillScreenIframe(), as frameLoadListener does.

  3. REMOVE_PLAYERS on a tab URL change reaches frame 0 only (background.mjs: chrome.tabs.sendMessage(tabid, {type: REMOVE_PLAYERS}, {frameId: 0}, ...)): a site whose video lives in its own iframe changes episode by pushState; the top frame removes nothing and the iframe's content script, which holds the player, is never told, so the old player stays with the old stream while the page's embed switches underneath. Fix: send without frameId (all frames), as HAS_PLAYER already is.

  4. Miniplayer on a detached or body-filling iframe throws inside the message handler and never answers (handleMiniplayer: element.parentNode.insertBefore(placeholder, element); getParentElementsWithSameBounds pushes BODY before breaking; placeholder.clientWidth / placeholder.clientHeight): (a) the page took the player iframe out before the request, parentNode is null, TypeError, no sendResponse, and the background's promise never settles (the case content-cleanup fixed for fullscreen); (b) a site that switched its wrapper to fill the body after the player opened: BODY becomes element, a second <body> is inserted before the real one and the real body is styled position: fixed at miniplayer size; (c) a zero-height placeholder gives NaN sizes. Fix: try/catch answering 'error' like handleFullscreen; stop before pushing BODY for the miniplayer; guard clientHeight === 0.

Confidence: high (1, 2, 4a), medium (3, 4b).


From a line-by-line code review of the tree at 41363fd (2026-10-02). Each claim above was checked against the source before filing; the reproduction steps and line references are as of that commit.


Generated by Claude Code

Activity

  1. added
    bugSomething isn't working
    code-reviewFound by the 2026-10-02 line-by-line review
    on Oct 2, 2026
  2. Nawid3333 commented on Oct 3, 2026

    @Nawid3333
    OwnerAuthor

    Closing: items 1, 2 and 4 were fixed in #306. Item 3 (sending REMOVE_PLAYERS to every frame when the address changes) is not done on purpose (decision D1 in #316): sites keep embeds across navigations, and closing the players in them would break those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcode-reviewFound by the 2026-10-02 line-by-line review

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions