Closed
Bug 1503560
Opened 6 years ago
Closed 5 years ago
Tracker blocking rarely working
Categories
(GeckoView :: General, defect, P1)
GeckoView
General
Tracking
(geckoview62 wontfix, geckoview63 wontfix, geckoview64 fixed, firefox63 wontfix, firefox64 fixed, firefox65 fixed)
RESOLVED
FIXED
mozilla65
People
(Reporter: davidb, Assigned: esawin)
Details
Attachments
(1 file, 2 obsolete files)
1.84 KB,
patch
|
esawin
:
review+
jcristau
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•6 years ago
|
||
Susheel we think this might be a Focus issue. Is there a related GitHub issue filed already?
Flags: needinfo?(sdaswani)
Reporter | ||
Updated•6 years ago
|
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)
Reporter | ||
Comment 3•6 years ago
|
||
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).
Reporter | ||
Comment 4•6 years ago
|
||
Eugen can you dig into this?
Assignee: nobody → esawin
Priority: -- → P1
Comment 5•6 years ago
|
||
Here is the Focus issue: https://github.com/mozilla-mobile/focus-android/issues/3826
status-firefox63:
--- → wontfix
status-firefox64:
--- → affected
status-firefox65:
--- → affected
status-geckoview62:
--- → wontfix
status-geckoview63:
--- → wontfix
Assignee | ||
Comment 6•6 years ago
|
||
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.
Updated•6 years ago
|
Attachment #9023490 -
Flags: review?(francois) → review+
Assignee | ||
Comment 7•6 years ago
|
||
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)
Assignee | ||
Comment 8•6 years ago
|
||
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).
Comment 9•6 years ago
|
||
(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)
Assignee | ||
Comment 10•6 years ago
|
||
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)
Updated•6 years ago
|
Attachment #9023993 -
Flags: review?(francois) → review+
Comment 11•6 years ago
|
||
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
Comment 12•6 years ago
|
||
Backed out changeset c9bd800631be (bug 1503560) for perma failing browser_trackingUI_fetch.js, test_bug1274685_unowned_list.js push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&group_state=expanded&selectedJob=210899138&revision=c9bd800631be838f0f9f99f31ff6b58ef5f407f5 failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&group_state=expanded&fromchange=01219b0ae60ee8a2f3302ceb1804330caee54abe&selectedJob=210913580&searchStr=linux%2Copt%2Cxpcshell%2Ctests%2Ctest-linux32%2Fopt-xpcshell-8%2Cx%28x8%29 https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&group_state=expanded&fromchange=01219b0ae60ee8a2f3302ceb1804330caee54abe&selectedJob=210926935&searchStr=windows%2C10%2Cx64%2Cdebug%2Cmochitests%2Cwith%2Ce10s%2Ctest-windows10-64%2Fdebug-mochitest-browser-chrome-e10s-1%2Cm-e10s%28bc1%29 backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/d33d5a71940366ca9d0ed7609dc5dfac2c5a9355
Flags: needinfo?(esawin)
Assignee | ||
Comment 13•5 years ago
|
||
Add default pref value, r=me.
Attachment #9023993 -
Attachment is obsolete: true
Flags: needinfo?(esawin)
Attachment #9024495 -
Flags: review+
Comment 14•5 years ago
|
||
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
Comment 15•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9a3984fec4a4
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Assignee | ||
Comment 17•5 years ago
|
||
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 18•5 years ago
|
||
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+
Comment 19•5 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/e14b94e8cc97
Updated•5 years ago
|
Product: Firefox for Android → GeckoView
Updated•5 years ago
|
Target Milestone: Firefox 65 → mozilla65
You need to log in
before you can comment on or make changes to this bug.
Description
•