Pass login state with youtube detection event - #2677
Conversation
[Beta] Generated file diffTime updated: Wed, 29 Apr 2026 17:26:57 GMT Android
File has changed AppleFile has changed Chrome-mv3File has changed FirefoxFile has changed IntegrationFile has changed WindowsFile has changed |
Made-with: Cursor
1a451fb to
fe9afc7
Compare
Build Branch
Static preview entry points
QR codes (mobile preview)
Integration commandsnpm (Android / Extension): Swift Package Manager (Apple): .package(url: "https://github.com/duckduckgo/content-scope-scripts.git", branch: "pr-releases/youtube-detection-login-state")git submodule (Windows): git -C submodules/content-scope-scripts fetch origin pr-releases/youtube-detection-login-state
git -C submodules/content-scope-scripts checkout origin/pr-releases/youtube-detection-login-statePin to exact commitnpm (Android / Extension): Swift Package Manager (Apple): .package(url: "https://github.com/duckduckgo/content-scope-scripts.git", revision: "75c5e423583c63f388f4128b0d076b2e9c8cc5db")git submodule (Windows): git -C submodules/content-scope-scripts fetch origin pr-releases/youtube-detection-login-state
git -C submodules/content-scope-scripts checkout 75c5e423583c63f388f4128b0d076b2e9c8cc5db |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Web Compatibility Assessment
No findings. The PR does not add or modify browser API wrappers/shims, prototype patches, DOM mutation behavior, iframe access, or timing-sensitive load-time code. The changed detector event path preserves the existing webEvents.fireEvent contract and continues to send a structured { type, data } payload.
Security Assessment
No findings. The new loginState payload is constructed explicitly from detector state as { loginState: ... }, with no object spreading or forwarding of incoming/native payloads. It still flows through webEvents.fireEvent, which constructs notify params explicitly and avoids nativeData leakage.
Risk Level
Medium Risk: this changes an injected runtime event payload sent to native/webEvents consumers, but it is narrowly scoped, config-gated through the existing YouTube detection event settings, and does not touch message transports, bridge security checks, captured globals, or API overrides.
Recommendations
No blocking recommendations. Optional cleanup: update the stale runYoutubeAdDetection JSDoc callback type to include the data argument so future callsites match the new detector callback contract.
Validation: npm run test-unit -- --filter='YouTubeAdDetector|WebEvents' passed (30 specs, 0 failures).
Sent by Cursor Automation: Web compat and sec
|
This PR requires a manual review and approval from a member of one of the following teams:
|
There was a problem hiding this comment.
Web Compatibility Assessment
No findings.
The change only extends the existing webEvent payload for YouTube detection events with an internally derived data.loginState string. It does not add or modify browser API overrides, prototype patches, DOM mutations, event listeners, timers, or iframe access patterns.
Security Assessment
No findings.
The outgoing message remains explicitly constructed as { type, data }; there is no spread of page-controlled objects and no nativeData forwarding path. The new loginState value is synthesized from detector state and constrained to the detector's known state strings or unknown.
Risk Level
Medium Risk: this changes an injected runtime event payload sent through native messaging on supported platforms, but the behavior is narrowly scoped, config-gated by existing YouTube detection event settings, and does not touch wrapper utilities, captured globals, message bridge security checks, or API shims.
Recommendations
No blocking recommendations. Consider adding fixture coverage for non-unknown login states (logged-in, logged-out, premium) if native consumers depend on those exact values.
Validation run:
npm run test-unit -- unit-test/youtube-ad-detection.jsnpx playwright test integration-test/web-interference-detection-events.spec.js --reporter list
Sent by Cursor Automation: Web compat and sec
GuiltyDolphin
left a comment
GuiltyDolphin
left a comment
There was a problem hiding this comment.
Haven't tested (assuming you've tested and this is a simple enough change). Let me know if you need manual testing and I can look into it tomorrow.
Overall looks good. Main ask is can we add tests for the non-unknown cases? It'd be great to have integration/e2e tests covering this such that we can check the correct login state is being sent when appropriate indicators are on the page.
Maybe see if Cursor can build a couple of those. If it's a lot of work we can skip.
2 participants
Asana Task/Github Issue:
https://app.asana.com/1/137249556945/project/72649045549333/task/1214157598747980
Description
Adding login state to the youtube detection events
Testing Steps
cd injected && npm run test-unit
cd injected && npx playwright test web-interference-detection-events --reporter list
Checklist
Please tick all that apply:
Note
Medium Risk
Changes the runtime payload shape for
webEventnotifications (addsdata.loginState), which may affect downstream consumers expecting an empty object; logic is otherwise small and covered by tests.Overview
YouTube interference detection events now include a
datapayload with the detector’s currentloginState(defaulting to'unknown') when firingyoutube_*web events.This threads an optional
dataargument throughYouTubeAdDetector’sonEventcallback andWebInterferenceDetection’swebEvents.fireEventcall, and updates unit/integration tests to assert the new event shape.Reviewed by Cursor Bugbot for commit fe9afc7. Bugbot is set up for automated code reviews on this repo. Configure here.