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?

I updated https://people.mozilla.org/~fdesre/ to serve as a test page as well.

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.

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?

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 :)

You're right, my default profile has a stale list. A fresh desktop profile and the b2g one are matching!

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.

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.

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.

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

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

Comment on attachment 8624973 [details] gaia tests patch Very strange that you get those reflow errors, but this looks fine to me. Thanks!

Gaia part: https://github.com/mozilla-b2g/gaia/commit/311c4e59936a407e64509f54fecb440d8a78e3c8

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.

Setting ni? to ensure comment 18 is not missed.

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?

(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).

We should also unset it globally like I mentioned in comment 18.

(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 ;)

(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.

(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?

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.

(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.

So safe browsing is disabled again in bug 1180503. Why was it enabled in the first place, then?

(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.