Skip to content
This repository has been archived by the owner on Dec 3, 2020. It is now read-only.

Disable extraction and telemetry in PBM, with DNT and/or Tracking Protection on #72

Closed
1 task done
biancadanforth opened this issue Aug 27, 2018 · 19 comments
Closed
1 task done
Assignees
Milestone

Comments

@biancadanforth
Copy link
Collaborator

biancadanforth commented Aug 27, 2018

One challenge I have encountered with Shield studies in the past is if and how behavior should change when the user is in Private Browsing mode.

In my Shield experience, Private Browsing would be detected by the extension, and the user would not be prompted, nor would they see any study-related UI.

I acknowledge that we are not building a study here ultimately, so our extension may behave differently. It is also important to acknowledge that the desired behavior might change depending how the extension is deployed:

  • An extension as a temporary study (likely opt-out)
  • An extension shipped to users via Shield (likely opt-out)
  • An extension available on AMO (opt-in)
  • @javaun , I know we're still figuring out deployment options, but do you have any thoughts on this, and/or could you check with various folks on the telemetry/legal/privacy side if there are any guidelines?

Closing this issue would also include implementing any code changes necessary.

@biancadanforth biancadanforth added this to the November MVP milestone Aug 27, 2018
@biancadanforth
Copy link
Collaborator Author

@Osmose Osmose added the story label Sep 6, 2018
@Osmose Osmose removed this from the November MVP milestone Sep 6, 2018
@Osmose Osmose closed this as completed Sep 12, 2018
@Osmose Osmose reopened this Sep 12, 2018
@Osmose Osmose removed story labels Sep 12, 2018
@javaun
Copy link

javaun commented Sep 25, 2018

Great question, and thanks for raising it Bianca! We can talk options here and finalize a decision in our Friday meeting. ENG estimates (ease of implementation ) is a big consideration here.

My belief is we should disable Price Alerts in PBM. I think doing so would avoid a bunch of corner cases and confusing situations. All of these are assumptions that may not be correct...

  1. Polling (background price checking) should probably only happen in normal mode. Sites may change prices on users based on who they are, so if cookies/logins are not sent, the price alerts function may not work (should background polling only happen when a normal window is open?)
  2. Assumptions about how PBM behaves would be less complicated if Price Alerts didn't work there. Users may not understand that products added in PBM will also be available in normal mode (i.e. "wedding ring shopping")
  3. Background polling will also leak IP address, since it's a background call to a website. A user at a coffee shop only in PBM might assume their session is safer than it is
  4. Tracking protection is also active by default in PBM. That could be breaking other things in shopping workflows (Shopping is tracking-heavy)

Assuming these assumptions are correct and that disabling is something we want to do, what are our options? We could have a blank drop down that says "Shopping items cannot be added or viewed in Private Windows". Or, wee could have a "locked" group of items. What do you think Bianca? And are there any other arguments for making this work in PBM?

@Osmose
Copy link
Contributor

Osmose commented Sep 25, 2018

We could have a blank drop down that says "Shopping items cannot be added or viewed in Private Windows".

We can detect if a specific tab is in private browsing mode and stop extraction before it starts, effectively treating it as a page without a product. The track button will be disabled, but the user will still be able to open and view products in private browsing mode, which IMO is a reasonable thing to have.

@marniepw
Copy link
Collaborator

So, does this need to be in the November milestone? Sounds like there will be some work required, either way. Also, I talked to Paul from QA and he said that he didn't remember any TxP experiments working in PBM.

@Osmose Osmose added this to the November MVP milestone Sep 26, 2018
@Osmose
Copy link
Contributor

Osmose commented Sep 26, 2018

I think we can/should put this in the MVP.

@javaun What do you think about the suggested behavior of not performing extraction in PBM but still allowing access to the list?

@biancadanforth
Copy link
Collaborator Author

Per Chuck in slack:

I'm r+ on that.
Also no metrics reporting in PBM or with DNT or Tracking Protection on (a standard across all of Test Pilot)

@javaun
Copy link

javaun commented Sep 28, 2018

Seconded. This is great. And Osmose walked me through how this solves the Private Browsing concerns of not leaving a trace of the private window history. The background price update polling can still send cookies (so user sees the "logged in state" prices).

@biancadanforth biancadanforth changed the title Determine how the extension should behave in Private Browsing Disable extraction and telemetry in PBM, with DNT and/or Tracking Protection on Oct 10, 2018
@groovecoder
Copy link
Member

Thanks for raising this issue and including it in the MVP/November milestone! With Advance, we did set a precedent to prevent background/unexpected/invisible tracking even for opt-in Test Pilot experiments. They included verbiage like: "To respect your Privacy/DNT/Tracking Protection settings, Advance is disabled when Private Browsing and/or Tracking Protection are enabled."

As a side note, some interesting data could come from a study that compared price quotes fetched via regular browsing cookie vs. "fresh" (e.g., temporary/throw-away container) cookies. I wonder if tracked (by cookies) users get lower or higher prices for certain products.

@Osmose Osmose self-assigned this Oct 12, 2018
@Osmose
Copy link
Contributor

Osmose commented Oct 15, 2018

To summarize, I understand the following changes as part of this issue:

  • Do not perform extraction on tabs that are in a private browsing window.

  • Disable telemetry reporting when Content Blocking is enabled. This is the big "ON" switch seen here:

    screen shot 2018-10-15 at 10 53 21 am

  • Disable telemetry repoting when Do Not Track is set to always be sent:

    screen shot 2018-10-15 at 10 54 32 am


It's been brought up multiple times to "not send metrics for private browsing sessions". As far as I know there's no such thing as a "private browsing session", there's only private browsing windows. It's unclear to me what we want to do in various situations:

  • User has a normal browsing window and no private browsing windows open
  • User has a normal browsing window and a private browsing window open
  • User has no normal browsing windows and a private browsing window open

@groovecoder Can you elaborate on what you mean by a private browsing session, and how we detect it?

My current best guess for what we want to do is to disable collecting events for UI interactions in a private browsing window, e.g. clicking the toolbar icon in a private browsing window or any interactions in a panel launched from a private browsing window.

@groovecoder
Copy link
Member

groovecoder commented Oct 15, 2018

Do not perform extraction on tabs that are in a private browsing window.

Correct. You should also probably disable the browserAction completely for private browsing windows?

Disable telemetry reporting when Content Blocking is enabled. This is the big "ON" switch seen here.

That screenshot looks like it's from Nightly? (Or at least 63+?) The 62 UI looks like this:

screen shot 2018-10-15 at 1 35 13 pm

And the telemetry reporting and price polling should be disabled when either of those are set to "Always". (i.e., privacy.trackingprotection.enabled = true or privacy.donottrackheader.enabled = true)

In 63+ the Content Blocking switch will always be "On" because "Trackers: Only in Private Windows" will be checked. 63+ also adds the Content Blocking option for the user to check "Third-Party Cookies", so if you plan to study during 63+ time-frame, then you should also disable price extraction & polling when network.cookie.cookieBehavior = 4 ) So basically, the 3 prefs that should disable price polling across 62 and 63+:

  • privacy.trackingprotection.enabled = true
  • privacy.donottrackheader.enabled = true
  • network.cookie.cookieBehavior = 4

@biancadanforth
Copy link
Collaborator Author

It seems like the thinking on this has evolved quite a lot over time; I'm having difficulty figuring out what feature flags I should expect in different privacy scenarios to review Osmose's PR. I think it's gotten complicated enough that a table would help.

This is a boolean table indicating true if a feature should be enabled in a particular privacy context (my best guess from prior discussions):

Feature(down)/Context(across) Private Browsing Window Do Not Track Tracking Protection Reject Cookies
Background extraction (price polling) false false false false
Content extraction false true true true
Extension telemetry false false false false

@groovecoder , @Osmose , does this seem right? Particularly if the last row is wrong in any way, I'd like to know, so I can update METRICS.md. Maybe we should just get together in a room and hash this out.

@Osmose
Copy link
Contributor

Osmose commented Oct 16, 2018

Looks right to me. The extension telemetry in a private window bit is missing from my PR, I think.

@groovecoder
Copy link
Member

Yup, looks right to me. Thanks for putting it together that way - very nicely organized info.

@chuckharmston
Copy link

As @johngruen mentioned in Slack yesterday evening, the traditional Test Pilot practice to not send any usage information for users with DNT or Tracking Protection largely exists because Test Pilot experiments generally use Google Analytics to collect data. Since we're using event telemetry, that gives us a bit of leeway.

That said, we still do have a responsibility to be as lean as possible when collecting personal data of users regardless of their privacy settings. To that end, I chatted with @groovecoder this morning to try to work out the right place where we get the usage information we need to evaluate and iterate upon the experiment while still respecting users' privacy and wishes. Here's where I landed:

I do think we should omit any category 3 data (i.e. the aggregate counts of visits to sites we support) if DNT, Tracking Protection or cookie settings indicate a user's intent not to be tracked. I do think we should still collect basic extension interaction metrics in those cases. This is consistent with Firefox, which still collects cat 1 + 2 telemetry data from those users.

I propose amending the above table thusly:

Feature Private Browsing Do Not Track Tracking Protection Reject cookie
Price Polling false true* true* true*
Content Extraction** false true true true
Extension Telemetry (category 1-2) false true true true
Extension Telemetry (category 3) false false false false

* As additional consent to the polling, we should add information to the "no products" screen for these users to make sure that they understand that by adding an item to their list, we'll be checking prices on their behalf. @groovecoder may be able to help us with good wording here?

This all meets Luke's goal of making sure we don't make third-party requests for users with DNT/TP without their consent, and does some great proactive reinforcement of our organizational privacy stance.

Additionally, we should add to all pings a property mapping each of these properties (plus one aggregate one that is true if any of them are true):

"privacy": {
  "doNotTrack": [true | false],
  "trackingProtection": [true | false],
  "rejectCookies": [true | false],
  "any": [true | false]
}

This will help us segment analyses to understand if users who are privacy-conscious have differences in behaviors that may skew our results, which will help us be better stewards of this information in the future.

@Osmose
Copy link
Contributor

Osmose commented Oct 16, 2018

@chuckharmston Can we consider that change a post-MVP task and ship the more-restrictive set of changes for the MVP? That way we don't have to scramble to make these new changes, but also never ship anything less strict than what we're comfortable with.

If so, we should probably file your comment as a followup issue to this one.

@groovecoder
Copy link
Member

It seems fine to allow price polling for DNT, TP, and Reject Cookie users after informing users that adding products to their list will cause background requests to those sites.

Possible notice text:

"Note: Adding products will send price checks to the online shop"

Ideally, even update the first button text to say "Add this product anyway" to call the user's attention to the "Note:"

@Osmose
Copy link
Contributor

Osmose commented Oct 18, 2018

@chuckharmston Since this issue is closed, if you want us to make changes beyond what landed in the PR (reflected by @biancadanforth 's table with one small alteration), please file a new issue. Otherwise we won't be following up on it.

@biancadanforth
Copy link
Collaborator Author

Updated table to reflect the current state of privacy-related behavior changes upon merging this PR (the only difference is row 1 column 1 (Background extraction in Private Browsing windows) is true. You can see the discussion in the PR here. @groovecoder has agreed this difference is non-blocking and filed #177 .

Feature(down)/Context(across) Private Browsing Window Do Not Track Tracking Protection Reject Cookies
Background extraction (price polling) true false false false
Content extraction false true true true
Extension telemetry false false false false

Separately, it looks like network.cookie.cookieBehavior is default to 4 now in Nightly 64 which based on the behavior added in this PR would disable telemetry and background extraction by default. I am following up with @groovecoder and others about what, if anything we need to do now or very soon to handle this appropriately when 64 becomes Release (very soon after Cyber Monday). Here is the privacy review requirement for that setting currently from groovecoder:

I think it would be okay to continue price-polling even with cookieBehavior set to 4 so long as there’s an extra opt-in interstitial in the experiment. Something like what I commented in #72 (comment)

@chuckharmston
Copy link

Broke out the additional work in #184, #185, and #186. Nice job getting this landed, team!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants