1465877 - Some options are not getting disabled or enabled; Tracking Protection does not work

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[J.D.M]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20180531101452

Steps to reproduce:

After some updates that Firefox Nightly for Android received, some options are not getting enabled or disabled, for example, if I enable Cookies, excluding third parties (the default is Enabled) the option is enabled, but if I leave the browser and start it again, some options that I've enabled or disabled are not saved.

Also, the tracking protection is not working, a test site is the "Apkmirror", when I enter the site, the icon that shows that the tracking option is enabled, is not enabled, I have done tests with several sites and all the sites I tested have the same problem. So I do not know if this is a bug.

I've tested the Beta version of the browser, and everything works as it should. The tracking protection is working and the options are according to the option I chose.

I'm using an Xiaomi Mi A1;

System info: https://i.imgur.com/jbue1rW.jpg
Firefox Nightly: https://1drv.ms/v/s!AhHaljE4bL520SiHz_GLqKSWAFQd (Onedrive)
Firefox Beta: https://1drv.ms/v/s!AhHaljE4bL520Scf2nF3nnkqwLD6 (Onedrive)



Expected results:

The options should be enabled or disabled depending on the choice I made. The tracking protection should be working and the icon being displayed.

Comment 1

[J.D.M]

I think I discovered the problem, I do not know if I'm wrong, but the Firefox Nightly version of Google Play is in trouble, I downloaded the latest version of the browser through the Mozilla server and the browser worked correctly, the tracking protection worked and the options are saved, since the Google Play version does not work as it should, and it is having problems as mentioned above.

Firefox Nightly (Google Play): https://1drv.ms/v/s!AhHaljE4bL520Sm-pploEUO1q1vb (Onedrive)
Firefox FTP Mozilla: https://1drv.ms/v/s!AhHaljE4bL520SrMtQpbZ4aFswIq (Onedrive)

Comment 2

[rdz_bug]

I have the tracking protection bug. It doesn't work anymore.

Comment 3

[rdz_bug]

Some quick test showed urlclassifier.trackingTable is set to blank. 

You can reset to the default value but after browser restart it is again set to blank.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Thanks for the analysis. Bogdan, can you reproduce? If so, can you NI Susheel on this bug? Not listening to user preferences seems like a bad regression.

Comment 5

[Bogdan Surd, QA [:bsurd, NI]]

Devices:
Reproduced on:
 - Huawei MediaPad M2 (Android 5.1.1);
 - Nokia 6 (Android 7.1.1);
Did not reproduce on:
 - Nexus 9 (Android 6.0.1);

Hello and thank you for you report, regarding the tracking protection we do have Bug 1466511 that confirms it, this is available for both the google play and FTP versions. 

As for options toggling on and off only Cookies seem to always go back to Enabled from any other option selected. Tracking protection toggle is not affected, or any other options from what I've noticed on my devices. As mentioned in Comment 1 only the Play Store version of Nightly(2018-06-04) is affected, FTP version(2018-06-04) works as expected.

NI-ing Susheel for input and prioritization.

Comment 6

[:sdaswani]

The Softvision Dev Crew will take a look at this as a high priority item. I think we have some time before it ships.

Comment 7

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

https://www.androidauthority.com/state-of-usb-type-c-870996/ is an example of a page that in Nightly (from Play Store) with Tracking Protection enabled for all browsing shows a lot of ads, which based on spot-checks are supposed to be on the Tracking Protection list, and doesn't show the Tracking Protection shield icon while in release-channel Firefox (also from Play Store) the Tracking Protection shield is shown and the ads don't appear.

Comment 9

[François Marier [:francois]]

