Closed Bug 1503560 Opened 6 years ago Closed 5 years ago

Tracker blocking rarely working

Categories

(GeckoView :: General, defect, P1)

defect

Tracking

(geckoview62 wontfix, geckoview63 wontfix, geckoview64 fixed, firefox63 wontfix, firefox64 fixed, firefox65 fixed)

RESOLVED FIXED
mozilla65
Tracking Status
geckoview62 --- wontfix
geckoview63 --- wontfix
geckoview64 --- fixed
firefox63 --- wontfix
firefox64 --- fixed
firefox65 --- fixed

People

(Reporter: davidb, Assigned: esawin)

Details

Attachments

(1 file, 2 obsolete files)

James and I confirmed similar findings while dogfooding. This affects at least Focus 8.0/63

STR
1. go to cnn
2. check # blocked trackers
3. often 0 (expected lots)

Sometimes it works but not often -- something is broken somehere.
Susheel we think this might be a Focus issue. Is there a related GitHub issue filed already?
Flags: needinfo?(sdaswani)
Summary: Tracking rarely working → Tracker blocking rarely working
Jeff, how does Focus Android calculate this number? Do we just get it from GV / WV?
Flags: needinfo?(sdaswani)
Flags: needinfo?(jboek)
Flags: needinfo?(j)
EmilyK and Eugen were looking into this a bit. It sounds like STR might include this step: "delete cache and app data via android settings", (which is also something I happened to do recently, before seeing the bug).
Eugen can you dig into this?
Assignee: nobody → esawin
Priority: -- → P1
Recent changes to the blocklist download mechanics (bug 1482306 and bug 1501978) require the pref "privacy.trackingprotection.enabled" to be true, otherwise TP blocklist updates and initial downloads are disabled.

The pref isn't otherwise used in GeckoView, since we control TP through a GeckoSession setting which translates to a property on the docshell.

Our tests haven't picked this issue up, because we (can) only test against test-* blocklists which are added independently from the pref setting.
Flags: needinfo?(jboek)
Flags: needinfo?(j)
Attachment #9023490 - Flags: review?(francois)
Attachment #9023490 - Flags: review?(francois) → review+
The patch will help with the intermediate issue of TP being broken in GeckoView, but the dependency on the pref renders docshell.useTrackingProtection moot (see [1]).

François, is there a good way to resolve this? Currently, the GeckoSession.USE_TRACKING_PROTECTION setting is the only way to control TP on a session level. Having separate prefs for the TP active state and TP blocklist update state would be one way to resolve the issue for us.

[1] https://searchfox.org/mozilla-central/source/docshell/base/nsDocShell.cpp#12926
Flags: needinfo?(francois)
Alternatively, we could extend SafeBrowsing.controlUpdateChecking() to accept options to control the individual lists and therefore override the prefs, e.g., SafeBrowsing.enableUpdates({phishing: true, tracking: true, ...}).

Reducing and/or moving pref-dependencies out of the modules would generally help us with GeckoView development in the future since we have a strict decoupling of the session (~window) and its runtime (~Gecko).
(In reply to Eugen Sawin [:esawin] from comment #7)
> François, is there a good way to resolve this? Currently, the
> GeckoSession.USE_TRACKING_PROTECTION setting is the only way to control TP
> on a session level. Having separate prefs for the TP active state and TP
> blocklist update state would be one way to resolve the issue for us.

In the short-term, perhaps we could try to find a way to set `this.trackingEnabled`: https://searchfox.org/mozilla-central/rev/17f55aee76b7c4610a974cffd3453454e0c8de7b/toolkit/components/url-classifier/SafeBrowsing.jsm#237

to true on GeckoView. We used to be able to have #ifdef's in JS code, but I'm not sure that's possible anymore. Would there be another way to look for GeckoView in there?

i.e.

  this.trackingEnabled =
      Services.isThisGeckoView() ||
      Services.prefs.getBoolPref("privacy.trackingprotection.enabled") ||
      Services.prefs.getBoolPref("privacy.trackingprotection.pbmode.enabled");

Longer-term, I'm currently looking at revamping the URL Classifier list management and I'll add this as a req
Flags: needinfo?(francois)
For a short-term solution, we could override the TP pref with GeckoView's telemetry pref, which is only and always enabled for GeckoView.
Attachment #9023490 - Attachment is obsolete: true
Attachment #9023993 - Flags: review?(francois)
Attachment #9023993 - Flags: review?(francois) → review+
Pushed by esawin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9bd800631be
[1.1] Override tracking protection pref on GeckoView to re-enable blocklist updates. r=francois
Add default pref value, r=me.
Attachment #9023993 - Attachment is obsolete: true
Flags: needinfo?(esawin)
Attachment #9024495 - Flags: review+
Pushed by esawin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a3984fec4a4
[1.2] Override tracking protection pref on GeckoView to re-enable blocklist updates. r=francois
https://hg.mozilla.org/mozilla-central/rev/9a3984fec4a4
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Eugen let's get this approved for beta.
Flags: needinfo?(esawin)
Comment on attachment 9024495 [details] [diff] [review]
0001-Bug-1503560-1.2-Override-tracking-protection-pref-on.patch

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1482306

User impact if declined: Tracking protection does not work, not trackers are blocked.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): The patch only affects GeckoView behaviour by enabling tracking protection blocklist updates by default.

String changes made/needed: None
Flags: needinfo?(esawin)
Attachment #9024495 - Flags: approval-mozilla-beta?
Comment on attachment 9024495 [details] [diff] [review]
0001-Bug-1503560-1.2-Override-tracking-protection-pref-on.patch

fix TP on geckoview, approved for beta64
Attachment #9024495 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
status-geckoview64=fixed
Product: Firefox for Android → GeckoView
Target Milestone: Firefox 65 → mozilla65
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: