Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Bug] www.google-analytics.com not blocked even with strict ETP on apkmirror.com #14071

Closed
s-ankur opened this issue Aug 22, 2020 · 22 comments
Closed
Assignees
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified Feature:TrackingProtection S2 Major Functionality/product severely impaired and a satisfactory workaround doesn't exist

Comments

@s-ankur
Copy link
Contributor

s-ankur commented Aug 22, 2020

from https://www.reddit.com/r/firefox/comments/ieh6ad/so_tracking_protection_now_allowing_google/

Steps to reproduce

open apkmirror.com
set etp to strict mode

Expected behavior

www.google-analytics.com is blocked

Actual behavior

it is not

Device information

  • Android device: Asus Zenfone
  • Fenix version: Nightly 200822 06:01 (Build #2015759411)
    AC: 56.0.20200821184145, 96a8a14dc
    GV: 81.0a1-20200820093209
    AS: 61.0.12

┆Issue is synchronized with this Jira Task

@s-ankur s-ankur added the 🐞 bug Crashes, Something isn't working, .. label Aug 22, 2020
@github-actions github-actions bot added the needs:triage Issue needs triage label Aug 22, 2020
@s-ankur
Copy link
Contributor Author

s-ankur commented Aug 22, 2020

Screenshot_20200822-200255_Firefox_Nightly

@s-ankur
Copy link
Contributor Author

s-ankur commented Aug 22, 2020

Another thing Ive noticed:
connect.facebook.com is blocked with Strict Mode ETP but not with Custom ETP.

@st3fan
Copy link
Contributor

st3fan commented Aug 22, 2020

@Amejia481 - do you think it is possible to implement a test either in A-C or Fenix (or both) to make sure we don't regress ETP?

@liuche this seeems to work in 79 and 80 but not on Nightly - probably a 81 release blocker.

@Amejia481
Copy link
Contributor

Amejia481 commented Aug 22, 2020

@Amejia481 - do you think it is possible to implement a test either in A-C or Fenix (or both) to make sure we don't regress ETP?

@liuche this seeems to work in 79 and 80 but not on Nightly - probably a 81 release blocker.

We have some UI tests on Fenix, completely agree, we have to detect regressions early.

This is related to the latest update on theWebCompat extension mozilla-mobile/android-components@5e9e03a, we added some rules to reduce tracking protection breakage. If I prevent the extension for being installed the issue disappears (Be aware, the web extension installation API is persistent, that means if you had previously installed the extension you need to uninstall and re-install the app without installing the extension to see the results). cc: @wisniewskit

Some logs from the js console:

Ads by Google is being shimmed by Firefox. See https://bugzilla.mozilla.org/show_bug.cgi?id=1629644 for details. sandbox eval code:1:9
Google Analytics is being shimmed by Firefox. See https://bugzilla.mozilla.org/show_bug.cgi?id=1493602 for details. sandbox eval code:1:9
Ads by Google is being shimmed by Firefox. See https://bugzilla.mozilla.org/show_bug.cgi?id=1629644 for details. sandbox eval code:1:9
Facebook SDK is being shimmed by Firefox. See https://bugzilla.mozilla.org/show_bug.cgi?id=1226498 for details. shim_messaging_helper.js:23:15
Some cookies are misusing the “SameSite“ attribute, so it won’t work as expected 3
Cookie “__cfduid” has been rejected because it is in a cross-site context and its “SameSite” is “Lax” or “Strict”

@Amejia481
Copy link
Contributor

I think the rules are only active on nightly as we want to get a bigger audience for testing

@Amejia481
Copy link
Contributor

Amejia481 commented Aug 22, 2020

A workaround in the meantime is to go to about:config and search for extensions.webcompat.enable_shims and toggle the preference to false.

@wisniewskit
Copy link

wisniewskit commented Aug 24, 2020

Actually, the requests are still being blocked, but the "tracking content" summary is unaware of that fact. The way it works is that the webcompat addon needs to be able to bypass regular tracking protection to replace those requests with a shim (more info here). We'll need to find a way to update the summary to indicate this.

@Amejia481, who would be best to talk to in order to figure out how to update the summary UI so it's aware that the requests are being shimmed instead of just being blocked?

@Amejia481
Copy link
Contributor

It depends in which layer of the stack we want to make the update, Fenix,AC,GV, or Gecko, lets chat about it and see the pros and cons. Right now the UI gets populated from ContentBlocking:RequestLog.

@Amejia481 Amejia481 self-assigned this Aug 24, 2020
@Amejia481
Copy link
Contributor

I tested locally with different variants beta and release, and the issue is not reproducible. It could a good idea, if the QA team adds this to their test plan to avoid leaking the bug to other variants.

@Amejia481 Amejia481 added the eng:qa:needed QA Needed label Aug 24, 2020
@liuche liuche removed the needs:triage Issue needs triage label Aug 25, 2020
@ebalazs-sv
Copy link

This issue is reproducible on Nightly 200826 GV 81, from 8/26 with Pixel 2 (Android 9).

The workaround from #14071 (comment) solves the issue.

Not reproducible on RC 80.1.0 GV 80, from 8/25 and Beta 80.0.1-beta.2 GV 80 from 8/18.

I have added a test to our smoke test suite. I will remove the qa:needed label for now.

@ebalazs-sv ebalazs-sv added Feature:TrackingProtection S2 Major Functionality/product severely impaired and a satisfactory workaround doesn't exist and removed eng:qa:needed QA Needed labels Aug 26, 2020
@Amejia481
Copy link
Contributor

Thanks @ebalazs-sv!

@clientenq
Copy link

How can workaround from #14071 (comment) solve the issue if the stable version has no about:config ?

@Amejia481
Copy link
Contributor

This feature is only activate on nightly, as we are testing it and on nightly about:config is accessible :)

