Skip to content

fix(match): surface replayUrl with new-tab navigation, URL validation, and accessibility#585

Open
petahade wants to merge 4 commits into
Arenax-gaming:mainfrom
petahade:feat/match-replay-url
Open

fix(match): surface replayUrl with new-tab navigation, URL validation, and accessibility#585
petahade wants to merge 4 commits into
Arenax-gaming:mainfrom
petahade:feat/match-replay-url

Conversation

@petahade

@petahade petahade commented Jun 24, 2026

Copy link
Copy Markdown

Closes #555


Problem

MatchDetailView already had a conditional block for match.replayUrl, but it was broken in three ways:

  1. Wrong navigation target — it used Next.js <Link href={match.replayUrl}>, which pushes the URL through the client-side router. Replay URLs are external (e.g., a video CDN or replay platform), so the tab would navigate away from ArenaX instead of opening a new tab.
  2. No security attributes — no rel="noopener noreferrer", leaving the replay page able to reference window.opener and potentially redirect the parent tab (reverse tabnabbing).
  3. No URL validation — the only guard was a truthy check on match.replayUrl. A whitespace-only string, a javascript: URI, or a relative path would all pass and produce a broken or unsafe link.

Changes

frontend/src/components/match/MatchDetailView.tsx

isValidHttpUrl helper (module-level, before the component)

function isValidHttpUrl(value: string): boolean {
  try {
    const { protocol } = new URL(value);
    return protocol === "http:" || protocol === "https:";
  } catch {
    return false;
  }
}
  • Uses the platform URL constructor — throws on malformed input, which the catch converts to false.
  • Explicitly asserts http: or https: protocol, rejecting javascript:, data:, blob:, and relative strings that URL might parse with a base.

Derived variables (inside MatchDetailView)

const replayUrl = match.replayUrl?.trim() ?? "";
const hasReplay = replayUrl.length > 0 && isValidHttpUrl(replayUrl);
  • trim() eliminates whitespace-only values before validation.
  • hasReplay is the single gate used in JSX, keeping the render condition readable.

Button / link replacement

{hasReplay && (
  <Button variant="outline" size="sm" asChild>
    <a
      href={replayUrl}
      target="_blank"
      rel="noopener noreferrer"
      aria-label="Watch match replay (opens in new tab)"
    >
      <Play className="h-4 w-4 mr-2" aria-hidden="true" />
      Watch Replay
    </a>
  </Button>
)}
  • <a> instead of <Link> — native anchor handles external URLs correctly.
  • target="_blank" opens in a new tab.
  • rel="noopener noreferrer" removes window.opener reference and suppresses the Referer header.
  • aria-label includes "(opens in new tab)" so screen reader users know what to expect before activating the link.
  • aria-hidden="true" on the decorative Play icon prevents the icon name from being announced alongside the label.
  • asChild on <Button> delegates all button styling to the <a> — no wrapper element, no nesting of interactive elements.
  • If hasReplay is false (null, undefined, empty, whitespace, malformed, non-HTTP), nothing is rendered and no space is reserved in the layout.

Acceptance criteria

Criterion Status
replayUrl present and valid → "Watch Replay" button rendered
Clicking opens the URL in a new tab ✅ (target="_blank")
Reverse tabnabbing prevented ✅ (rel="noopener noreferrer")
replayUrl is null / undefined → no button rendered
replayUrl is "" or " " → no button rendered ✅ (trim + length check)
replayUrl is malformed (e.g. "not a url") → no button rendered ✅ (URL constructor throws → false)
replayUrl uses non-HTTP protocol (e.g. "javascript:alert(1)") → no button rendered ✅ (protocol assertion)
Existing header layout and "Report Issue" button unaffected ✅ (same flex gap-2 container, no structural change)
Keyboard accessible ✅ (native <a> is focusable and activatable)
Screen reader friendly ✅ (aria-label with tab-open hint, decorative icon hidden)

Test plan

  • Pass a valid HTTPS URL as replayUrl — button appears in the header next to "Report Issue".
  • Click "Watch Replay" — replay URL opens in a new browser tab; ArenaX tab is unchanged.
  • Inspect the rendered <a> — confirm target="_blank" and rel="noopener noreferrer" are present.
  • Tab to the button with keyboard and press Enter — same new-tab behaviour.
  • Inspect with a screen reader — label reads "Watch match replay (opens in new tab)", Play icon not announced.
  • Pass replayUrl: null / undefined / "" / " " / "ftp://example.com" / "javascript:void(0)" / "notaurl" — confirm no button rendered and no layout shift.
  • Confirm "Report Issue" button still renders when canDispute is true, independent of replay state.
  • Confirm all other sections of MatchDetailView (round breakdown, stats cards, prize distribution, bracket link) are unaffected.

petahade added 4 commits June 24, 2026 16:36
- Extend `useLeaderboard` with an optional `season` param; appends
  `?season=<id>` to every API request and includes it in the React Query
  key so season changes trigger fresh fetches rather than reusing cached
  data for the wrong season.

- `/leaderboard` page: add a Season filter (alongside category/search/sort),
  initialize it from the `?season=` query param on page load, and update
  the URL via `router.push(..., { scroll: false })` on each change — no
  full reload, back/forward navigation preserved.

- `/leaderboards` page: wire existing `SeasonSelector` to the same URL-sync
  handler instead of isolated local state; season is now passed through to
  `useLeaderboard` so the API call reflects the selection.

- Both pages use `isFetching` (not just `isLoading`) to show skeleton rows
  and a `Loader2` spinner in the table heading while a season-specific
  request is in flight, preventing stale rankings from lingering on screen.

- Wrap both page components in a `<Suspense>` boundary required by Next.js
  14 for `useSearchParams` in client components.
…RL validation

The "Watch Replay" button previously used Next.js <Link>, which routes
internally and provides no `target` or `rel`, so clicking it navigated
the current tab via the client-side router rather than opening the
external replay service.

Changes:
- Replace <Link> with a native <a> element carrying `target="_blank"` and
  `rel="noopener noreferrer"` to open replays in a new tab and prevent
  reverse tabnabbing.
- Add `isValidHttpUrl` helper that runs the value through the URL
  constructor and asserts an http/https protocol, guarding against empty
  strings, whitespace-only values, and malformed or non-HTTP URLs.
- Derive `replayUrl` by trimming the raw field value and `hasReplay` by
  running validation, so the button is never rendered for invalid inputs.
- Add `aria-label="Watch match replay (opens in new tab)"` for keyboard
  and screen reader users, and mark the decorative Play icon as
  `aria-hidden="true"` to avoid duplicate announcements.
@petahade petahade requested a review from anonfedora as a code owner June 24, 2026 19:28
@vercel

vercel Bot commented Jun 24, 2026

Copy link
Copy Markdown

@petahade is attempting to deploy a commit to the paul joseph's projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Jun 24, 2026

Copy link
Copy Markdown

@petahade Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[frontend] - Match detail view replayUrl link is never rendered

1 participant