Closed
Bug 1168658
Opened 7 years ago
Closed 7 years ago
Enable tracking protection in b2g
Categories
(Firefox OS Graveyard :: Runtime, defect)
Tracking
(firefox41 fixed)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: fabrice, Assigned: fabrice)
References
Details
Attachments
(2 files, 2 obsolete files)
François, I'm not sure to have all the pieces together here. On desktop when loading http://ericschmidt.com/ I see that blocking works because the console logs "The resource at "https://platform.twitter.com/widgets.js" was blocked because tracking protection is enabled." and the twitter handle is unstyled. However on b2g we still load everything. Any idea what I'm missing?
Attachment #8610928 -
Flags: feedback?(francois)
Assignee | ||
Comment 1•7 years ago
|
||
I updated https://people.mozilla.org/~fdesre/ to serve as a test page as well.
Comment 2•7 years ago
|
||
Comment on attachment 8610928 [details] [diff] [review] wip Review of attachment 8610928 [details] [diff] [review]: ----------------------------------------------------------------- Here's another test page: https://people.mozilla.org/~fmarier/tracking-test/ Since tracking protection relies on the Safe Browsing code, are you sure that works on B2G yet? This page should be blocked if you have malware protection turned on: http://itisatrap.org/firefox/its-an-attack.html Also, until bug 1157081 is fixed, either malware or phishing protection must be turned on in the prefs, you can't just have tracking protection on. I can see you've done that correctly in your patch.
Attachment #8610928 -
Flags: feedback?(francois)
Assignee | ||
Comment 3•7 years ago
|
||
Thanks François. Malware protection works fine with this patch, and using your test page I get a "You have successfully enabled tracking protection. Please continue!" message... So I'm really not sure why the blocking of twitter js code doesn't work on b2g - I made sure to start with a fresh profile and no http cache. Are we using different lists depending on the UA or platform?
Comment 4•7 years ago
|
||
The list is the same on all platform. It comes from: https://services.disconnect.me/disconnect-plaintext.json but we remove these entries: https://github.com/mozilla-services/shavar-list-exceptions/blob/master/allow_list Twitter.com is on our allow list so it shouldn't be blocked. Perhaps what you're seeing on desktop is due to a stale list entry (we currently have a bug on our server implementation which means you may not have received the removal). Have you tried a fresh profile on desktop? I don't get a shield icon on http://ericschmidt.com/ so it seems like it's working fine on B2G :)
Assignee | ||
Comment 5•7 years ago
|
||
You're right, my default profile has a stale list. A fresh desktop profile and the b2g one are matching!
Assignee | ||
Comment 6•7 years ago
|
||
Attachment #8610928 -
Attachment is obsolete: true
Attachment #8613674 -
Flags: review?(francois)
Comment 7•7 years ago
|
||
Comment on attachment 8613674 [details] [diff] [review] tracking-protection.patch Review of attachment 8613674 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/app/b2g.js @@ +356,2 @@ > > +pref("browser.safebrowsing.debug", true); I think we should leave this off by default. @@ +403,5 @@ > // URL for checking the reason for a malware warning. > pref("browser.safebrowsing.malware.reportURL", "https://safebrowsing.google.com/safebrowsing/diagnostic?client=%NAME%&hl=%LOCALE%&site="); > + > +// Tracking protection > +pref("privacy.trackingprotection.enabled", true); If it's on, it means TP is always on for every page load, Private Browsing mode or otherwise. While I think that's a good idea on B2G, I think you need a product person to decide what the default should be for this option. As a first step, perhaps we should add an option in the developer options to toggle this ON and OFF? @@ +404,5 @@ > pref("browser.safebrowsing.malware.reportURL", "https://safebrowsing.google.com/safebrowsing/diagnostic?client=%NAME%&hl=%LOCALE%&site="); > + > +// Tracking protection > +pref("privacy.trackingprotection.enabled", true); > +pref("privacy.trackingprotection.pbmode.enabled", true); This one is unnecessary if the previous one is true. `pbmode.enabled` means that in Private Browsing, TP is always on.
Attachment #8613674 -
Flags: review?(francois) → review-
Assignee | ||
Comment 8•7 years ago
|
||
Addressed review comments. I've let it enabled by default, and we'll decide if we want to change that when doing the UI work in bug 1154605.
Attachment #8613674 -
Attachment is obsolete: true
Attachment #8617591 -
Flags: review?(francois)
Comment 9•7 years ago
|
||
Comment on attachment 8617591 [details] [diff] [review] tracking-protection.patch v2 Review of attachment 8617591 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/app/b2g.js @@ +356,3 @@ > > // Prevent loading of pages identified as malware > +pref("browser.safebrowsing.malware.enabled", true); BTW, once bug 1157081 has landed we won't need to have these turned ON for tracking protection to work.
Attachment #8617591 -
Flags: review?(francois) → review+
https://hg.mozilla.org/mozilla-central/rev/3c8a737a5f8a
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S14 (12june)
Comment 12•7 years ago
|
||
Backed out in https://hg.mozilla.org/mozilla-central/rev/bfd82015df48 for causing frequent Gij(10) failures in homescreen_navigation_test.js and edges_gesture_test.js like https://treeherder.mozilla.org/logviewer.html#?job_id=2135010&repo=b2g-inbound and https://treeherder.mozilla.org/logviewer.html#?job_id=2134285&repo=b2g-inbound
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: 2.2 S14 (12june) → ---
Assignee | ||
Comment 14•7 years ago
|
||
I need this gaia prefs to not get weird intermittent reflow errors in Gij(10). Green try run at https://treeherder.mozilla.org/#/jobs?repo=try&revision=fcb249781bfa
Attachment #8624973 -
Flags: review?(kgrandon)
Comment 15•7 years ago
|
||
Comment on attachment 8624973 [details] [review] gaia tests patch Very strange that you get those reflow errors, but this looks fine to me. Thanks!
Attachment #8624973 -
Flags: review?(kgrandon) → review+
Assignee | ||
Comment 16•7 years ago
|
||
Gaia part: https://github.com/mozilla-b2g/gaia/commit/311c4e59936a407e64509f54fecb440d8a78e3c8
Comment 18•7 years ago
|
||
Fabrice, now that Bug 1157081 is fixed, it would be a good idea to flip the Safe Browsing prefs off by default (and test that TP still works) because Safe Browsing is way more data-hungry than TP.
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
Setting ni? to ensure comment 18 is not missed.
Flags: needinfo?(fabrice)
Comment 20•7 years ago
|
||
This is why test_cost_control_data_alert_mobile.py in Gaia UI test is failing. This is sure going to give me a nice bill from my provider while trying to find the regression range for that bug. Is safe browsing supposed to be on while on a mobile data connection?
Assignee | ||
Comment 21•7 years ago
|
||
(In reply to Martijn Wargers [:mwargers] (QA) from comment #20) > This is why test_cost_control_data_alert_mobile.py in Gaia UI test is > failing. This is sure going to give me a nice bill from my provider while > trying to find the regression range for that bug. > > Is safe browsing supposed to be on while on a mobile data connection? Martijn, You can unset the pref like I did for a couple of other gaia tests (look at the PR).
Flags: needinfo?(fabrice)
Comment 22•7 years ago
|
||
We should also unset it globally like I mentioned in comment 18.
Assignee | ||
Comment 23•7 years ago
|
||
(In reply to François Marier [:francois] from comment #22) > We should also unset it globally like I mentioned in comment 18. I'll be happy to review ;)
Comment 24•7 years ago
|
||
(In reply to Fabrice Desré [:fabrice] from comment #21) > Martijn, > You can unset the pref like I did for a couple of other gaia tests (look at > the PR). Yes, I know. Question is more, would it be wise to download MBs of data while the user is on a mobile data connection? I don't think so.
Comment 25•7 years ago
|
||
(In reply to Martijn Wargers [:mwargers] (QA) from comment #24) > Yes, I know. Question is more, would it be wise to download MBs of data > while the user is on a mobile data connection? I don't think so. Fabrice, I can take care of bug 1177553, but would you consider this a valid bug? I think b2g needs to be careful with data when using mobile data, no?
Flags: needinfo?(fabrice)
Assignee | ||
Comment 26•7 years ago
|
||
Yes, I'm surprised that we download so much. We need to investigate with a real profile, ie. not one that is reset all the time to run tests.
Flags: needinfo?(fabrice)
Comment 27•7 years ago
|
||
(In reply to Fabrice Desré [:fabrice] from comment #26) > Yes, I'm surprised that we download so much. We need to investigate with a > real profile, ie. not one that is reset all the time to run tests. I filed bug 1179770 with a logcat.
Comment 28•7 years ago
|
||
So safe browsing is disabled again in bug 1180503. Why was it enabled in the first place, then?
Comment 29•7 years ago
|
||
(In reply to Martijn Wargers [:mwargers] (QA) from comment #28) > So safe browsing is disabled again in bug 1180503. Why was it enabled in the > first place, then? See my review comments in comment 9.
You need to log in
before you can comment on or make changes to this bug.
Description
•