Closed Bug 1064951 Opened 5 years ago Closed 5 years ago

Default FHR pref to 'off' in 'Firefox Privacy Coach' add-on

Categories

(Firefox for Android :: General, defect)

35 Branch
All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
fennec + ---

People

(Reporter: krudnitski, Assigned: Margaret)

References

Details

Default the FHR pref 'off' for the 'Firefox Confidential' add-on
tracking-fennec: ? → 33+
tracking-fennec: 33+ → +
Summary: Default FHR pref to 'off' in 'Firefox Confidential' add-on → Default FHR pref to 'off' in 'Firefox Privacy Coach' add-on
rnewman, is there any good way to do this from an add-on? 

The solution for this and bug 1064954 will probably look the same, but unfortunately we'll need a way to fix this bug without shipping a patch in the product.
Assignee: nobody → margaret.leibovic
Flags: needinfo?(rnewman)
You would need to use JNI to set the SharedPreference value android.not_a_preference.healthreport.uploadEnabled to false.
Status: NEW → ASSIGNED
Flags: needinfo?(rnewman)
(In reply to Richard Newman [:rnewman] from comment #2)
> You would need to use JNI to set the SharedPreference value
> android.not_a_preference.healthreport.uploadEnabled to false.

Setting the SharedPreference would flip it? In that case, could I just use SharedPreferences.jsm?
Flags: needinfo?(rnewman)
Yes.

Make sure you're writing to the correct pref file (should be obvious during testing -- see if your JS code changes the checkbox in Settings!).

It should be per-profile prefs, but I don't remember for sure.


Strictly speaking we should also call

  GeckoPreferences.broadcastHealthReportUploadPref(context, false);

which is what happens if you uncheck the box in Settings. If you can't do it, it's possible that an upload will still occur, because the service hasn't been told about the change.

Doing that broadcast using JNI should be easy.
Flags: needinfo?(rnewman)
(In reply to Richard Newman [:rnewman] from comment #4)
> Yes.
> 
> Make sure you're writing to the correct pref file (should be obvious during
> testing -- see if your JS code changes the checkbox in Settings!).
> 
> It should be per-profile prefs, but I don't remember for sure.

Looks like it's per-app:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/preferences/GeckoPreferences.java#918

Using SharedPreferences.forApp() is working as expected.

> Strictly speaking we should also call
> 
>   GeckoPreferences.broadcastHealthReportUploadPref(context, false);
> 
> which is what happens if you uncheck the box in Settings. If you can't do
> it, it's possible that an upload will still occur, because the service
> hasn't been told about the change.
> 
> Doing that broadcast using JNI should be easy.

This was slightly tricky for the stumbler case, since that's only available in Firefox 35+, but I just wrapped that call in a version check.

I also wanted to use the JNI.jsm that's in the tree, but it only landed in Fx34, so I copied a version of it into the add-on.

I didn't do a good job making organized commits, but the logic that handles all this has landed here:
https://github.com/leibovic/privacy-coach/blob/master/bootstrap.js

rnewman, do you just want to take a look at this code to do a quick review? So far nobody has looked at any of it, and I feel like adding some JNI is a good time to have someone look at it :D
Flags: needinfo?(rnewman)
Add-on review comments:

* License blocks.
* Maybe take a conditional approach to importing JNI.jsm:

  let scope = {};
  Cu.import(".../JNI.jsm", scope);
  if ("whatever we expect" in scope.JNI) {
    // Hoist!
    var JNI = scope.JNI;
  } else {
    // We need to use our own.
    Cu.import("resource://...");
  }

The point of this would be to take advantages of JNI.jsm changes in *future* releases of Firefox.

* let not var, use strict. (I don't *think* you need to specify a JS source version in an add-on, but if you do, please do!)

* You restore old pref values when you uninstall the add-on. Is that what the user would expect?

Other than that, this looks good to me!
Flags: needinfo?(rnewman)
(In reply to Richard Newman [:rnewman] from comment #6)

> * You restore old pref values when you uninstall the add-on. Is that what
> the user would expect?

I was operating under the assumption that uninstalling an add-on should undo whatever it did from installing it. I didn't want to just totally reset the prefs to their default state, in case the user made some change *before* installing the add-on.

However, this system isn't perfect if the user manually toggles some of these settings after the add-on has been installed.

We may change the add-on to inform users about how to change these prefs, rather than automatically changing them, in which case this wouldn't be as much of a problem. We can address this in a different bug if we decide to change it.

I addressed the review comment (thanks!) so I'm going to close this bug.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.