Open Bug 1733370 Opened 3 years ago Updated 3 years ago

Support onlyIfDFPIActive flag for shim content scripts

Categories

(Core :: Privacy: Anti-Tracking, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: pbz, Unassigned)

References

(Blocks 1 open bug)

Details

Currently content scripts run with dFPI disabled even if onlyIfDFPIActive is set.
We should honor this flag and only register content scripts if dFPI is enabled. It would also be good to react to pref changes on runtime here.

It might be enough to add to the check here:
https://searchfox.org/mozilla-central/source/browser/extensions/webcompat/lib/shims.js#147

But that's not async, and the check is: https://searchfox.org/mozilla-central/source/browser/extensions/webcompat/lib/shims.js#577

So we would probably just want to cache the value ahead of time in a global variable, reading it in enabled. We could initially check it in the Promise.all statement here: https://searchfox.org/mozilla-central/source/browser/extensions/webcompat/lib/shims.js#100

And we could provide an experimental API into trackingProtection.js to monitor the prefs changing: https://searchfox.org/mozilla-central/source/browser/extensions/webcompat/experiment-apis/aboutConfigPrefs.js#17

But it's possible that we might want to refactor the content script handling code so that it always registers the content scripts, before any async code like that Promise.all check (which might fix the case where the cached tab initially loaded by Firefox from the user's previous session does not get the content script). In that case we would probably want to change the code around a bit more as well.

See Also: → 1733566

(In reply to Thomas Wisniewski [:twisniewski] from comment #1)

So we would probably just want to cache the value ahead of time in a global variable, reading it in enabled. We could initially check it in the Promise.all statement here: https://searchfox.org/mozilla-central/source/browser/extensions/webcompat/lib/shims.js#100
nt script). In that case we would probably want to change the code around a bit more as well.

We have separate dFPI states for private and normal browsing. That means we need this info when making the decision to register or unregister the content script. I can see that you're getting the PB flag via browser.tabs.get(details.tabId)).incognito;. However, that means we need to have an existing tab. Seems like a tricky stage to register or unregister content scripts.
Perhaps we could always register the content scripts and send them a "disabled" signal at the earliest occasion?

Flags: needinfo?(twisniewski)

Sure, we could do that. Maybe it's even worth arguing for a new add-on API as well, as content scripts already have access to a browser.extension.inIncognitoContext value. Maybe we could also grant them access to something like browser.extension.inFirstPartyIsolatedContext value (or some sort of finer-grained object like trackingProtectionSettings, so we can also grant access to other values we might need, like whether content blocking is active).

Of course if dPFI is actively toggled on tabs when the user flips the about:config preference (as opposed to being "locked" on a tab until it reloads) then we might need an event content scripts could listen to, which would complicate this.

Flags: needinfo?(twisniewski)
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.