(In reply to Rodze from comment #3)
> Some quick test showed urlclassifier.trackingTable is set to blank. 

That's why tracking protection doesn't work.

To make it work:

1. go into about:config and reset the value of urlclassifier.trackingTable so that it's set to the default (non-empty) value
2. go into about:url-classifier and trigger a list update for the "mozilla" provider
3. go to https://itisatrap.org/firefox/its-a-tracker.html and everything should be green (with the shield icon show it)

Comment 10

[rdz_bug]

(In reply to François Marier [:francois] from comment #9)
> (In reply to Rodze from comment #3)
> > Some quick test showed urlclassifier.trackingTable is set to blank. 
> 
> That's why tracking protection doesn't work.
> 
> To make it work:
> 
> 1. go into about:config and reset the value of urlclassifier.trackingTable
> so that it's set to the default (non-empty) value
> 2. go into about:url-classifier and trigger a list update for the "mozilla"
> provider
> 3. go to https://itisatrap.org/firefox/its-a-tracker.html and everything
> should be green (with the shield icon show it), 

The whole problem is the configuration are lost after browser restart, as explained in the bug filing.

Comment 11

[James Willcox (:snorp) ([e-mail]) (he/him)]

Eugen, this seems related to your recent work.

Comment 12

[Eugen Sawin [:esawin]]

The empty tracking table config is bug 1465877, should be fixed now.
I can look into the cookies setting issue if that's reproducing.

Comment 13

[Eugen Sawin [:esawin]]

Sorry, I meant bug 1466511.

Comment 14

[Vlad Baicu]

I can confirm that the tracking protection works, for me at least

Comment 15

[J.D.M]

(In reply to Vlad Baicu from comment #14)
> I can confirm that the tracking protection works, for me at least

Yeah, works fine now.

Comment 16

[Eugen Sawin [:esawin]]

The general issue is that in GeckoView we override default pref values in GeckoRuntimeSettings since we don't want any persistence between runtime sessions.
This is in conflict with Fennec's model that is based on persistent prefs.
I've discussed this issue a while back with Nicholas [:njn], but this hasn't been a high priority yet.

In the meanwhile, we can add an option to GeckoRuntimeSettings to not override prefs with GeckoView defaults on startup, which Fennec would use to avoid this issue.

Comment 17

[Eugen Sawin [:esawin]]

Vlad, I'm going to steal this one, since it's a regression from bug 1445420 and is best solved on the GeckoView side.

Comment 18

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1465877-1.0-Add-a-mechanic-to-test-whether-we-re.patch

I haven't worked on Fennec long enough to forget if there is a better way to check for this, do we need this?

Comment 19

[Eugen Sawin [:esawin]]

Attachment: 0002-Bug-1465877-2.0-Keep-track-of-modified-settings-to-p.patch

Let's keep track of modified settings to avoid overriding (persistent) prefs when not required.

Comment 20

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8988315 [details] [diff] [review]
0002-Bug-1465877-2.0-Keep-track-of-modified-settings-to-p.patch

Review of attachment 8988315 [details] [diff] [review]:
-----------------------------------------------------------------

Kinda gross, but probably about the best we can do right now.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc04076339d3
[1.0] Add a mechanic to test whether we're running GeckoView in Fennec environment. r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/a17a279fda0f
[2.0] Keep track of modified settings to prevent overriding prefs when not desired. r=snorp

Comment 23

[Chris Peterson [:cpeterson]]

We should uplift this fix to Beta 62.

Comment 24

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/dc04076339d3
https://hg.mozilla.org/mozilla-central/rev/a17a279fda0f

Comment 26

[Liz Henry (:lizzard) (relman/hg->git project)]

Eugen, can you request uplift to beta?
Bogdan, can you verify the fix in nightly? Thanks!

Comment 27

[Liz Henry (:lizzard) (relman/hg->git project)]

Ryan, n-i in case you want to consider this for a fennec 51 dot release.

Comment 28

[Liz Henry (:lizzard) (relman/hg->git project)]

I mean 61. We should make totally sure that it's really a regression from 61 though.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

It would be great if we could get confirmation that this indeed affects Fennec 61 currently on release.

Comment 30

[Jan Henning [:JanH]]

Bug 1468070 (which was marked as a duplicate of this one) was definitively caused by bug 1433968, which only affects Firefox 62 and onwards.

Comment 33

[Bogdan Surd, QA [:bsurd, NI]]

Devices:
 - Nexus 5 (Android 6.0.1);
 - OnePlus 5T (Andoid 8);
 - Pixel (Android 9)

Tested on the above-mentioned devices and the issue is no longer reproducible. No other settings seem to be affected by this change. Marking as verified.

Comment 34

[Liz Henry (:lizzard) (relman/hg->git project)]

Can you request uplift to 62 (since Eugen is out on PTO)?   

And, I wonder if you can help verify whether this affects 61 or not? We can also ask for help from QA.

Comment 35

[Liz Henry (:lizzard) (relman/hg->git project)]

I'm going to aim for getting this into the beta 5 build today, and won't block beta updates on the issue.

Comment 36

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

I can't reproduce duped bug #1472543 in Firefox 61. Now only Firefox 62 is affected.

Comment 37

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

FYI - This bug is regression from bug #1433968, not bug #1445420, as Jan Henning [:JanH] already mention this in comment #30.

Comment 38

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Forwarding uplift request to 62 to Snorp (comment 34) who is more appropriate to uplift because he's the reviewer.

Comment 39

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8988314 [details] [diff] [review]
0001-Bug-1465877-1.0-Add-a-mechanic-to-test-whether-we-re.patch

Approval Request Comment
[Feature/Bug causing the regression]: 1445420
[User impact if declined]: Some Fennec features won't work, such as tracking protection and debug support.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Yes. Ensure toggling tracking protection in settings has the intended effect.
[List of other uplifts needed for the feature/fix]: Both patchs on this bug
[Is the change risky?]: No
[Why is the change risky/not risky?]: Tested in Nightly, can't break it anymore than it already is :)
[String changes made/needed]: None

Comment 40

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8988314 [details] [diff] [review]
0001-Bug-1465877-1.0-Add-a-mechanic-to-test-whether-we-re.patch

Verified in nightly, very convincing risk assessment, please uplift for beta 6

Comment 41

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/e0394cc8f42d
https://hg.mozilla.org/releases/mozilla-beta/rev/5a6eba66a378

Comment 42

[Oana Horvath [:ohorvath]]

Verified on Beta 62.0b7. The Cookies toggle and TP are working as expected.
Device: Samsung Galaxy Tab 3 (Android 7.0)