We need to revise the in product permission in the "Choose what to share" alert.

We also need to add build flags around the Settings UI for this feature. It should not be visible in Beta ro Release, but that is a separate bug IIRC. Since this is a strings bug, it's not for Fx25 which is Aurora.

This patch removes building of the wifi/cell tower UI if we are on the release builds.

With respect to non-UI code in, are we going to just leave that in release builds, or do we also want to wrap those in build flags? If we want to build them conditionally, we'll want to another build flag (MOZ_GEO_REPORTING perhaps) and add that to confvars.sh and configure.in, the way the other datareporting prefs are handled. At the moment, wifi/cell tower is wrapped in MOZ_DATA_REPORTING, but since it's now conditionally built, we might want to change that.

(In reply to Chenxia Liu [:liuche] from comment #3) > With respect to non-UI code in, are we going to just leave that in release > builds, or do we also want to wrap those in build flags? If we want to build > them conditionally, we'll want to another build flag (MOZ_GEO_REPORTING > perhaps) and add that to confvars.sh and configure.in, the way the other > datareporting prefs are handled. At the moment, wifi/cell tower is wrapped > in MOZ_DATA_REPORTING, but since it's now conditionally built, we might want > to change that. For now we can keep the code. If we find an issue with it later, we can file a new bug to add a flag around the code too.

Comment on attachment 795713 [details] [diff] [review] Patch: Don't build UI for release builds + tests Review of attachment 795713 [details] [diff] [review]: ----------------------------------------------------------------- Just the one issue, but it's an important one! ::: mobile/android/base/tests/testSettingsMenuItems.java.in @@ +169,5 @@ > > + // Anonymous cell tower/wifi collection; dependent on RELEASE_BUILD > + Field releaseBuildField = appConstants.getField("RELEASE_BUILD"); > + boolean releaseBuild = releaseBuildField.getBoolean(appConstants); > + if (releaseBuild) { Shouldn't this be "if (!releaseBuild)"? Or am I confused?

You're totally right, Geoff, thanks for catching that. Updated the patch, try push here: https://tbpl.mozilla.org/?tree=Try&rev=f70b1c3b2820

Chris, just poking you about strings for this so we can make sure to get this into 26.

(In reply to Chenxia Liu [:liuche] from comment #8) > Chris, just poking you about strings for this so we can make sure to get > this into 26. Thanks for the reminder. These strings are on my list.

Comment on attachment 796166 [details] [diff] [review] Patch: Don't build UI for release builds + tests v2 Moving this patch over to bug 909938, since this should be tracking string changes only.

Merge is in about a week - any strings that we want? Also, changing the tracking flags to "?" because string changes means this is for 26, the current Nightly, and shouldn't be tracked for 25.

We won't have the new strings ready before the uplift, so I don't think we need to track this bug. The data collection server also changed its data format, so we need to update Fennec's collection code too.

Should we also untrack for 26 in that case? bug 909938 seems to have us covered

(In reply to Alex Keybl [:akeybl] from comment #13) > Should we also untrack for 26 in that case? bug 909938 seems to have us > covered Yes, we don't need to track this for Fx 26.

Unassigning myself for now until we get some movement.

Neeinfo Liu to understand what the next steps here are for Firefox 27 ? Do we need to do anything here ? Or is 909938 covering us on Firefox27.

Bhavana: I don't think there is anything we need to do for Firefox 27. luiche is waiting for new text from me (and I'm working with Legal). I'm assigning this bug to me to show that this bug is blocked on me.

This is the screenshot for the geo collection text. The strings are not entirely within our Android style, which encourages shorter, user-friendly strings; no multi-sentence things. Ian, any thoughts on this string? These strings have some legal implications, so some feedback from you would be great.

I will be on PTO for two weeks, so unassigning myself from this bug for now. The patch up should be easy to change for whatever new string we end up deciding to use.

I agree with Chenxia. The strings feel too long. Can we use shorter strings and perhaps use a "Learn more" style UI to send people to a in-content based description. That could be a simple chrome:// URI. It doesn't need to be web-based.

(In reply to Chenxia Liu [:liuche] from comment #19) > Created attachment 8334072 [details] > Screenshot: geo strings > > This is the screenshot for the geo collection text. > > The strings are not entirely within our Android style, which encourages > shorter, user-friendly strings; no multi-sentence things. > > Ian, any thoughts on this string? These strings have some legal > implications, so some feedback from you would be great. Sorry it took so long to reply to this. The string is definitely way too long, and inconsistent with the way we phrase our other prefs in this section. Is it possible to use a slightly modified version of the current string we have, with a "learn more" link to a more lengthy description that includes some of the details in Chenxia's screenshot? Could be a website, another page in Settings, a popup... So it would read: --------------------------------------------------------------------------------- Mozilla location services Help improve geolocation services for the Open Web by letting Firefox collect and send anonymous Wi-Fi and cellular tower data --------------------------------------------------------------------------------- Learn more > --------------------------------------------------------------------------------- Jishnu, would capturing the additional details (regarding service providers, data storage and background data collection) in a secondary page still satisfy the legal requirements for this pref?

Thank you, ibarlow! Jishnu, can you take a gander at the above proposed text?

Mika will be taking over the Fennec text from Jishnu.

I have a two-part recommendation for this: Part 1 - updated Fennec Text "Mozilla location services Collects Wi-Fi and cellular location data from users of Firefox on Android (when running in background) and shares with Mozilla to improve our geolocation service. Learn More" (temporarily redirects to: https://location.services.mozilla.com/) Part 2 - Privacy Notice I'll give some more thought to whether the "Learn More" should redirect to a separate geolocation specific Privacy Notice that's posted on the MLS page, or an existing Privacy Notice (to which we add a section on Geolocation). I'll get back to you on this piece. LMK if you have questions. Mika

(In reply to Mika from comment #25) > I have a two-part recommendation for this: > > Part 1 - updated Fennec Text > "Mozilla location services > Collects Wi-Fi and cellular location data from users of Firefox on Android > (when running in background) and shares with Mozilla to improve our > geolocation service. Learn More" (temporarily redirects to: > https://location.services.mozilla.com/) Can we clip the "from users of Firefox on Android" part? It's extraneous, since people are already reading this from within Firefox for Android. So it would read: --------------------------------------------------------------------------------- Mozilla location services Collects Wi-Fi and cellular location data when running in the background, and shares it with Mozilla to improve our geolocation service. --------------------------------------------------------------------------------- Learn more > ---------------------------------------------------------------------------------

(In reply to Ian Barlow (:ibarlow) from comment #26) > Can we clip the "from users of Firefox on Android" part? It's extraneous, > since people are already reading this from within Firefox for Android. > > So it would read: > > ----------------------------------------------------------------------------- > ---- > Mozilla location services > Collects Wi-Fi and cellular location data when running in the background, > and shares it with Mozilla to improve our geolocation service. > ----------------------------------------------------------------------------- > ---- > Learn more > > ----------------------------------------------------------------------------- > ---- If this is only seen by users on Android, then yes, please clip. If desktop users also see the notice, then keep it so that desktop folks know that it's not applicable. Thanks!

Ian: are you OK with the text you proposed in comment 26? If so, Mika says engineering can proceed to enable the Wi-Fi data collection feature and UI.

Ship it

Chenxia: will you have time to update these UI strings for Nightly 29 or 30?

Since we're using "Learn more" in multiple places now, I also renamed the locale dtd resource (that was previously only used by remote debugging).

Ian, take a look at the screenshot and let me know if it looks good, or if there's anything that you want changed.

(In reply to Chenxia Liu [:liuche] from comment #33) > Ian, take a look at the screenshot and let me know if it looks good, or if > there's anything that you want changed. Looks good!

Forgot to check for RELEASE build flag.

I've been working on some other stuff, but since this breaks a test that I'd like to fix correctly and we're at the end of a cycle, I'll plan to land this after the merge.

Chenxia: Mika/Legal has requested these text changes: > - <!ENTITY datareporting_wifi_title "&vendorShortName; location services"> > + <!ENTITY datareporting_wifi_title "&vendorShortName; Location Service"> > > - <!ENTITY datareporting_wifi_geolocation_summary "Collects Wi-Fi and cellular location data when running in the background, and shares it with &vendorShortName; to improve our geolocation services"> > + <!ENTITY datareporting_wifi_geolocation_summary "Receives Wi-Fi and cellular location data and shares it with &vendorShortName; and its partners to improve our geolocation services">

Chenxia: do you have an ETA for landing the geolocation text changes (including the changes from comment 38) in Nightly 30?

Sorry for the delay! This got moved to the back burner - I'll be sure to get to it today or tomorrow though.

Updated strings, carrying over r+ because only string changes.

Tests used to fail on scrolling in Preferences, moved the scroll code to BaseTest so other tests that use Preferences can use it. Try push: https://tbpl.mozilla.org/?tree=Try&rev=b9bb17c15b01

Wrong patch queue.

Just running these strings by you, Ian.

The "and its partners" bit makes me feel uneasy. It's ambiguous and sounds kind of sketchy in a 'who else exactly are you sharing this with' kind of way. Are we explaining this in more detail in the "learn more" page? If we are, I guess it's fine. If we aren't, we should fix that. Also please add a comma after "cellular location data"

(In reply to Ian Barlow (:ibarlow) from comment #45) > The "and its partners" bit makes me feel uneasy. It's ambiguous and sounds > kind of sketchy in a 'who else exactly are you sharing this with' kind of > way. Are we explaining this in more detail in the "learn more" page? If we > are, I guess it's fine. If we aren't, we should fix that. Mika: Do we need to add more information in our "Learn More" page about possible partnerships?

I think we explain this more in the privacy policy currently found at https://location.services.mozilla.com/privacy (and linked from the "learn more" page). The "When do we share your information with others?" section covers this in more detail.

(In reply to Hanno Schlichting [:hannosch] from comment #47) > I think we explain this more in the privacy policy currently found at > https://location.services.mozilla.com/privacy (and linked from the "learn > more" page). The "When do we share your information with others?" section > covers this in more detail. Hm, not that I can see... I am seeing information about when we share, but there is no mention of partners or any specific information around that. This is important, because the way this additional line reads right now implies that we are handing this off (or perhaps even selling it) to some unknown third party, so we really need to either a. clarify that, or b. remove that bit of text.

I just want to elaborate here a bit, in case that last comment was unclear. The point I'm trying to make is, if we are saying we have partners who will use this data (either now or some time in the future), I would like to see an explicit "Partners" section as part of the page that is linked from "learn more", that offers at least a sentence or two one what exactly that means. I'm not suggesting that we need an actual list of partners, but just some clarification and assurance to the user that we are using their data in good faith, that we aren't selling it, and so on.

Comment on attachment 8386430 [details] [diff] [review] Part 2: tests Review of attachment 8386430 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/tests/BaseTest.java @@ +72,5 @@ > public Device mDevice; > protected DatabaseHelper mDatabaseHelper; > protected StringHelper mStringHelper; > + protected int screenMidWidth; > + protected int screenMidHeight; Having mixed mX fields with the new convention is kind of gross. Can you either create a part 0 to rename the existing mX fields, or have the new ones just follow the old mX convention until someone gets around to changing them all? @@ +423,5 @@ > + // Hacky way to scroll down. solo.scroll* does not work in dialogs. > + MotionEventHelper meh = new MotionEventHelper(getInstrumentation(), mDriver.getGeckoLeft(), mDriver.getGeckoTop()); > + meh.dragSync(screenMidWidth, screenMidHeight+100, screenMidWidth, screenMidHeight-100); > + > + foundText = mSolo.waitForText(txt); Should this be in a loop? What if the text is more than one scroll-length down?

Comment on attachment 8386430 [details] [diff] [review] Part 2: tests Review of attachment 8386430 [details] [diff] [review]: ----------------------------------------------------------------- Marking r- until questions/comments from comment 50 are addressed.

Updated the naming. I went through the robotium API's and there doesn't seem to be a good way to figure out if you've scrolled to the bottom. I added a comment that Settings items should be added to tests in order. I'd also like to wait to land this until we hear back about the documentation clarification - I also feel like the strings sound a little unreassuring about how Mozilla is using user data, and could see this being upsetting to users.

We are working on an updated privacy notice. WIP at https://docs.google.com/a/mozilla.com/document/d/13OJ5zQTkEWHY6_2l0foetvP0VMt2XfPsTmeJN3sWG3A/edit (should be visible for all). Website should be updated some time next week.

Comment on attachment 8391464 [details] [diff] [review] Part 2: tests Review of attachment 8391464 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/tests/testSettingsMenuItems.java @@ +26,5 @@ > * These menu items are the ones that are always present - to test menu items that differ > * based on build (e.g., release vs. nightly), add the items in <code>addConditionalSettings</code>. > + * > + * Note: Due to the limitation of Robotium in detecting scroll position, preference items should be > + * listed in order, so that offscreen items can be found in a subsequent scroll. Nit: trailing whitespace

Ok - sorry this dragged on so long, and I should have left more leeway for discussion like this, but it looks like these strings are ready to land. I'd personally be more comfortable waiting for a clear explanation for "Mozilla and its partners" on our ToS, just so there isn't any confusion or misunderstanding. Does that sound reasonable? I don't see a partners section in the privacy notice yet - do these strings absolutely need to be in 30, or can we take the time to get all these details ironed out?

(In reply to Chenxia Liu [:liuche] from comment #55) > Ok - sorry this dragged on so long, and I should have left more leeway for > discussion like this, but it looks like these strings are ready to land. I'd > personally be more comfortable waiting for a clear explanation for "Mozilla > and its partners" on our ToS, just so there isn't any confusion or > misunderstanding. Does that sound reasonable? I don't see a partners section > in the privacy notice yet - do these strings absolutely need to be in 30, or > can we take the time to get all these details ironed out? No problem. These strings do not absolutely need to be in 30. I will follow up with Mika about the "Mozilla and its partners" text.

Any update on this?

I just reminded Mika and elan about revising this opt-in text.

Hi everyone, really sorry for not responding sooner. Just to recap, the proposed text says - Mozilla Location Service Receives Wi-Fi and cellular location data when running in background and shares it with Mozilla and its partners to improve our geolocation services. Learn More. Concern raised in bug - Who are these partners? how is the data being used? Answer - The intention was to be transparent but concise (because of space considerations) and give users notice that we might share their data with others. Currently, we don't share the data with anyone else. But we need to reserve the right to do that b/c in order to provide a better location service we might have to work with another entity. If and when this happens, we will name the entity in the link from "Learn More" and also link to their privacy policy. Suggestion - (1) leave the text as is. (2) change text to: "Receives Wi-Fi and cellular location data when running in background and shares it with Mozilla to improve our location service. Learn More." In #2, I removed the partner reference. The data is shared directly from Firefox to Mozilla. Right now, we don't share it. Only if Mozilla chooses to, will the data be subsequently shared with a partner. And this would be referenced in the full "Learn More" section as well as the MLS privacy notice (which is being written). So if it reduces confusion, my suggestion is to go with this option because it is accurate. Again, sorry for delayed response. Let me know if this explanation is helpful. Mika

Thanks Mika! (In reply to Mika from comment #59) > (2) change text to: "Receives Wi-Fi and cellular location data when running > in background and shares it with Mozilla to improve our location service. > Learn More." > > In #2, I removed the partner reference. The data is shared directly from > Firefox to Mozilla. Right now, we don't share it. Only if Mozilla chooses > to, will the data be subsequently shared with a partner. And this would be > referenced in the full "Learn More" section as well as the MLS privacy > notice (which is being written). So if it reduces confusion, my suggestion > is to go with this option because it is accurate. This would be my preference as well (#2). Just add the word "the" before "background" please :)

Okay, so the decision is to revise the text to the following: "Mozilla Location Service Receives Wi-Fi and cellular location data when running in the background and shares it with Mozilla to improve our location service. Learn More." I'll let someone else close the bug once it's implemented. Thanks, Mika

Screenshot - look good?

(In reply to Chenxia Liu [:liuche] from comment #63) > Screenshot - look good? shiiiiiip iiiiiiiit :)

https://hg.mozilla.org/integration/fx-team/rev/5cd3b8b0039b https://hg.mozilla.org/integration/fx-team/rev/a85917844058

https://hg.mozilla.org/mozilla-central/rev/5cd3b8b0039b https://hg.mozilla.org/mozilla-central/rev/a85917844058