TODO:
- Use utils'
getTabUrl() in tracker-protection instead of internal function
- Delete
getTabURL() from tracker-protection.js
- Import
getTabUrl from ../utils.js
- Replace the single call site (
this._topLevelUrl = getTabURL() in init())
- Zero behavioral change for tracker-protection since both resolve to the same URL
The two functions are functionally near-identical. Both:
- Try
globalThis.top.location.href first (full URL with path)
- On cross-origin
catch, fall back to ancestorOrigins (origin-only) and document.referrer
- Wrap the result in
new URL() with a null-safe catch
- Return
URL | null
Minor differences:
| Aspect |
utils.js getTabUrl() |
tracker-protection.js getTabURL() |
| Fallback priority in catch |
ancestorOrigins ?? referrer |
referrer, then ancestorOrigins overrides |
| Null guard on top |
@ts-expect-error + bare access |
Optional chaining top?.location.href |
| Null input to URL |
Passes null to new URL() (throws, caught) |
Explicit framingOrigin ? guard |
The fallback priority difference is the only semantic one: utils.js prefers ancestorOrigins over referrer (via ??), while tracker-protection.js lets ancestorOrigins override referrer. In practice the result is the same — when ancestorOrigins is available it wins in both cases; the referrer assignment in the tracker-protection version is just overwritten.
- The tracker-protection version was intentionally written with the bugbot-flagged
ancestorOrigins fix already applied (the comment at line 33–34 is explicit about it). The utils.js version arguably has a subtler bug — if ancestorOrigins returns null and referrer is empty string, ?? picks the empty string (truthy for ?? but invalid for new URL). Unifying would be a good opportunity to pick the better implementation and propagate it, but that's a separate discussion.
- The dead-code nature makes the change trivial and safe to defer.
A one-line import swap in a fast-follow is low risk and keeps this PR focused on the tracker-protection feature itself.
Originally posted by @cursor[bot] in #2322 (comment)
-
Zap destroy() dead code (see comment)
-
Do we want this in CI?
TODO:
getTabUrl()in tracker-protection instead of internal functiongetTabURL()fromtracker-protection.jsgetTabUrlfrom../utils.jsthis._topLevelUrl = getTabURL()ininit())The two functions are functionally near-identical. Both:
globalThis.top.location.hreffirst (full URL with path)catch, fall back toancestorOrigins(origin-only) anddocument.referrernew URL()with a null-safe catchURL | nullMinor differences:
utils.jsgetTabUrl()tracker-protection.jsgetTabURL()ancestorOrigins ?? referrerreferrer, thenancestorOriginsoverrides@ts-expect-error+ bare accesstop?.location.hrefnulltonew URL()(throws, caught)framingOrigin ?guardThe fallback priority difference is the only semantic one:
utils.jsprefersancestorOriginsoverreferrer(via??), whiletracker-protection.jsletsancestorOriginsoverridereferrer. In practice the result is the same — whenancestorOriginsis available it wins in both cases; thereferrerassignment in the tracker-protection version is just overwritten.ancestorOriginsfix already applied (the comment at line 33–34 is explicit about it). Theutils.jsversion arguably has a subtler bug — ifancestorOriginsreturnsnullandreferreris empty string,??picks the empty string (truthy for??but invalid fornew URL). Unifying would be a good opportunity to pick the better implementation and propagate it, but that's a separate discussion.A one-line import swap in a fast-follow is low risk and keeps this PR focused on the tracker-protection feature itself.
Originally posted by @cursor[bot] in #2322 (comment)
Zap
destroy()dead code (see comment)Do we want this in CI?