@Amejia481
Copy link
Contributor

We are tracking the update of the extension on this Bugzilla bug https://bugzilla.mozilla.org/show_bug.cgi?id=1661330

@Amejia481 Amejia481 added the 🙅 waiting Issues that are blocked or has dependencies that are not ready label Sep 8, 2020
@lobontiumira
Copy link

This issue still reproduces on 10/8 Nightly build on HTC 10 (Android 8):
etp

@Amejia481
Copy link
Contributor

We landed a patch for exposing the right category, but we still need to add the category in the ui this will be covered on #15783

@Amejia481
Copy link
Contributor

This should be fixed in nightly :)

@Amejia481 Amejia481 added eng:qa:needed QA Needed and removed 🙅 waiting Issues that are blocked or has dependencies that are not ready labels Oct 27, 2020
@Amejia481
Copy link
Contributor

@s-ankur could you verify if it's working for you? :)

@s-ankur s-ankur closed this as completed Oct 27, 2020
@s-ankur s-ankur reopened this Oct 27, 2020
@s-ankur
Copy link
Contributor Author

s-ankur commented Oct 27, 2020

seems to work properly, sorry for closing this accidentally

@Amejia481
Copy link
Contributor

Thanks for verifying, the QA team will take a look as they have a wider range of devices where they can test :)

@ebalazs-sv
Copy link

ebalazs-sv commented Oct 29, 2020

Verified as fixed on Nightly 201029 05:01 (Build #2015772457) GV 84.0a1 from 10/29 with the following devices:

  • Google Pixel 2 (Android 9),
  • Motorola Moto G6 (Android 8),
  • Huawei P9 Lite (Android 7),
  • Nexus 5 (Android 6.0.1).

I will remove the qa:needed label and close this issue.

@ebalazs-sv ebalazs-sv added eng:qa:verified QA Verified and removed eng:qa:needed QA Needed labels Oct 29, 2020
@Amejia481
Copy link
Contributor

Thanks @ebalazs-sv!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified Feature:TrackingProtection S2 Major Functionality/product severely impaired and a satisfactory workaround doesn't exist
Projects
None yet
Development

No branches or pull requests

8 participants