Conversation
📝 WalkthroughWalkthroughThis PR adds support for a new Hotstar app with middleware-based ad-blocking and telemetry-disabling rules, introduces auto picture-in-picture functionality for Netflix, implements a rule support system for conditional feature-based rule enabling, extends DNR rules for Hotstar, and enhances the fetch polyfill with header tracking and response mocking support. Changes
Sequence DiagramssequenceDiagram
participant Client as Request
participant Middleware as blockAds Middleware
participant Parser as JSON Parser
participant Remover as Payload Transform
participant Response as Response Patch
Client->>Middleware: Intercept /api/internal/bff/v2/
Middleware->>Parser: Parse response JSON
Parser-->>Middleware: Parsed payload
Middleware->>Remover: removeDisplayAdWidgets()
Middleware->>Remover: removePlayerAdData()
Remover-->>Middleware: Ads removed, indices recorded
Middleware->>Response: Patch response if removals occurred
Response-->>Client: Updated response with ads stripped
sequenceDiagram
participant Content as Content Script
participant RuleSupport as withRuleSupport
participant FeatureDetect as isRuleSupported
participant Config as App Config
participant UI as UI (RuleItem)
Content->>RuleSupport: Enrich app configs
RuleSupport->>FeatureDetect: Check Netflix PiP support
FeatureDetect-->>RuleSupport: boolean (device capability)
RuleSupport->>Config: Attach supported flag to rules
Config-->>UI: Pass enriched config
UI->>UI: Disable rule if supported=false
UI-->>UI: Show unsupportedDescription if disabled
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/fetch-polyfill.ts (1)
72-94:⚠️ Potential issue | 🟠 MajorXHR header rewrites stop at
ctx.initand never reach the real request.These lines capture headers for middleware, but they still call
originalSetRequestHeader()beforerunMiddlewares(ctx). Any middleware update toctx.init.headersis ignored on pass-through XHRs, and the captured header set also survives acrossopen()calls on a reused XHR instance.💡 Suggested direction
- const requestHeaders = new Headers(); + let requestHeaders = new Headers(); xhr.open = function ( _method: string, _url: string | URL, async?: boolean, username?: string | null, password?: string | null, ) { method = _method; url = _url.toString(); + requestHeaders = new Headers(); return originalOpen(_method, _url, async ?? true, username, password); }; xhr.setRequestHeader = function (name: string, value: string) { - requestHeaders.set(name, value); - return originalSetRequestHeader(name, value); + requestHeaders.append(name, value); }; xhr.send = async function ( body?: Document | XMLHttpRequestBodyInit | null, ) { requestBody = body; logger.debug("🎯 Processing XHR for:", url); const ctx: MiddlewareContext = { request: url, init: { method, body: body as BodyInit | undefined, headers: requestHeaders, }, url, handled: false, setHandled() { this.handled = true; }, setResponse(resp: Response) { this.response = resp; }, response: undefined, originalFetch: originalFetch.bind(window), }; await runMiddlewares(ctx); + + for (const [name, value] of new Headers( + ctx.init?.headers ?? requestHeaders, + ).entries()) { + originalSetRequestHeader(name, value); + } if (ctx.handled && ctx.response) {Also applies to: 103-108
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/fetch-polyfill.ts` around lines 72 - 94, The override of xhr.setRequestHeader calls originalSetRequestHeader immediately (using originalSetRequestHeader) so middleware changes to ctx.init.headers never get applied and captured requestHeaders persist across reused XHRs; fix by resetting method, url, requestBody and requestHeaders inside xhr.open (clear previous state), change xhr.setRequestHeader to only mutate the local requestHeaders (do NOT call originalSetRequestHeader there), and move the calls to originalSetRequestHeader into the send path (inside xhr.send where runMiddlewares(ctx) is executed): after runMiddlewares(ctx) merge requestHeaders with ctx.init.headers and then iterate those headers to call originalSetRequestHeader before invoking originalSend; reference the symbols xhr.open, xhr.setRequestHeader, originalSetRequestHeader, requestHeaders, requestBody, method, url, runMiddlewares, xhr.send, and originalSend.
🧹 Nitpick comments (1)
src/lib/rule-support.ts (1)
18-20: Preserve config-authored support flags when annotating rules.
withRuleSupport()currently replaces any existingrule.supportedvalue. That makes a statically unsupported rule render as supported unlessisRuleSupported()happens to special-case it, which also bypasses theunsupportedDescriptionpath insrc/components/OTTModal.tsx.Suggested change
rules: app.rules.map((rule) => ({ ...rule, - supported: isRuleSupported(app.id, rule.id), + supported: + rule.supported !== false && isRuleSupported(app.id, rule.id), })),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/rule-support.ts` around lines 18 - 20, withRuleSupport currently overwrites any existing rule.supported flag; change the mapping in withRuleSupport (the app.rules.map callback) to only assign supported when the rule doesn’t already define it (e.g. use rule.supported === undefined ? isRuleSupported(app.id, rule.id) : rule.supported), leaving unsupportedDescription and any existing support flags intact so static “unsupported” metadata still flows through to OTTModal.tsx; do this by updating the mapping that references isRuleSupported and rule.supported rather than unconditionally replacing rule.supported.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/apps/hotstar/block-ads.ts`:
- Around line 168-170: The current code calls ctx.originalFetch(ctx.request,
ctx.init) and then on later errors falls back to await next(), which can replay
the request; instead preserve and return the original Response when patching
fails: after successfully awaiting ctx.originalFetch and storing const response,
avoid calling await next() on subsequent failures—catch patching/parsing errors,
log or handle them, and return the previously saved response; apply the same
change for the second occurrence handling the fetch at the block that spans the
other affected section (the code that clones
response.toString()/response.clone().text() and later may call next()) so no
successful fetch is re-sent via next().
In `@src/lib/apps/hotstar/middleware.ts`:
- Around line 3-33: The hotstarHeaderMiddleware export is not registered in the
Hotstar app config so it never runs; update the Hotstar configuration where
middlewares are registered (the same place that currently registers blockAds and
disableTelemetry) to import hotstarHeaderMiddleware and add it to the middleware
list/array so it gets executed for requests (ensure ordering if needed so
hotstarHeaderMiddleware runs before any middleware that might short-circuit
requests).
In `@src/lib/apps/netflix/auto-picture-in-picture.ts`:
- Around line 9-49: The current code in startAutoPictureInPicturePatch
disconnects the MutationObserver after the first successful call to
patchAutoPictureInPictureOnSingleVideo, which prevents re-patching when Netflix
swaps video elements; instead, keep the observer running so
patchAutoPictureInPictureOnSingleVideo is invoked on subsequent mutations
(remove the observer.disconnect() call in the MutationObserver callback) and
optionally add a lightweight debounce/throttle inside
startAutoPictureInPicturePatch or the observer callback to limit frequency;
refer to the observer variable and the functions startAutoPictureInPicturePatch
and patchAutoPictureInPictureOnSingleVideo when making the change.
In `@src/lib/apps/netflix/config.ts`:
- Around line 31-32: The current binding of startAutoPictureInPicturePatch to
AppRule.onInit creates a MutationObserver on document.documentElement with no
teardown path; instead, remove calling startAutoPictureInPicturePatch from
onInit and either (A) move its invocation into autoPictureInPictureMode (the
middleware) where you can create the observer only when a video page is active
and disconnect it when the middleware tears down, or (B) change
startAutoPictureInPicturePatch to return a disposer (a function that disconnects
the observer) and register that disposer with the rule's teardown lifecycle;
update the config to stop wiring startAutoPictureInPicturePatch directly to
onInit so the observer is always disconnected when the rule/middleware is
stopped.
In `@src/lib/dnr-rules.ts`:
- Around line 233-241: The telemetry rule(s) (e.g., the rule with id 1111 that
uses HOTSTAR_INITIATOR_DOMAINS and TELEMETRY_RESOURCE_TYPES) incorrectly block
generic redirect/navigation hosts like "||t.co/" and "||www.google.co.in", which
breaks non-telemetry flows; fix by removing those generic domain urlFilters from
telemetry rules and either (a) narrow each urlFilter to specific telemetry
collector paths/parameters for Hotstar, or (b) move the generic domain blocks
into a separate navigation/blocking rule set (not the Disable Telemetry rules).
Ensure changes reference the rule entries that include HOTSTAR_INITIATOR_DOMAINS
and TELEMETRY_RESOURCE_TYPES so only telemetry-specific endpoints remain
blocked.
- Around line 126-130: HOTSTAR_INITIATOR_DOMAINS is too narrow and should use
the same origin pattern as the Hotstar app config: replace the hard-coded
["www.hotstar.com"] with the pattern "(^|\\.)hotstar\\.com$" (or the equivalent
RegExp/string used in src/lib/apps/hotstar/config.ts) so DNR rules match
hotstar.com and all subdomains; also remove or narrow the generic Google block
(the rule that blocks "||www.google.co.in/") and instead block only the specific
telemetry host(s) (e.g., analytics.google.com) to avoid breaking legitimate
Google navigation.
In `@wxt.config.ts`:
- Around line 18-31: Remove the third-party host permissions listed in the host
permissions array in wxt.config.ts that are only used for DNR blocking (the
entries for analytics.google.com, www.googletagmanager.com,
connect.facebook.net, static.ads-twitter.com, bat.bing.com, a.quora.com, t.co,
usersvc.hotstar.com, www.google-analytics.com, www.google.co.in,
*.doubleclick.net, *.ingest.sentry.io, q.quora.com, cdn.growthbook.io) and
instead rely on the DNR blocking rules in src/lib/dnr-rules.ts; also remove the
duplicate usersvc.hotstar.com entry since it is covered by the existing
*.hotstar.com wildcard. Ensure no other code references these hosts before
removing them.
---
Outside diff comments:
In `@src/lib/fetch-polyfill.ts`:
- Around line 72-94: The override of xhr.setRequestHeader calls
originalSetRequestHeader immediately (using originalSetRequestHeader) so
middleware changes to ctx.init.headers never get applied and captured
requestHeaders persist across reused XHRs; fix by resetting method, url,
requestBody and requestHeaders inside xhr.open (clear previous state), change
xhr.setRequestHeader to only mutate the local requestHeaders (do NOT call
originalSetRequestHeader there), and move the calls to originalSetRequestHeader
into the send path (inside xhr.send where runMiddlewares(ctx) is executed):
after runMiddlewares(ctx) merge requestHeaders with ctx.init.headers and then
iterate those headers to call originalSetRequestHeader before invoking
originalSend; reference the symbols xhr.open, xhr.setRequestHeader,
originalSetRequestHeader, requestHeaders, requestBody, method, url,
runMiddlewares, xhr.send, and originalSend.
---
Nitpick comments:
In `@src/lib/rule-support.ts`:
- Around line 18-20: withRuleSupport currently overwrites any existing
rule.supported flag; change the mapping in withRuleSupport (the app.rules.map
callback) to only assign supported when the rule doesn’t already define it (e.g.
use rule.supported === undefined ? isRuleSupported(app.id, rule.id) :
rule.supported), leaving unsupportedDescription and any existing support flags
intact so static “unsupported” metadata still flows through to OTTModal.tsx; do
this by updating the mapping that references isRuleSupported and rule.supported
rather than unconditionally replacing rule.supported.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6219b54e-c071-4093-8fdb-a6082879bfdd
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
.changeset/tender-heads-marry.md.changeset/thirty-webs-travel.md.gitignorepackage.jsonsrc/components/OTTModal.tsxsrc/components/RuleItem.tsxsrc/entrypoints/content/index.tsxsrc/lib/apps/hotstar/block-ads.tssrc/lib/apps/hotstar/config.tssrc/lib/apps/hotstar/disable-telemetry.tssrc/lib/apps/hotstar/middleware.tssrc/lib/apps/netflix/auto-picture-in-picture.tssrc/lib/apps/netflix/config.tssrc/lib/apps/netflix/picture-in-picture.tssrc/lib/apps/registry.tssrc/lib/apps/ui-config.tssrc/lib/dnr-rules.tssrc/lib/fetch-polyfill.tssrc/lib/rule-support.tssrc/lib/shared/types.tswxt.config.ts
💤 Files with no reviewable changes (1)
- .gitignore
Comment on lines
+3
to
+33
| export const hotstarHeaderMiddleware: Middleware = async (ctx, next) => { | ||
| const HEADER_NAME = "x-hs-client"; | ||
| const PATTERN_TO_REPLACE = /platform:web/g; | ||
| const REPLACEMENT_VALUE = "platform:android"; | ||
|
|
||
| if (!ctx.url.includes("hotstar.com")) { | ||
| await next(); | ||
| return; | ||
| } | ||
|
|
||
| let headers: Headers | undefined; | ||
| if (ctx.init?.headers instanceof Headers) { | ||
| headers = ctx.init.headers; | ||
| } else if (ctx.init?.headers && typeof ctx.init.headers === "object") { | ||
| headers = new Headers(ctx.init.headers as Record<string, string>); | ||
| } | ||
|
|
||
| if (headers?.has(HEADER_NAME)) { | ||
| const originalValue = headers.get(HEADER_NAME) || ""; | ||
| const modifiedValue = originalValue.replace( | ||
| PATTERN_TO_REPLACE, | ||
| REPLACEMENT_VALUE | ||
| ); | ||
| if (originalValue !== modifiedValue) { | ||
| headers.set(HEADER_NAME, modifiedValue); | ||
| ctx.init = { ...ctx.init, headers }; | ||
| } | ||
| } | ||
|
|
||
| await next(); | ||
| }; |
There was a problem hiding this comment.
Wire this middleware into the Hotstar config or it never runs.
The current Hotstar config only registers blockAds and disableTelemetry, so this export stays inert and x-hs-client is never rewritten to platform:android.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/apps/hotstar/middleware.ts` around lines 3 - 33, The
hotstarHeaderMiddleware export is not registered in the Hotstar app config so it
never runs; update the Hotstar configuration where middlewares are registered
(the same place that currently registers blockAds and disableTelemetry) to
import hotstarHeaderMiddleware and add it to the middleware list/array so it
gets executed for requests (ensure ordering if needed so hotstarHeaderMiddleware
runs before any middleware that might short-circuit requests).
Comment on lines
+9
to
+49
| function patchAutoPictureInPictureOnSingleVideo(): boolean { | ||
| const videos = document.querySelectorAll("video"); | ||
| if (videos.length !== 1) { | ||
| return false; | ||
| } | ||
|
|
||
| const video = videos[0] as AutoPictureInPictureVideo; | ||
| video.removeAttribute("disablepictureinpicture"); | ||
| video.setAttribute("autopictureinpicture", ""); | ||
|
|
||
| if ("autoPictureInPicture" in video) { | ||
| video.autoPictureInPicture = true; | ||
| } | ||
|
|
||
| logger.info("[Netflix] Enabled automatic Picture-in-Picture on video", { | ||
| source: "netflix", | ||
| middleware: "auto-picture-in-picture", | ||
| }); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| export function startAutoPictureInPicturePatch() { | ||
| if (!isAutoPictureInPictureSupported()) { | ||
| return; | ||
| } | ||
|
|
||
| if (patchAutoPictureInPictureOnSingleVideo()) { | ||
| return; | ||
| } | ||
|
|
||
| const observer = new MutationObserver(() => { | ||
| if (patchAutoPictureInPictureOnSingleVideo()) { | ||
| observer.disconnect(); | ||
| } | ||
| }); | ||
|
|
||
| observer.observe(document.documentElement, { | ||
| childList: true, | ||
| subtree: true, | ||
| }); |
There was a problem hiding this comment.
Keep patching across video swaps instead of disconnecting after the first success.
src/entrypoints/script.ts:37-41 only calls onInit() once. After observer.disconnect() runs here, any later <video> replacement on episode/title navigation will never get patched. The inverse case is also rough: pages that never reach videos.length === 1 keep a full-document observer alive until unload.
Suggested change
+const patchedVideos = new WeakSet<HTMLVideoElement>();
+
function patchAutoPictureInPictureOnSingleVideo(): boolean {
const videos = document.querySelectorAll("video");
if (videos.length !== 1) {
return false;
}
const video = videos[0] as AutoPictureInPictureVideo;
+ if (patchedVideos.has(video)) {
+ return true;
+ }
+
video.removeAttribute("disablepictureinpicture");
video.setAttribute("autopictureinpicture", "");
if ("autoPictureInPicture" in video) {
video.autoPictureInPicture = true;
}
+
+ patchedVideos.add(video);
logger.info("[Netflix] Enabled automatic Picture-in-Picture on video", {
source: "netflix",
middleware: "auto-picture-in-picture",
});
@@
- const observer = new MutationObserver(() => {
- if (patchAutoPictureInPictureOnSingleVideo()) {
- observer.disconnect();
- }
- });
+ const observer = new MutationObserver(() => {
+ patchAutoPictureInPictureOnSingleVideo();
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/apps/netflix/auto-picture-in-picture.ts` around lines 9 - 49, The
current code in startAutoPictureInPicturePatch disconnects the MutationObserver
after the first successful call to patchAutoPictureInPictureOnSingleVideo, which
prevents re-patching when Netflix swaps video elements; instead, keep the
observer running so patchAutoPictureInPictureOnSingleVideo is invoked on
subsequent mutations (remove the observer.disconnect() call in the
MutationObserver callback) and optionally add a lightweight debounce/throttle
inside startAutoPictureInPicturePatch or the observer callback to limit
frequency; refer to the observer variable and the functions
startAutoPictureInPicturePatch and patchAutoPictureInPictureOnSingleVideo when
making the change.
Comment on lines
+31
to
+32
| middleware: autoPictureInPictureMode, | ||
| onInit: startAutoPictureInPicturePatch, |
There was a problem hiding this comment.
Avoid binding a non-disposable DOM observer to onInit.
Line 32 wires startAutoPictureInPicturePatch into onInit, but that helper creates a MutationObserver on document.documentElement and only disconnects after the patch succeeds (src/lib/apps/netflix/auto-picture-in-picture.ts:31-50). AppRule.onInit is () => void, so there is no teardown path if the user never reaches a single-video page or disables the rule later. That leaves a tab-wide observer running for the lifetime of the document.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/apps/netflix/config.ts` around lines 31 - 32, The current binding of
startAutoPictureInPicturePatch to AppRule.onInit creates a MutationObserver on
document.documentElement with no teardown path; instead, remove calling
startAutoPictureInPicturePatch from onInit and either (A) move its invocation
into autoPictureInPictureMode (the middleware) where you can create the observer
only when a video page is active and disconnect it when the middleware tears
down, or (B) change startAutoPictureInPicturePatch to return a disposer (a
function that disconnects the observer) and register that disposer with the
rule's teardown lifecycle; update the config to stop wiring
startAutoPictureInPicturePatch directly to onInit so the observer is always
disconnected when the rule/middleware is stopped.
Comment on lines
+233
to
+241
| { | ||
| id: 1111, | ||
| priority: 1, | ||
| action: { type: "block" }, | ||
| condition: { | ||
| urlFilter: "||t.co/", | ||
| initiatorDomains: HOTSTAR_INITIATOR_DOMAINS, | ||
| resourceTypes: TELEMETRY_RESOURCE_TYPES, | ||
| }, |
There was a problem hiding this comment.
Don't block generic redirect/search hosts in a telemetry rule.
Lines 238 and 278 block t.co and www.google.co.in, which are general navigation endpoints, not telemetry-specific collectors. Shipping them under “Disable Telemetry” can break share/help/login flows initiated from Hotstar even though the user only opted into telemetry blocking.
✂️ Suggested change
{
- id: 1111,
- priority: 1,
- action: { type: "block" },
- condition: {
- urlFilter: "||t.co/",
- initiatorDomains: HOTSTAR_INITIATOR_DOMAINS,
- resourceTypes: TELEMETRY_RESOURCE_TYPES,
- },
- },
- {
id: 1112,
priority: 1,
action: { type: "block" },
@@
{
- id: 1115,
- priority: 1,
- action: { type: "block" },
- condition: {
- urlFilter: "||www.google.co.in/",
- initiatorDomains: HOTSTAR_INITIATOR_DOMAINS,
- resourceTypes: TELEMETRY_RESOURCE_TYPES,
- },
- },
-];
+];Also applies to: 273-281
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/dnr-rules.ts` around lines 233 - 241, The telemetry rule(s) (e.g.,
the rule with id 1111 that uses HOTSTAR_INITIATOR_DOMAINS and
TELEMETRY_RESOURCE_TYPES) incorrectly block generic redirect/navigation hosts
like "||t.co/" and "||www.google.co.in", which breaks non-telemetry flows; fix
by removing those generic domain urlFilters from telemetry rules and either (a)
narrow each urlFilter to specific telemetry collector paths/parameters for
Hotstar, or (b) move the generic domain blocks into a separate
navigation/blocking rule set (not the Disable Telemetry rules). Ensure changes
reference the rule entries that include HOTSTAR_INITIATOR_DOMAINS and
TELEMETRY_RESOURCE_TYPES so only telemetry-specific endpoints remain blocked.
Merged
This was referenced Mar 6, 2026
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 by CodeRabbit
New Features
Chores