Closed
Bug 1461534
Opened 7 years ago
Closed 7 years ago
Honor user and add-on overrides in tracking annotations
Categories
(Toolkit :: Safe Browsing, enhancement, P1)
Toolkit
Safe Browsing
Tracking
()
VERIFIED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | verified |
People
(Reporter: francois, Assigned: francois)
References
Details
Attachments
(2 files)
We'll need to remove this early return in nsChannelClassifier::ShouldEnableTrackingProtectionInternal():
https://searchfox.org/mozilla-central/rev/2b9779c59390ecc47be7a70d99753653d8eb5afc/netwerk/base/nsChannelClassifier.cpp#378-383
since there may be user overrides and add-ons may need to prevent their loads from being annotated as tracking.
We'll also need to avoid caching marking annotated channels as already classified:
https://searchfox.org/mozilla-central/rev/2b9779c59390ecc47be7a70d99753653d8eb5afc/netwerk/base/nsChannelClassifier.cpp#706
There may be a performance impact to allowing users to override tracking annotations since we'll be doing permission manager lookups for every network request at BeginConnectContinue() time:
https://searchfox.org/mozilla-central/rev/2b9779c59390ecc47be7a70d99753653d8eb5afc/netwerk/protocol/http/nsHttpChannel.cpp#6394
https://searchfox.org/mozilla-central/rev/2b9779c59390ecc47be7a70d99753653d8eb5afc/netwerk/base/nsChannelClassifier.cpp#1203
Assignee | ||
Updated•7 years ago
|
Priority: -- → P2
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Updated•7 years ago
|
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•7 years ago
|
||
Here are the steps to test my fix:
1. start Firefox (fresh profile) with MOZ_LOG="nsChannelClassifier:3"
2. go to https://cnn.com
3. look for evidence that trackers are de-prioritized on the terminal log:
Setting PRIORITY_LOWEST for channel[0x7fb06add7050] (https://dt.adsafeprotected.com/)
4. set tracking protection to Always in about:preferences#privacy
5. reload https://cnn.com
6. click on shield in the URL bar and then "Disable For This Site"
7. set tracking protection back to "Only in private windows" in about:preferences#privacy
8. reload https://cnn.com
9. look to ensure there aren't any messages like this on the terminal:
Setting PRIORITY_LOWEST for channel[0x7fb06add7050] (https://dt.adsafeprotected.com/)
10. instead, confirm that the terminal log includes messages like:
User override on channel[0x7fb07a031050] (https://revee.outbrain.com/page/view) for https://www.cnn.com"
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8989884 [details]
Bug 1461534 - Enable user/add-on overrides of tracking annotations.
https://reviewboard.mozilla.org/r/254876/#review262440
::: netwerk/base/nsChannelClassifier.cpp
(Diff revision 1)
> - // Unlike full Tracking Protection, annotations don't block anything
> - // so we don't need to take into account add-ons or user exceptions.
> - if (aAnnotationsOnly) {
> - *result = true;
> - return NS_OK;
> - }
Do we plan to use |aAnnotationsOnly| in the future?
If not, I think we can also remove it from parameter.
Attachment #8989884 -
Flags: review?(dlee) → review+
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8989885 [details]
Bug 1461534 - Improve logging for the channel classifier.
https://reviewboard.mozilla.org/r/254878/#review262442
::: netwerk/base/nsChannelClassifier.cpp:1264
(Diff revision 1)
> }
>
> nsCString trackingBlacklist =
> CachedPrefs::GetInstance()->GetTrackingBlackList();
> if (trackingBlacklist.IsEmpty()) {
> - LOG(("nsChannelClassifier[%p]:CheckIsTrackerWithLocalTable blacklist is empty",
> + LOG_WARN(("nsChannelClassifier[%p]:CheckIsTrackerWithLocalTable blacklist is empty",
whitepsace after [%p]:
Attachment #8989885 -
Flags: review?(dlee) → review+
Assignee | ||
Comment 6•7 years ago
|
||
(In reply to Dimi Lee[:dimi][:dlee] from comment #4)
> Do we plan to use |aAnnotationsOnly| in the future?
Yes, in fact we'll be extending that feature :)
Comment hidden (mozreview-request) |
Pushed by fmarier@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b4818535bde9
Enable user/add-on overrides of tracking annotations. r=dimi
https://hg.mozilla.org/integration/autoland/rev/152002e28671
Improve logging for the channel classifier. r=dimi
Comment 9•7 years ago
|
||
Backed out 2 changesets (bug 1461534) for nsChannelClassifier.cpp related build bustages.
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=152002e286718d7ff9c64c9aaa4fb1e50a720622
Backout link: https://hg.mozilla.org/integration/autoland/rev/6354a8dfce2fa6f5fd973e61daaf1e94a618c850
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=187195469&repo=autoland
Flags: needinfo?(francois)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(francois)
Comment 11•7 years ago
|
||
Pushed by fmarier@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4b7cb393a455
Enable user/add-on overrides of tracking annotations. r=dimi
https://hg.mozilla.org/integration/autoland/rev/58c256b60254
Improve logging for the channel classifier. r=dimi
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4b7cb393a455
https://hg.mozilla.org/mozilla-central/rev/58c256b60254
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Updated•7 years ago
|
Flags: qe-verify+
Comment 13•7 years ago
|
||
I can not reproduced this issue on nightly 2018.05.14, the log file not populated with messages.
I try to reproduce on another beta build 62.0b4 (2018.06.28) before the fix but the log file not populated.
I manage to verified it on 63.0b7 on Windows 10 x64, in log file I see this message:
User override on channel[0000022201A47040] (https://revee.outbrain.com/page/view). This message is ok? The message from step 9 not there.
Flags: needinfo?(francois)
Assignee | ||
Comment 14•7 years ago
|
||
(In reply to Timea Zsoldos [:zstimi/tzsoldos], Desktop Release QA from comment #13)
> I manage to verified it on 63.0b7 on Windows 10 x64, in log file I see this
> message:
> User override on channel[0000022201A47040]
> (https://revee.outbrain.com/page/view). This message is ok? The message from
> step 9 not there.
That looks good to me.
Flags: needinfo?(francois)
Comment 15•7 years ago
|
||
I can confirm this issue is fixed, I verified using Firefox 63.0b11 on Ubuntu 18.04 x64, Windows 10 x64 and Mac OS X 10.13 and based on comment 14, removed the flag qe+ and mark this issue as verified-fixed.
Updated•7 years ago
|
QA Contact: francois
You need to log in
before you can comment on or make changes to this bug.
Description
•