Include private browsing in URL counts

VERIFIED FIXED in Firefox 61

Status

()

P1
normal
VERIFIED FIXED
8 months ago
7 months ago

People

(Reporter: mkaply, Assigned: mkaply)

Tracking

Trunk
Firefox 63
Points:
---

Firefox Tracking Flags

(firefox61blocking verified, firefox62+ verified, firefox63 verified)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

8 months ago
In bug 1271313, we added instrumentation for the counting of URIs/domains, but excluded private browsing.

Because of how we are using this data now (to determine active users), we should be counting private browsing as well.
(Assignee)

Comment 1

8 months ago
Francois, Marshall asked me to involve you as the data steward for this change.
Flags: needinfo?(francois)
(Assignee)

Updated

8 months ago
Summary: Include private browing in URL counts → Include private browsing in URL counts
Comment on attachment 8993471 [details]
Bug 1477042 - Include private browsing in URI counts.

Florian Quèze [:florian] has approved the revision.

https://phabricator.services.mozilla.com/D2250
Attachment #8993471 - Flags: review+
I see that Rebecca gave the original data review (https://bugzilla.mozilla.org/show_bug.cgi?id=1271313#c34) but I don't see a data review request form attached on bug 1271313. Would you mind filling out https://github.com/mozilla/data-review/blob/master/request.md and attaching it as a .txt file to this bug? It would be good to have a clear record of what we are collecting for this.

From what I can see in your patch, we would not be collecting Private Browsing data separately or labelling it in any way. It would go in the same bucket as all of the normal mode browsing. If that's accurate, please state so in the data review request form.

One thing that comes to mind though is people who are set to be in private browsing mode all of the time. Should we filter them out? My guess is that they don't expect that we're sending anything about their web browsing if they've set their browser to be permanently in Private Browsing.
Flags: needinfo?(francois)
Component: Telemetry → General
Product: Toolkit → Firefox
(Assignee)

Comment 5

8 months ago
This is a new patch that adds a preference so a shield study can be run on this.

Updated

8 months ago
status-firefox62: --- → affected

Updated

8 months ago
status-firefox61: --- → affected
Tracking for 62 since we may uplift once this lands.
tracking-firefox61: --- → ?
tracking-firefox62: --- → +
Comment on attachment 8995640 [details]
Bug 1477042 - Add a pref for counting URIs in private browsing mode.

Mike de Boer [:mikedeboer] has approved the revision.

https://phabricator.services.mozilla.com/D2479
Attachment #8995640 - Flags: review+

Comment 9

8 months ago
Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/c03547ff0797
Add a pref for counting URIs in private browsing mode. r=mikedeboer
Backed out for browser chrome failures on browser_address_edit.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-searchStr=Linux%20x64%20asan%20Mochitests%20with%20e10s%20test-linux64-asan%2Fopt-mochitest-browser-chrome-e10s-7%20M-e10s(bc7)&fromchange=c03547ff07973aa3f3c6b46903d914a585f39f1b&tochange=6b0d60723f1ca9fd3c0cc811738b9e45a809c141&selectedJob=190608027

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=190608027&repo=autoland&lineNumber=2102

Backout link: https://hg.mozilla.org/integration/autoland/rev/6b0d60723f1ca9fd3c0cc811738b9e45a809c141

[task 2018-07-27T20:47:58.661Z] 20:47:58     INFO - TEST-PASS | browser/components/payments/test/browser/browser_address_edit.js | Check country in response - 
[task 2018-07-27T20:47:58.662Z] 20:47:58     INFO - Console message: [JavaScript Warning: "Sending message that cannot be cloned. Are you trying to send an XPCOM object?" {file: "resource://testing-common/content-task.js" line: 62}]
[task 2018-07-27T20:47:58.663Z] 20:47:58     INFO - Console message: [JavaScript Error: "NS_NOINTERFACE: " {file: "resource://gre/modules/PrivateBrowsingUtils.jsm" line: 56}]
[task 2018-07-27T20:47:58.664Z] 20:47:58     INFO - Buffered messages finished
[task 2018-07-27T20:47:58.665Z] 20:47:58     INFO - TEST-UNEXPECTED-FAIL | browser/components/payments/test/browser/browser_address_edit.js | [JavaScript Error: "NS_NOINTERFACE: " {file: "resource://gre/modules/PrivateBrowsingUtils.jsm" line: 56}] - 
[task 2018-07-27T20:47:58.666Z] 20:47:58     INFO - Stack trace:
[task 2018-07-27T20:47:58.667Z] 20:47:58     INFO - chrome://mochitests/content/browser/browser/components/payments/test/browser/head.js:onConsoleMessage:329
[task 2018-07-27T20:47:58.668Z] 20:47:58     INFO - chrome://mochikit/content/tests/SimpleTest/specialpowersAPI.js:observe/<:377
Flags: needinfo?(mozilla)
(In reply to Cosmin Sabou [:CosminS] from comment #10)
> Backed out for browser chrome failures on browser_address_edit.

Also, the data review hasn't been done yet. See my questions in comment 4.
(Assignee)

Comment 12

8 months ago
> Also, the data review hasn't been done yet. See my questions in comment 4.

This is a setup for a shield study, not the actual change.

As far as the other error goes, dumb cut and paste error that (unfortunately) passed local testing. New patch coming.
Flags: needinfo?(mozilla)

Comment 13

8 months ago
Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/4c4e3c2df1f2
Add a pref for counting URIs in private browsing mode. r=mikedeboer

Comment 14

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4c4e3c2df1f2
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
status-firefox63: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
We have finished testing the PBM URL counts change from bug 1477042. 

QA’s recommendation: YELLOW - SHIP IT CONDITIONALLY

Reasoning:
There were no issues found during testing, but from QA perspective we are unsure what our desired behavior regarding the users that are forcing PBM through about:config is (question also asked in comment 4).
Users that are aware of this pref and actively use it, would probably expect no data collection at all.

Testing Summary:
We verified that the counting of URIs/domains works while in Private Browsing Mode in the following ways:
- Browsing normally.
- Navigating to a website after opening Private Browsing Window from the Firefox menu or using the keyboard shortcut.
- Opening a website in a New Tab and New Private Window from the context menu.
- Opening a website in a New Tab and New Private Window using keyboard navigation, CTRL/CMD, SHIFT + mouse actions or mouse middle click.
- Browsing while also being in Safe Mode.
- While being in Forced/Permanent Private Mode (set from Settings > Privacy & Security > History > Never remember history / Use custom settings for history  > Always use private browsing mode. 
-  By forcing PBM trough “browser.privatebrowsing.autostart” pref change to “true” value in “about:config” page.
- While ad blockers are installed.
- After a browser refresh, restart or update.


Tested Firefox versions:
- Firefox Release 63.0a1

Tested Platforms:
- Windows 10 x64;
- Mac 10.13.3;
- Arch Linux x64;
> Tested Firefox versions:
> - Firefox Release 63.0a1

I meant to write Firefox Nightly 63.0a1
(Assignee)

Comment 17

8 months ago
> There were no issues found during testing, but from QA perspective we are unsure what our desired behavior regarding the users that are forcing PBM through about:config is (question also asked in comment 4).

I believe it should still collect.

Private browsing is about not collecting browser history or tracking information (and we are very specific about that on the private browsing page).

If we are only tacking occasional private browsing users, I would assert that this measurement will have no effect because the users would still be considered active because of their other activity.

This additional count will primarily only affect users that are always private browsing.
Hi Mike, can you please nominate the patch for uplift to Beta62? Thanks! I'll try to get this landed in b13 that we plan to gtb in an hour.
Flags: needinfo?(mozilla)
(Assignee)

Comment 19

8 months ago
Comment on attachment 8995640 [details]
Bug 1477042 - Add a pref for counting URIs in private browsing mode.

Approval Request Comment
[Feature/Bug causing the regression]: Enable shield study
[User impact if declined]: None
[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]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Adds a pref. Existing codepath not changed unless pref is set.
[String changes made/needed]:
Flags: needinfo?(mozilla)
Attachment #8995640 - Flags: approval-mozilla-beta?
Comment on attachment 8995640 [details]
Bug 1477042 - Add a pref for counting URIs in private browsing mode.

Fix was verified in Nightly63, this change is needed to run a study on release, Beta62+
Attachment #8995640 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
We retested the PBM URL counts change with the scenarios from comment 15 on Firefox Beta 62.0b13, on the following platforms: Windows 10 x64; Mac 10.13.3; Arch Linux x64. 

No issues were found during testing, therefore the QA’s recommendation is: GREEN - SHIP IT
Status: RESOLVED → VERIFIED
status-firefox62: fixed → verified
status-firefox63: fixed → verified
Attachment #8993471 - Attachment is obsolete: true
tracking-firefox61: ? → +
tracking-firefox61: + → blocking
(Assignee)

Comment 23

8 months ago
Comment on attachment 8995640 [details]
Bug 1477042 - Add a pref for counting URIs in private browsing mode.

Approval Request Comment
[Feature/Bug causing the regression]: Add pref for shield study
[User impact if declined]: None
[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]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Adds a pref, doesn't change existing behavior.
[String changes made/needed]:
Attachment #8995640 - Flags: approval-mozilla-release?
Comment on attachment 8995640 [details]
Bug 1477042 - Add a pref for counting URIs in private browsing mode.

Needed for an upcoming study on release. Approved for 61.0.2.
Attachment #8995640 - Flags: approval-mozilla-release? → approval-mozilla-release+

Comment 25

8 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-release/rev/d22d9ccbec14
status-firefox61: affected → fixed
Flags: qe-verify+
We retested the PBM URL counts change with the scenarios from comment 15 on the latest Firefox RC (61.0.2), on the following platforms: Windows 10 x64; Mac 10.13.3; Ubuntu 16.04 x64. 

No issues were found during testing, therefore the QA’s recommendation is: GREEN - SHIP IT
status-firefox61: fixed → verified
You need to log in before you can comment on or make changes to this bug.