Skip to content

Compare overlay: switching a version does not reset per-side sync offsets (offA/offB), silently mis-syncing the new pair #182

Description

@ravirajsinh45

Component: apps/web/components/review/compare/compare-overlay.tsx (video compare / wipe-and-scrubber overlay, introduced in PR #169)

Repro

  1. Open the compare overlay on a video asset with 3+ ready versions (e.g. v1, v2, v3).
  2. Compare v1 (left/A) vs v2 (right/B).
  3. Use the offset steppers in the scrubber to nudge A and/or B (offA/offB) to align the two clips (e.g. because v2 has a few extra frames of pre-roll).
  4. Without clicking the "Sync" / reset-offsets button, switch either side's version via its CompareVersionSelect dropdown — e.g. change the left pane from v1 to v3.
  5. Observe: the new pairing (v3 vs v2) plays back with the stale offset(s) from the v1/v2 comparison still applied, so the two clips are silently mis-synced. Nothing in the UI indicates the offsets are stale/carried over.

Root cause

offA/offB are read straight from URL search params and fed into timingA/timingB:

// compare-overlay.tsx:87-88
const offA = parseOffsetParam(searchParams.get('offA'))
const offB = parseOffsetParam(searchParams.get('offB'))

The two version-select dropdowns' onChange handlers only update which version is shown — they never touch the offset params:

// compare-overlay.tsx:313 (left/A version switch)
onChange={(v) => writeParams((p) => p.set('compare', v.id))}

// compare-overlay.tsx:341 (right/B version switch)
onChange={(v) => setCurrentVersion(v)}

CompareVersionSelect itself (compare-version-select.tsx:72-76) is a plain controlled dropdown that just invokes the onChange prop it's given — it has no concept of offsets, so there's no hidden reset happening there either.

The only code path that clears offA/offB is the explicit "Sync" button in the scrubber:

// compare-overlay.tsx:499-501
onResetOffsets={() =>
  writeParams((p) => { p.delete('offA'); p.delete('offB') })
}

wired to the manual reset button in compare-scrubber.tsx:229-236 (title="Reset offsets — re-sync both sides").

So any offset dialed in for one version pairing silently carries over to whatever new pairing the user selects next, with no reset and no warning.

Suggested fix

Clear offA/offB whenever the identity of either compared version changes, not just when the user explicitly clicks Sync. E.g. in the two onChange handlers at compare-overlay.tsx:313 and :341, also delete offA/offB from the params before setting the new version:

onChange={(v) => writeParams((p) => { p.set('compare', v.id); p.delete('offA'); p.delete('offB') })}

and for the right/B side, similarly clear the offset params (via writeParams, or by having setCurrentVersion's caller also clear them) before/along with setCurrentVersion(v). Alternatively, key offA/offB off the pair (left.id, right.id) so a URL replace / effect resets them automatically whenever either id changes.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingfrontendapps/web — Next.js frontend

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions