Add VK Video support with shadow-DOM aware video detection#81
Open
ZiraiMode wants to merge 1 commit into
Open
Conversation
VK Video (vkvideo.ru, vk.com, /video_ext.php embeds) mounts its <video> inside an open shadow root rooted at div.shadow-root-container, so detectPageVideos and the injected sync code never found it via document.querySelector*. - Add the vkvideo entry to SITE_CONFIGS in both video-detection.ts and the inline copy in background.ts (the latter is serialised into the page via executeScript and can't share imports — collapsing the duplication is left to a follow-up PR). - Introduce collectAllVideos / deepQuerySelector helpers that recurse through open shadow roots, and route findSiteVideo, findGenericVideo, detectPageVideos, the injected.ts data-synclify-id lookup, and the videoPlayer.tsx attach lookup through them. - Skip the isPlayable filter when the site-specific selector matches — VK uses MSE blob URLs that only appear after the first Play, so the <video> would otherwise be discarded for missing src/dimensions. - Bump the inject retry budget to 5 x 800 ms; the VK SPA player loads noticeably slower than YouTube/Netflix chunks. - Surface backend errors in the popup: createOrJoinRoom was silently swallowing rejected sendMessage promises, so a missing socket server produced no UI feedback. Add a .catch and an error pill under the Create Room button. - Add .env.example documenting WXT_SOCKET_ENDPOINT and unignore it in .gitignore (the .env* glob was hiding it).
andreademasi
requested changes
Jun 25, 2026
andreademasi
left a comment
andreademasi
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the PR! 🙏🏼
I found one issue I’d like fixed before merge: the new trustSelector path in background.ts skips the isPlayable() check for any known site once the site-specific selector matches. That looks broader than intended. For VK it makes sense because the video may not have src/dimensions before first play, but for existing sites like YouTube/Netflix/Max this can now select unloaded or placeholder videos that previously would have been ignored/retried.
Could you scope that relaxed behavior to VK only, instead of applying it to all site-specific selectors?
A couple of smaller follow-ups worth considering:
handleCreateRoom()still returns the response body even whenfetch()gets a non-OK status, so the popup.catchwon’t catch backend HTTP errors unless the request rejects. It should probably throw on!res.ok.- The injected fallback lookup is now shadow-aware, but the
MutationObserverstill only observesdocument. If VK creates the shadow host first and inserts the video later inside the shadow root, the retry may miss it. shouldShowCustomPlayer()still usesvideo.closest(), which won’t cross shadow boundaries. Now that generic detection can find shadow-DOM videos, unknown commercial players inside shadow roots may be misclassified.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/video-detection.ts (1)
49-55: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake the unplayable fallback opt-in.
Line 312 now returns the first matched
<video>for every known site, not just VK. That widens selectors likevideo(paramountplus,hotstar,mubi) into arbitrary preload/ad elements when nothing is playable yet. Please add an explicitallowUnplayableMatchflag and enable it only forvkvideo; the inline detector insrc/entrypoints/background.tsshould use the same contract.Suggested direction
export type SiteConfig = { hostPatterns: RegExp[] videoSelector: string playerContainer: string watchPageTest?: () => boolean excludeSelector?: string + allowUnplayableMatch?: boolean } vkvideo: { hostPatterns: [/(^|\.)vkvideo\.ru$/, /(^|\.)vk\.com$/], videoSelector: '[data-testid="video-container"] video', playerContainer: ".vk-vp-root", watchPageTest: () => /^\/video_ext\.php/.test(location.pathname) || /^\/video-?\d+_\d+/.test(location.pathname), - excludeSelector: ".ads-container video" + excludeSelector: ".ads-container video", + allowUnplayableMatch: true } for (const video of candidates) { if (isPlayableVideo(video)) return video } - return candidates[0] ?? null + return config.allowUnplayableMatch ? (candidates[0] ?? null) : nullAlso applies to: 288-312
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/video-detection.ts` around lines 49 - 55, The fallback that returns the first matched <video> is currently applied too broadly in the site detection logic, which can match non-playable preload or ad elements for sites like paramountplus, hotstar, and mubi. Add an explicit allowUnplayableMatch flag to SiteConfig and update the matching flow in the video detection code so only vkvideo enables this fallback; keep the other site configs on the playable-only path. Also update the inline detector in background.ts to use the same contract and gate the fallback through the same flag in the relevant matching function.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.example:
- Around line 10-12: The socket endpoint env key is mismatched between the
workflow and the extension, so production builds can fall back to localhost.
Update the GitHub Actions secret/injection in submit workflow to use
WXT_SOCKET_ENDPOINT instead of PLASMO_PUBLIC_SOCKET_ENDPOINT, and make sure the
extension-side lookup that reads WXT_SOCKET_ENDPOINT stays consistent with the
injected name.
In `@src/entrypoints/background.ts`:
- Around line 408-447: The current trustSelector bypass in the video candidate
selection logic is too broad and skips isPlayable for every site-configured
match, which can surface placeholder or hidden videos. Restrict this bypass in
the background video detection flow inside the candidate filtering around
collectAllVideos()/isPlayable so it only applies to the intended VK-style case,
or wire it to the same per-site flag used by src/lib/video-detection.ts; keep
generic site configs like Paramount+, Hotstar, and Mubi still requiring
isPlayable before returning candidates.
In `@src/entrypoints/popup/App.tsx`:
- Around line 247-253: The room join flow in App.tsx is validating the raw
`roomCode` from the `.then((roomCode: string) => ...)` handler, so
whitespace-only or newline-padded payloads can slip through as valid IDs.
Normalize the value first by trimming before the empty-string check, then use
the trimmed result for the `typeof`/length validation and for the `roomCallback`
call so only clean room codes are accepted.
---
Outside diff comments:
In `@src/lib/video-detection.ts`:
- Around line 49-55: The fallback that returns the first matched <video> is
currently applied too broadly in the site detection logic, which can match
non-playable preload or ad elements for sites like paramountplus, hotstar, and
mubi. Add an explicit allowUnplayableMatch flag to SiteConfig and update the
matching flow in the video detection code so only vkvideo enables this fallback;
keep the other site configs on the playable-only path. Also update the inline
detector in background.ts to use the same contract and gate the fallback through
the same flag in the relevant matching function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f963666a-5b7a-49e3-8913-6bae382e3a64
📒 Files selected for processing (7)
.env.example.gitignoresrc/entrypoints/background.tssrc/entrypoints/injected.tssrc/entrypoints/popup/App.tsxsrc/lib/video-detection.tssrc/runtime/videoPlayer.tsx
Comment on lines
+10
to
+12
| # The upstream production URL lives only in GitHub Actions secrets | ||
| # (PLASMO_PUBLIC_SOCKET_ENDPOINT in .github/workflows/submit.yml) and is | ||
| # not committed to this repo. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect submit workflow env/secret variable names.
fd -a 'submit\.yml$|submit\.yaml$' .github/workflows
for f in $(fd 'submit\.yml$|submit\.yaml$' .github/workflows); do
echo "=== $f ==="
sed -n '1,220p' "$f"
done
# Cross-check all references to socket endpoint env keys in repo.
rg -n "WXT_SOCKET_ENDPOINT|PLASMO_PUBLIC_SOCKET_ENDPOINT" . -SRepository: synclify/Synclify
Length of output: 2319
Rename the injected socket env key
.github/workflows/submit.yml still writes PLASMO_PUBLIC_SOCKET_ENDPOINT, but the extension reads WXT_SOCKET_ENDPOINT. Update the workflow/secret name to WXT_SOCKET_ENDPOINT so production builds don’t fall back to localhost.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.env.example around lines 10 - 12, The socket endpoint env key is mismatched
between the workflow and the extension, so production builds can fall back to
localhost. Update the GitHub Actions secret/injection in submit workflow to use
WXT_SOCKET_ENDPOINT instead of PLASMO_PUBLIC_SOCKET_ENDPOINT, and make sure the
extension-side lookup that reads WXT_SOCKET_ENDPOINT stays consistent with the
injected name.
Comment on lines
+408
to
+447
| /* Find candidate videos. When the site-specific selector matches, | ||
| trustSelector=true and we skip the isPlayable filter — the player | ||
| <video> may not yet have src/dimensions (e.g. VK Video uses MSE | ||
| blob-URL only after the first Play). On the fallback path and on | ||
| unknown sites we still require isPlayable so generic detection | ||
| picks the right element. */ | ||
| const allVideos = collectAllVideos() | ||
| let candidates: HTMLVideoElement[] | ||
| let trustSelector = false | ||
| if (siteConfig) { | ||
| // Use site-specific selector for more precise matching | ||
| candidates = Array.from( | ||
| document.querySelectorAll<HTMLVideoElement>(siteConfig.videoSelector) | ||
| ) | ||
| // Filter out excluded elements (ads, overlays, etc.) | ||
| if (siteConfig.excludeSelector) { | ||
| const excludeSel = siteConfig.excludeSelector | ||
| candidates = candidates.filter((v) => !v.matches(excludeSel)) | ||
| } | ||
| // If site-specific selector returned nothing, fall back to all videos | ||
| if (candidates.length === 0) { | ||
| candidates = Array.from(document.getElementsByTagName("video")) | ||
| const sel = siteConfig.videoSelector | ||
| const exclude = siteConfig.excludeSelector | ||
| candidates = allVideos.filter((v) => { | ||
| try { | ||
| if (!v.matches(sel)) return false | ||
| } catch { | ||
| return false | ||
| } | ||
| if (exclude) { | ||
| try { | ||
| if (v.matches(exclude)) return false | ||
| } catch { | ||
| /* ignore bad exclude selector */ | ||
| } | ||
| } | ||
| return true | ||
| }) | ||
| if (candidates.length > 0) { | ||
| trustSelector = true | ||
| } else { | ||
| // Fall back to all <video> (including shadow) if site selector matched nothing | ||
| candidates = allVideos | ||
| } | ||
| } else { | ||
| candidates = Array.from(document.getElementsByTagName("video")) | ||
| candidates = allVideos | ||
| } | ||
|
|
||
| return candidates | ||
| .map((video) => { | ||
| if (!isPlayable(video)) return null | ||
| if (!trustSelector && !isPlayable(video)) return null |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't bypass isPlayable() for every site match.
Line 447 currently trusts any site-configured selector match. For configs like paramountplus, hotstar, and mubi where videoSelector is just video, this will emit every matched element, including hidden/pre-roll placeholders, and can flip the flow to MULTIPLE_VIDEOS or the wrong videoId. Scope this bypass to VK only, or better, drive it from the same per-site flag as src/lib/video-detection.ts.
Suggested direction
let candidates: HTMLVideoElement[]
let trustSelector = false
if (siteConfig) {
const sel = siteConfig.videoSelector
const exclude = siteConfig.excludeSelector
@@
- if (candidates.length > 0) {
- trustSelector = true
+ if (candidates.length > 0) {
+ trustSelector = detectedSite === "vkvideo"
} else {
// Fall back to all <video> (including shadow) if site selector matched nothing
candidates = allVideos
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /* Find candidate videos. When the site-specific selector matches, | |
| trustSelector=true and we skip the isPlayable filter — the player | |
| <video> may not yet have src/dimensions (e.g. VK Video uses MSE | |
| blob-URL only after the first Play). On the fallback path and on | |
| unknown sites we still require isPlayable so generic detection | |
| picks the right element. */ | |
| const allVideos = collectAllVideos() | |
| let candidates: HTMLVideoElement[] | |
| let trustSelector = false | |
| if (siteConfig) { | |
| // Use site-specific selector for more precise matching | |
| candidates = Array.from( | |
| document.querySelectorAll<HTMLVideoElement>(siteConfig.videoSelector) | |
| ) | |
| // Filter out excluded elements (ads, overlays, etc.) | |
| if (siteConfig.excludeSelector) { | |
| const excludeSel = siteConfig.excludeSelector | |
| candidates = candidates.filter((v) => !v.matches(excludeSel)) | |
| } | |
| // If site-specific selector returned nothing, fall back to all videos | |
| if (candidates.length === 0) { | |
| candidates = Array.from(document.getElementsByTagName("video")) | |
| const sel = siteConfig.videoSelector | |
| const exclude = siteConfig.excludeSelector | |
| candidates = allVideos.filter((v) => { | |
| try { | |
| if (!v.matches(sel)) return false | |
| } catch { | |
| return false | |
| } | |
| if (exclude) { | |
| try { | |
| if (v.matches(exclude)) return false | |
| } catch { | |
| /* ignore bad exclude selector */ | |
| } | |
| } | |
| return true | |
| }) | |
| if (candidates.length > 0) { | |
| trustSelector = true | |
| } else { | |
| // Fall back to all <video> (including shadow) if site selector matched nothing | |
| candidates = allVideos | |
| } | |
| } else { | |
| candidates = Array.from(document.getElementsByTagName("video")) | |
| candidates = allVideos | |
| } | |
| return candidates | |
| .map((video) => { | |
| if (!isPlayable(video)) return null | |
| if (!trustSelector && !isPlayable(video)) return null | |
| /* Find candidate videos. When the site-specific selector matches, | |
| trustSelector=true and we skip the isPlayable filter — the player | |
| <video> may not yet have src/dimensions (e.g. VK Video uses MSE | |
| blob-URL only after the first Play). On the fallback path and on | |
| unknown sites we still require isPlayable so generic detection | |
| picks the right element. */ | |
| const allVideos = collectAllVideos() | |
| let candidates: HTMLVideoElement[] | |
| let trustSelector = false | |
| if (siteConfig) { | |
| const sel = siteConfig.videoSelector | |
| const exclude = siteConfig.excludeSelector | |
| candidates = allVideos.filter((v) => { | |
| try { | |
| if (!v.matches(sel)) return false | |
| } catch { | |
| return false | |
| } | |
| if (exclude) { | |
| try { | |
| if (v.matches(exclude)) return false | |
| } catch { | |
| /* ignore bad exclude selector */ | |
| } | |
| } | |
| return true | |
| }) | |
| if (candidates.length > 0) { | |
| trustSelector = detectedSite === "vkvideo" | |
| } else { | |
| // Fall back to all <video> (including shadow) if site selector matched nothing | |
| candidates = allVideos | |
| } | |
| } else { | |
| candidates = allVideos | |
| } | |
| return candidates | |
| .map((video) => { | |
| if (!trustSelector && !isPlayable(video)) return null |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/entrypoints/background.ts` around lines 408 - 447, The current
trustSelector bypass in the video candidate selection logic is too broad and
skips isPlayable for every site-configured match, which can surface placeholder
or hidden videos. Restrict this bypass in the background video detection flow
inside the candidate filtering around collectAllVideos()/isPlayable so it only
applies to the intended VK-style case, or wire it to the same per-site flag used
by src/lib/video-detection.ts; keep generic site configs like Paramount+,
Hotstar, and Mubi still requiring isPlayable before returning candidates.
Comment on lines
+247
to
+253
| .then((roomCode: string) => { | ||
| if (typeof roomCode !== "string" || roomCode.length === 0) { | ||
| setError(true) | ||
| setErrorMessage(t("videoNotDetectedYet")) | ||
| return | ||
| } | ||
| roomCallback(roomCode) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Normalize roomCode before validation to avoid accepting whitespace payloads.
res.text() payloads can contain trailing/newline whitespace; current logic accepts those as valid codes and can join with an invalid room ID.
Suggested fix
- .then((roomCode: string) => {
- if (typeof roomCode !== "string" || roomCode.length === 0) {
+ .then((roomCode: string) => {
+ const normalizedRoomCode =
+ typeof roomCode === "string" ? roomCode.trim() : ""
+ if (normalizedRoomCode.length === 0) {
setError(true)
setErrorMessage(t("videoNotDetectedYet"))
return
}
- roomCallback(roomCode)
+ roomCallback(normalizedRoomCode)
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| .then((roomCode: string) => { | |
| if (typeof roomCode !== "string" || roomCode.length === 0) { | |
| setError(true) | |
| setErrorMessage(t("videoNotDetectedYet")) | |
| return | |
| } | |
| roomCallback(roomCode) | |
| .then((roomCode: string) => { | |
| const normalizedRoomCode = | |
| typeof roomCode === "string" ? roomCode.trim() : "" | |
| if (normalizedRoomCode.length === 0) { | |
| setError(true) | |
| setErrorMessage(t("videoNotDetectedYet")) | |
| return | |
| } | |
| roomCallback(normalizedRoomCode) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/entrypoints/popup/App.tsx` around lines 247 - 253, The room join flow in
App.tsx is validating the raw `roomCode` from the `.then((roomCode: string) =>
...)` handler, so whitespace-only or newline-padded payloads can slip through as
valid IDs. Normalize the value first by trimming before the empty-string check,
then use the trimmed result for the `typeof`/length validation and for the
`roomCallback` call so only clean room codes are accepted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
vkvideoentry toSITE_CONFIGS(in bothsrc/lib/video-detection.tsand the inline copy serialised into pages fromsrc/entrypoints/background.ts), targeting the VK player rooted in an open Shadow DOM atdiv.shadow-root-container→.vk-vp-root→[data-testid="video-container"] video, with.ads-container videoas exclude.collectAllVideos/deepQuerySelector; routefindSiteVideo,findGenericVideo, the inlinedetectPageVideos, plus[data-synclify-id]lookups ininjected.tsandvideoPlayer.tsxthrough them.document.querySelector*does not cross shadow boundaries, so the previous code never saw the VK player.isPlayablefilter when the site-specific selector matches — VK uses MSE blob URLs that only appear after the first Play, so the<video>would otherwise be discarded for missingsrc/videoWidth.handleInject/reinjectTabto 5 × 800 ms; the VK SPA player loads noticeably slower than YouTube/Netflix chunks.createOrJoinRoomwas swallowing rejectedsendMessagepromises, so a missing socket server produced no feedback. Add.catchand an error pill under the Create Room button..env.exampledocumentingWXT_SOCKET_ENDPOINT; unignore it in.gitignore(the.env*glob was hiding it).Out of scope (left for a follow-up): collapsing the duplicated
SITE_CONFIGSbetweenvideo-detection.tsand the inlinedetectPageVideos. The inline copy can't share imports because it is serialised and run in the page viachrome.scripting.executeScript({ func }).Test plan
https://vkvideo.ru/video-387766_456270520, click the Synclify icon → Create Room. Expect "Video detected" toast and "connected and syncing" in the popup.isPlayablepath).autoInjectre-syncs the new player.https://vkvideo.ru/(no/video-X_Ypath) — Synclify must not activate (watchPageTestreturns false).<iframe src="https://vk.com/video_ext.php?oid=...&id=...">on a third-party page — detection works inside the frame (executeScriptalready runs withallFrames: true)./watchand Netflix/watch/...still detect, and Synclify still does not activate on YouTube/Netflix home pages.Summary by CodeRabbit
New Features
Bug Fixes
Documentation