Closes #311: enable tracking protection #318
Conversation
I think request.isForMainFrame() is unnecessary because of this line in
UrlMatcher.matches:
if (pageHost != null && pageHost.equals(resourceHost)) {
return false;
}
Which whitelists first party requests: this is assuming the `pageHost`, which
comes from `TrackingProtectionWebViewClient.currentPageUrl`, actually
represents the current page loading (and thus the main frame).
I noted that my Android TV emulator seems to block the same numbers of trackers
as Focus.
|
|
||
| // Don't block the main frame from being loaded. This also protects against cases where we | ||
| // open a link that redirects to another app (e.g. to the play store). | ||
| final Uri pageUri = Uri.parse(currentPageURL); | ||
|
|
||
| // if ((!request.isForMainFrame()) && |
There was a problem hiding this comment.
I dug through some code and it looks like the isForMainFrame was added to prevent non-browser links (like market://) from crashing on this matcher check.
If you verify that it still works, then I'm okay with it, but otherwise I'd leave it in.
There was a problem hiding this comment.
I can't use isForMainFrame because with this version of the webView we get a string, rather than the expected type we get in Focus.
That being said, I'll dig in on your research – thanks for doing that (good idea!)!
There was a problem hiding this comment.
it looks like the isForMainFrame was added to prevent non-browser links (like market://) from crashing on this matcher check.
Looking at the linked issue, it looks like the http/http(s) checks were added to prevent crashing with malformed URLs like (market:://) but isForMainFrame is unrelated. #38 is open for fixing the http(s) check, which is out of scope of this issue (and we haven't seen any problems for it yet - maybe AmazonWebView is different because we're on a TV).
The
isBlockingEnabledflag should still work when we add the UI.From my commit summary:
I think request.isForMainFrame() is unnecessary because of this line in
UrlMatcher.matches:
Which whitelists first party requests: this is assuming the
pageHost, whichcomes from
TrackingProtectionWebViewClient.currentPageUrl, actuallyrepresents the current page loading (and thus the main frame).
I noted that my Android TV emulator seems to block the same numbers of trackers
as Focus.