Tracker protection getTabUrl#2445
Conversation
|
Cursor Agent can help with this pull request. Just |
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/jkt/auto/tracker-protection-gettaburl-5e89")git submodule (Windows): git -C submodules/content-scope-scripts fetch origin pr-releases/jkt/auto/tracker-protection-gettaburl-5e89
git -C submodules/content-scope-scripts checkout origin/pr-releases/jkt/auto/tracker-protection-gettaburl-5e89Pin to exact commitnpm (Android / Extension): Swift Package Manager (Apple): .package(url: "https://github.com/duckduckgo/content-scope-scripts.git", revision: "6dcd3b26ce3bad044824047b151affdcbce7f7df")git submodule (Windows): git -C submodules/content-scope-scripts fetch origin pr-releases/jkt/auto/tracker-protection-gettaburl-5e89
git -C submodules/content-scope-scripts checkout 6dcd3b26ce3bad044824047b151affdcbce7f7df |
[Beta] Generated file diffTime updated: Fri, 29 May 2026 22:28:35 GMT AppleFile has changed IntegrationFile has changed |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Dead stores left behind after destroy() removal
- Removed the four dead property assignments (this._originalXHROpen, this._originalXHRSend, this._originalFetch, this._originalImageSrc) that had no remaining readers after the destroy() method was removed.
Or push these changes by commenting:
@cursor push fb9031323f
Preview (fb9031323f)
diff --git a/injected/src/features/tracker-protection.js b/injected/src/features/tracker-protection.js
--- a/injected/src/features/tracker-protection.js
+++ b/injected/src/features/tracker-protection.js
@@ -157,9 +157,6 @@
}
return originalSend.apply(this, args);
};
-
- this._originalXHROpen = originalOpen;
- this._originalXHRSend = originalSend;
}
_setupFetchInterception() {
@@ -183,8 +180,6 @@
}
return originalFetch.apply(window, args);
};
-
- this._originalFetch = originalFetch;
}
_setupImageSrcInterception() {
@@ -192,7 +187,6 @@
const originalDescriptor = Object.getOwnPropertyDescriptor(Image.prototype, 'src');
if (!originalDescriptor?.get || !originalDescriptor?.set) return;
- this._originalImageSrc = originalDescriptor;
/** @type {WeakSet<HTMLImageElement>} */
const imgTracked = new WeakSet();
const origGet = originalDescriptor.get;| this._observer = null; | ||
|
|
||
| // Get top-level URL for tracker matching | ||
| this._topLevelUrl = getTabURL(); |
There was a problem hiding this comment.
Dead stores left behind after destroy() removal
Low Severity
The destroy() method was removed as dead code, but the instance property assignments it relied on — this._originalXHROpen, this._originalXHRSend, this._originalFetch, and this._originalImageSrc — are still being set in _setupXHRInterception, _setupFetchInterception, and _setupImageSrcInterception. These are now dead stores with no remaining readers.
Please tell me if this was useful or not with a 👍 or 👎.
Additional Locations (2)
There was a problem hiding this comment.
PR already exists and description doesn't need updating. Done.
Removed the four dead stores (this._originalXHROpen, this._originalXHRSend, this._originalFetch, this._originalImageSrc) that were only used by the now-deleted destroy() method. All 166 TrackerResolver specs and unit tests pass.
|
This PR requires a manual review and approval from a member of one of the following teams:
|
|
@cursoragent rebase over main |
|
Rebased onto latest main ( |
…destroy() - Delete local getTabURL() function (functionally near-identical to utils version) - Import getTabUrl from ../utils.js - Replace the single call site in init() - Remove unused destroy() method (never called by ContentFeature lifecycle) Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com>
There was a problem hiding this comment.
Web Compatibility Assessment
No findings. The changed runtime path replaces the local top-level URL helper with existing getTabUrl() and removes unreachable cleanup/dead-store code; I did not find a new API-surface fidelity, DOM timing, SPA, or platform-compat regression in the changed lines.
Security Assessment
No findings. The shared getTabUrl() keeps the same top-location/referrer/ancestorOrigins fallback model used by the removed helper, and the diff does not relax message-bridge, origin-validation, config-gating, or iframe trust-boundary checks.
Risk Level
Medium Risk: this touches injected tracker-protection URL resolution used for blocking/reporting decisions, but does not add new API hooks, messaging paths, or security-sensitive trust-boundary changes.
Recommendations
No blocking recommendations. Targeted verification passed: npm run build-surrogates --workspace=injected, npm run tsc-strict-core, and npm run test-unit --workspace=injected -- --filter="TrackerProtection".
Sent by Cursor Automation: Web compat and sec
cursor
Bot
force-pushed
the
jkt/auto/tracker-protection-gettaburl-5e89
branch
from
7b93d8a to
e6d8ebb
Compare
3 participants
Asana Task/Github Issue: N/A
Description
getTabURL()function intracker-protection.jswith the sharedgetTabUrlutility from../utils.js, removing a duplicate implementation without behavioral change.destroy()method fromTrackerProtection, as it was dead code and never called.These changes address specific PR review comments.
Testing Steps
npm testand verify all unit tests pass.npm test -- injected/src/features/tracker-protection.jsand verify all TrackerResolver specs pass.Checklist
Please tick all that apply: