1007264 - Opt-in message is not clear that stumbling happens when Fennec is not running

Status: RESOLVED FIXED

(Android Background Services Graveyard :: Geolocation, defect)

Description

[Guilherme Gonçalves [:ggp]]

Do we want to have Fennec passively collecting cell, location and wifi data while it is not running?

For example, say the user has opted-in to data collection, then reboots their phone, so Fennec is decidedly not running unless the user starts it again. Should we still be listening passively for locations after the reboot, but before Fennec is opened for the next time?

While that behavior is technically feasible, it could be considered invasive by the user. Note that our current opt-in text:

"Receives Wi-Fi and cellular location data when running in the background and
shares it with &vendorShortName; to improve our geolocation service"

specifies that data collection will happen while Fennec is running in the background, but says nothing about when it is not running at all. So in my opinion, if we are to scan passively at all times, we should at least consider updating the opt-in text to make that clear.

Comment 1

[:garvan]

Relates to but not dependent on: Bug 1003609

Comment 2

[:garvan]

I think that the privacy issue is addressed in bug 1003609, if we could put privacy-related issues there. Unless there is a reason that the privacy issues are different in the case the Fennec is not running vs. backgrounded.

Another issue is the impact on the battery, which have to determine immediately. We need to take an approach that ensures absolute minimal battery impact. But again, maybe this should be a bug on its own, since it is an issue whether Fennec is backgrounded, or we are collecting when Fennec is not running.

Comment 3

[Chris Peterson [:cpeterson]]

Technically, Android does not consider an Activity to be "running" when it is not the foreground Activity. When the user switches from Fennec to a different app, we can't assume that the Fennec process will stay alive in the background. Fennec uses a lot of memory, so Android is likely to kill Fennec before any other background apps. So only collecting location data "while Fennec is running" might be a very short window of opportunity.
 
I don't think users would (be able to) make such a subtle distinction between Fennec as a running foreground app, a paused background app, a background app that was killed by memory pressure, and having Fennec installed but not launched since last rebooting their phone. I think trying to communicate that they must open Fennec after rebooting their phone (as a "double secret opt-in" step) would be unnecessarily confusing. If the user doesn't want Fennec to report their location, they should revert their opt-in setting.

Fennec uses a lot of memory, so Android is likely to kill Fennec before any other background apps. So only collecting location data "while Fennec is running" might be a very short window of opportunity.

Comment 4

[Guilherme Gonçalves [:ggp]]

(In reply to Chris Peterson (:cpeterson) from comment #3)  
> I don't think users would (be able to) make such a subtle distinction
> between Fennec as a running foreground app, a paused background app, a
> background app that was killed by memory pressure, and having Fennec
> installed but not launched since last rebooting their phone.

This is a very good point, but I worry that Android users may have _some_
simpler notion of "running" vs. "not running" (conveyed by the system's UI),
and that our opt-in text may not reflect that notion.

Personally, I think considering that an app is "not running" before it is
launched is not such a subtle distinction, but I admittedly know nothing
about the expectations of most of our users, so I can absolutely be convinced
otherwise.

Ultimately, of course, the goal is not to force the user to distinguish between
very technical and subtle cases, but rather to make sure our collection policy
and opt-in text match whatever simpler notion of "running" most users would have.

Comment 5

[Chris Peterson [:cpeterson]]

As you suggested earlier, I think we can avoid this ambiguity by just removing the phrase "when running" from our opt-in test. Your other concern was about the phrase "Receiving ... data", which we could change to something like "Collect".

So the current text:

> "Receives Wi-Fi and cellular location data when running in the background and
> shares it with &vendorShortName; to improve our geolocation service"

could become something like:

> "Collects Wi-Fi and cellular location data and
> shares it with &vendorShortName; to improve our geolocation service"

Comment 6

[Mika]

Is this bug referring to this notice: https://bug903535.bugzilla.mozilla.org/attachment.cgi?id=8403028

If yes, a few questions:
(1) What is the concern between the word "receives" and "collects"?

(2) Is the business decision to change the collection so that, for users who opt-into reporting location on their Android devices, Mozilla will receive location data regardless of whether Firefox is active or terminated? 
- If yes, I will change the language to reflect this.  However, wouldn't many people choose not to opt-into this collection because of the batter drain issue? 

(3) I don't know how many people opted into reporting location, but if it's a small number, it seems likely that those are the same people who also have MozStumbler already running. Is this a double instance of reporting that makes the battery drain worse? If so, seems like not much benefit to extending the Fennec data collection.

Comment 7

[:garvan]

>  However, wouldn't
> many people choose not to opt-into this collection because of the batter
> drain issue? 

The implementation should have little measurable power impact. We use passive listening to location, so if another app requests a GPS location, we get notified, and get that location. 

> (3) I don't know how many people opted into reporting location, but if it's
> a small number, it seems likely that those are the same people who also have
> MozStumbler already running. Is this a double instance of reporting that
> makes the battery drain worse? If so, seems like not much benefit to
> extending the Fennec data collection.

See above regarding power impact. Hopefully with a better UX (bug 1027078), we can sell folks the idea there is a no-cost way to contribute to an open location database, just click the ok button and we will do the rest in the background.

Comment 8

[Guilherme Gonçalves [:ggp]]

(In reply to Mika from comment #6)
> (1) What is the concern between the word "receives" and "collects"?

I think "receives" feels too passive to accurately describe what we're doing, and suggests a couple of questions that are left unanswered -- "receives" from whom? What can I do to stop whoever is "sending" the location data from doing so? "Collects" sounds better to me because it clearly states that we're actively gathering location data, which will otherwise not be shared with anyone if the user doesn't opt-in.

Of course, this distinction comes from my personal interpretation of the text, so if it looks inconsequential to you, feel free to stick with "receives".

Comment 9

[Chris Peterson [:cpeterson]]

Mika: what are the next steps here?

The current text is:

"Receives Wi-Fi and cellular location data when running in the background and shares it with Mozilla to improve our geolocation service"

If we replace the word "receives" with "collects" and remove the phrase "when running" to (perhaps too implicitly?) allow background data collection, would the following text be acceptable?

"Collects Wi-Fi and cellular location data and shares it with Mozilla to improve our geolocation service"

Comment 10

[Mika]

Does this language address the concerns:

The title is: Mozilla Location Service
The old text is here: http://screencast.com/t/dNVtbCxlMf

The new text would read: "Receives Wi-Fi and cellular location data based on the approximate location of your device to improve Mozilla's geolocation service"

I prefer to use the word "receive" instead of "collect" because Mozilla receives data when users enable the option.  This is how we describe our data practice in several other notices.

Comment 11

[Chris Peterson [:cpeterson]]

I see how "receive" is more technically accurate than "collect", but your proposed text does not mention that Fennec will upload this data to Mozilla.

Copying the "Telemetry" setting's description in your screenshot, how about something like the following?

"Shares Wi-Fi and cellular location data based on the approximate location of your device to improve Mozilla's geolocation service"

Comment 12

[Mika]

Since it's called "Mozilla Location Service" I think it's clear that the data is received by Mozilla.  But if you want to be more clear, here's a slight tweak of your proposal:

"Shares Wi-Fi and cellular location data based on the approximate location of your device with Mozilla to improve our geolocation service"

Comment 14

[Chris Peterson [:cpeterson]]

I like it. :)

Now we just need Garvan's feedback.

Comment 15

[Mika]

Yay!

Comment 16

[:garvan]

looks good, thanks

Comment 17

[:garvan]

Attachment: bug1007264-optin-wording.patch

Oops, almost forgot to update this message. Here is the patch.
Nick can you suggest what the process is for changing strings? Who reviews, who does it go to next for localization, those types of things.

Comment 18

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8499733 [details] [diff] [review]
bug1007264-optin-wording.patch

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

As for l10n review, if you have the sign-off from legal, no further code review necessary; but I'll flag Jeff since this is Android specific and legal-sensitive.

::: mobile/android/base/locales/en-US/android_strings.dtd
@@ +265,5 @@
>  <!-- Localization note (datareporting_crashreporter_title_short) : This string matches
>       (crashReporterSection.label) in en-US/chrome/browser/preferences/advanced.dtd.-->
>  <!ENTITY datareporting_crashreporter_title_short "Crash Reporter">
>  <!ENTITY datareporting_wifi_title2 "&vendorShortName; Location Service">
> +<!ENTITY datareporting_wifi_geolocation_summary "Shares Wi-Fi and cellular location data based on the approximate location of your device with &vendorShortName; to improve our geolocation service">

You must rename the entity (so make it ..._summary2, like the one above) when you change a string.  You'll need to update strings.xml.in to refer to it as well.

nit: geolocation service*s* -- we may have more than one, right?

Comment 19

[Jeff Beatty [:gueroJeff]]

Comment on attachment 8499733 [details] [diff] [review]
bug1007264-optin-wording.patch

Is this being uplifted? If not, the change will hit Aurora in a week and will be available for all official locales to translate.

I don't think this requires any additional explanation in the localization note or in the entity beyond changing it to summary2, as what was already mentioned. Looks good to me.

Comment 20

[:garvan]

(In reply to Nick Alexander :nalexander from comment #18)
> Comment on attachment 8499733 [details] [diff] [review]
> bug1007264-optin-wording.patch
> 
> Review of attachment 8499733 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> As for l10n review, if you have the sign-off from legal, no further code
> review necessary; but I'll flag Jeff since this is Android specific and
> legal-sensitive.
> 
> ::: mobile/android/base/locales/en-US/android_strings.dtd
> @@ +265,5 @@
> >  <!-- Localization note (datareporting_crashreporter_title_short) : This string matches
> >       (crashReporterSection.label) in en-US/chrome/browser/preferences/advanced.dtd.-->
> >  <!ENTITY datareporting_crashreporter_title_short "Crash Reporter">
> >  <!ENTITY datareporting_wifi_title2 "&vendorShortName; Location Service">
> > +<!ENTITY datareporting_wifi_geolocation_summary "Shares Wi-Fi and cellular location data based on the approximate location of your device with &vendorShortName; to improve our geolocation service">
> 
> You must rename the entity (so make it ..._summary2, like the one above)
> when you change a string.  You'll need to update strings.xml.in to refer to
> it as well.
> 
> nit: geolocation service*s* -- we may have more than one, right?

Just one service that I can think of (it does have both a submission and query interface).

Comment 21

[:garvan]

Attachment: bug1007264-optin-wording.patch

Thanks for the review Nick, hopefully I did this correctly:

- Changed to datareporting_wifi_geolocation_summary2
- updated mobile/android/base/strings.xml.in to match

Comment 22

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8499779 [details] [diff] [review]
bug1007264-optin-wording.patch

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

Yep, lgtm.

Comment 23

[:garvan]

> Is this being uplifted? 

Just for landing in Nightly and following the train. This wording change is to correspond with how data collection has already changed in Nightly (35).

Comment 24

[Jeff Beatty [:gueroJeff]]

(In reply to Garvan Keeley [:garvank] from comment #23)
> > Is this being uplifted? 
> 
> Just for landing in Nightly and following the train. This wording change is
> to correspond with how data collection has already changed in Nightly (35).

Sounds great!

Comment 25

[:garvan]

running a try+tests before checkin needed

Comment 26

[:garvan]

try green https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=6bbe9bf3c7d5

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/29fcac263c20

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Backed out for the robocop failures noted in bug 1061991 comment 7. Which were also visible on the Try run. Please verify that your Try pushes are green before requesting checkin in the future.
https://hg.mozilla.org/integration/fx-team/rev/d99a7f64432a

Comment 29

[:garvan]

Attachment: bug1007264-optin-wording.patch

Crap. Sorry. Friday night impatience. 
Actually passes robocop#4 -for real- on both platforms:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=7ca831b0c2ca

Comment 30

[:garvan]

Comment on attachment 8500803 [details] [diff] [review]
bug1007264-optin-wording.patch

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

::: mobile/android/base/tests/testSettingsMenuItems.java
@@ +171,5 @@
>              }
>  
>              if (AppConstants.MOZ_STUMBLER_BUILD_TIME_ENABLED) {
>                  // Anonymous cell tower/wifi collection
> +                String[] networkReportingUi = { "Mozilla Location Service", "Shares Wi-Fi and cellular location data based on the approximate location of your device with Mozilla to improve our geolocation service" };

This was not updated in the first patch, hence Robocop test failure.

Comment 31

[Richard Newman [:rnewman]]

Comment on attachment 8500803 [details] [diff] [review]
bug1007264-optin-wording.patch

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

r+ for the test fix, and a suggestion for a smoother sentence.

::: mobile/android/base/locales/en-US/android_strings.dtd
@@ +265,5 @@
>  <!-- Localization note (datareporting_crashreporter_title_short) : This string matches
>       (crashReporterSection.label) in en-US/chrome/browser/preferences/advanced.dtd.-->
>  <!ENTITY datareporting_crashreporter_title_short "Crash Reporter">
>  <!ENTITY datareporting_wifi_title2 "&vendorShortName; Location Service">
> +<!ENTITY datareporting_wifi_geolocation_summary2 "Shares Wi-Fi and cellular location data based on the approximate location of your device with &vendorShortName; to improve our geolocation service">

This is a pretty tortured sentence. How about

  Shares approximate Wi-Fi and cellular location data with &vendorShortName; to improve our geolocation service

::: mobile/android/base/tests/testSettingsMenuItems.java
@@ +171,5 @@
>              }
>  
>              if (AppConstants.MOZ_STUMBLER_BUILD_TIME_ENABLED) {
>                  // Anonymous cell tower/wifi collection
> +                String[] networkReportingUi = { "Mozilla Location Service", "Shares Wi-Fi and cellular location data based on the approximate location of your device with Mozilla to improve our geolocation service" };

This should really use string substitution, but oh well.

Comment 32

[:garvan]

> How about:
> 
>   Shares approximate Wi-Fi and cellular location data with &vendorShortName;
> to improve our geolocation service

More concise, which is good for android settings view since the text wraps heavily and occupies a lot of space in the view (and there is a lot of items battling for space in the prefs view already).

Mika is this ok instead?

Comment 33

[Chris Peterson [:cpeterson]]

(In reply to Garvan Keeley [:garvank] from comment #32)
> > How about:
> > 
> >   Shares approximate Wi-Fi and cellular location data with &vendorShortName;
> > to improve our geolocation service

LGTM, though we lose the "of your device" phrase, which might be important for explaining the privacy issue.

Comment 34

[:garvan]

And can we go even shorter, removing the words 'approximate' and 'data':

Shares Wi-Fi and cellular location with &vendorShortName; to improve our geolocation service

Comment 35

[:garvan]

I haven't been able to get ahold of Mika, is there a plan B? I need to get this landed tomorrow. 

The short and sweet looks great on-screen:
https://www.dropbox.com/s/x8f5s6yhydsgj7e/Screenshot_2014-10-09-22-38-16.png?dl=0

Comment 36

[Chris Peterson [:cpeterson]]

LGTM. Let's land this and we can tweak the text later, if needed.

One last bikeshed comment: maybe we should include the word "your" to emphasize the privacy aspect. The FHR text in your screenshot uses the word "your".

-"Shares Wi-Fi and cellular location with &vendorShortName; to improve our geolocation service"
+"Shares your Wi-Fi and cellular location with &vendorShortName; to improve our geolocation service"

Comment 37

[Mika]

This is final. Please use this:
"Shares approximate Wi-Fi and cellular location of your device with &vendorShortName; to improve our geolocation service"

Comment 38

[:garvan]

Attachment: bug1007264-optin-wording.patch

This is with Mika's latest wording:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=e62514f06659

Comment 39

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/4978e73d4444

Comment 40

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4978e73d4444