Change network.cookie.cookieBehavior default pref value in Firefox 65 back to 0 in release

RESOLVED FIXED in Firefox 65

Status

()

defect
P1
normal
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: tanvi, Assigned: Ehsan)

Tracking

65 Branch
Firefox 66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox65+ fixed, firefox66+ fixed)

Details

(Whiteboard: [privacy65][triage])

Attachments

(1 attachment)

(Reporter)

Description

5 months ago
https://searchfox.org/mozilla-central/rev/47edbd91c43db6229cf32d1fc4bae9b325b9e2d0/browser/app/profile/firefox.js#1511

We need to add back an #ifdef NIGHTLY_OR_EARLY_BETA for network.cookie.cookieBehavior.  Something like:

// Enable blocking access to storage from tracking resources by default
#ifdef NIGHTLY_OR_EARLY_BETA
pref("network.cookie.cookieBehavior", 4 /* BEHAVIOR_REJECT_TRACKER */);
#endif
pref("network.cookie.cookieBehavior", 0 /* BEHAVIOR_ACCEPT */);
(Reporter)

Updated

5 months ago
Target Milestone: --- → Firefox 65
(Reporter)

Updated

5 months ago
Priority: -- → P1
Posted patch pref.patchSplinter Review
Assignee: nobody → amarchesini
Attachment #9032480 - Flags: review?(tanvi)
(Reporter)

Comment 2

5 months ago
Comment on attachment 9032480 [details] [diff] [review]
pref.patch

baku, where is the default value of 0 for network.cookie.cookieBehavior set?
Flags: needinfo?(amarchesini)
Have we thought about how downgrading the pref from 4 to 0 during the beta cycle would work with the new Preferences UI by the way?  As in, which mode would users end up in on the beta channel, etc.
Flags: needinfo?(tanvi)
(Reporter)

Comment 5

5 months ago
No; are we already past early beta 65?  I will investigate.
(Reporter)

Comment 6

5 months ago
I guess it doesn't matter much whether or not we are still in early beta, it is still a problem.

The default value will switch from 4 to 0, so users who had it on will then end up with it turned off.  I think this is what happened in the beta 64 cycle.  The difference is the UI.  The UI will change with the default value.  A user would have been in Standard in until this pref change lands, where the default value was 4.  When we switch the default to 0, the users will still be in Standard but see the fallback UI. Users who have selected any other settings (Strict or Custom) won't change their category either.  So, unless I'm missing something, I don't think we have to worry about anything additional here.
Flags: needinfo?(tanvi)
(Reporter)

Updated

5 months ago
Attachment #9032480 - Flags: review?(tanvi) → review+
(In reply to Tanvi Vyas[:tanvi] from comment #6)
> I guess it doesn't matter much whether or not we are still in early beta, it
> is still a problem.
> 
> The default value will switch from 4 to 0, so users who had it on will then
> end up with it turned off.  I think this is what happened in the beta 64
> cycle.  The difference is the UI.  The UI will change with the default
> value.  A user would have been in Standard in until this pref change lands,
> where the default value was 4.  When we switch the default to 0, the users
> will still be in Standard but see the fallback UI. Users who have selected
> any other settings (Strict or Custom) won't change their category either. 
> So, unless I'm missing something, I don't think we have to worry about
> anything additional here.

Yes, I went through it in my head also, and I think you're right.  Now that I think about it, we did in fact think ahead about this problem and our solution was supposed to work in this case.  :-)

Comment 8

5 months ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/12f70c0d3d1e
network.cookie.cookieBehavior default to 0 in release, r=tanvi

Comment 9

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/12f70c0d3d1e
Status: NEW → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → FIXED
(Reporter)

Comment 10

5 months ago
Please request uplift.
Flags: needinfo?(amarchesini)
Comment on attachment 9032480 [details] [diff] [review]
pref.patch

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1492563

User impact if declined: This patch has been written to disable cookeBehavior 4 in release.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Just disabling a feature.

String changes made/needed: none
Flags: needinfo?(amarchesini)
Attachment #9032480 - Flags: approval-mozilla-beta?
Target Milestone: Firefox 65 → Firefox 66
Comment on attachment 9032480 [details] [diff] [review]
pref.patch

[Triage Comment]
Reverts our cookie handling back to our previous behavior after the early betas. Approved for 65.0b7. Note that I'm planning to flip the early beta ifdef after shipping b7 prior to building b8.
Attachment #9032480 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Tanvi Vyas[:tanvi] from comment #0)
> https://searchfox.org/mozilla-central/rev/
> 47edbd91c43db6229cf32d1fc4bae9b325b9e2d0/browser/app/profile/firefox.js#1511
> 
> We need to add back an #ifdef NIGHTLY_OR_EARLY_BETA for
> network.cookie.cookieBehavior.  Something like:
> 
> // Enable blocking access to storage from tracking resources by default
> #ifdef NIGHTLY_OR_EARLY_BETA

Such a macro does not exist.  It's EARLY_BETA_OR_EARLIER.

It seems like baku just copies and pasted this into the patch.  As a result, cookie restrictions have been disabled on Nightly since Friday.  :-(

> pref("network.cookie.cookieBehavior", 4 /* BEHAVIOR_REJECT_TRACKER */);
> #endif
> pref("network.cookie.cookieBehavior", 0 /* BEHAVIOR_ACCEPT */);
Depends on: 1516185
Backed out 2 changesets (Bug 1514853, Bug 1516185) for browser_contentblocking.js failures a=backout

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=c37650fcf69ca288b874f4bb780831af68dce283&selectedJob=218619396

Backout link: https://hg.mozilla.org/mozilla-central/rev/f87b08093cd56ae66434ab8a42759b37e3e4198a

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=218619396&repo=mozilla-central&lineNumber=8964

10:41:12     INFO - Entering test bound testContentBlockingCustomCategory
10:41:12     INFO - Buffered messages logged at 10:41:03
10:41:12     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_contentblocking.js | the pref urlclassifier.trackingTable remains as default value - 
10:41:12     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_contentblocking.js | the pref privacy.trackingprotection.enabled remains as default value - 
10:41:12     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_contentblocking.js | the pref privacy.trackingprotection.pbmode.enabled remains as default value - 
10:41:12     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_contentblocking.js | the pref network.cookie.cookieBehavior remains as default value - 
10:41:12     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_contentblocking.js | browser.contentblocking.category has been set to custom - 
10:41:12     INFO - Buffered messages logged at 10:41:04
10:41:12     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_contentblocking.js | browser.contentblocking.category has been set to custom - 
10:41:12     INFO - Buffered messages finished
10:41:12     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_contentblocking.js | Uncaught exception - undefined - timed out after 50 tries.
10:41:12     INFO - Leaving test bound testContentBlockingCustomCategory
...
10:41:36     INFO - GECKO(2099) | [Parent 2099, Main Thread] WARNING: NS_FAILED internal_GetScalarByEnum for CHILD: file /builds/worker/workspace/build/src/toolkit/components/telemetry/core/TelemetryScalar.cpp, line 2161
10:41:40     INFO - GECKO(2099) | --DOMWINDOW == 17 (0x118a36400) [pid = 2099] [serial = 388] [outer = 0x0] [url = about:preferences#privacy]
10:41:40     INFO - GECKO(2099) | --DOMWINDOW == 16 (0x11a367800) [pid = 2099] [serial = 396] [outer = 0x0] [url = about:preferences#privacy]
10:42:23     INFO - Longer timeout required, waiting longer...  Remaining timeouts: 1
10:43:53     INFO - Not taking screenshot here: see the one that was previously logged
10:43:53     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_contentblocking.js | Test timed out - 
10:43:54     INFO - GECKO(2099) | MEMORY STAT | vsize 4552MB | residentFast 514MB | heapAllocated 109MB
10:43:54     INFO - TEST-OK | browser/components/preferences/in-content/tests/browser_contentblocking.js | took 180361ms
10:43:54     INFO - Not taking screenshot here: see the one that was previously logged
10:43:54     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_contentblocking.js | Found a tab after previous test timed out: about:blank - 
10:43:54     INFO - Not taking screenshot here: see the one that was previously logged
10:43:54     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_contentblocking.js | Found a tab after previous test timed out: about:preferences#privacy - 
10:43:54     INFO - GECKO(2099) | ++DOCSHELL 0x114941800 == 1 [pid = 2104] [id = {1f7ee5b0-39e3-e641-b63a-6371c31e36c1}]
10:43:54     INFO - GECKO(2099) | ++DOMWINDOW == 1 (0x11e505c00) [pid = 2104] [serial = 22] [outer = 0x0]
10:43:54     INFO - GECKO(2099) | ++DOMWINDOW == 2 (0x11e5cd400) [pid = 2104] [serial = 23] [outer = 0x11e505c00]
10:43:54     INFO - GECKO(2099) | ++DOMWINDOW == 3 (0x11e5d2000) [pid = 2104] [serial = 24] [outer = 0x11e505c00]
10:43:54     INFO - checking window state
Flags: needinfo?(ehsan)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Also backed out from beta:

Backed out changeset 7af67b7f5f79 (Bug 1514853) as the followup 1516185 was backed out a=backout

Backout link: https://hg.mozilla.org/releases/mozilla-beta/rev/750811248cbc40b3cc086d1be799dfd858bad4ad

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=218619396&repo=mozilla-central&lineNumber=8964

10:41:12     INFO - Entering test bound testContentBlockingCustomCategory
10:41:12     INFO - Buffered messages logged at 10:41:03
10:41:12     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_contentblocking.js | the pref urlclassifier.trackingTable remains as default value - 
10:41:12     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_contentblocking.js | the pref privacy.trackingprotection.enabled remains as default value - 
10:41:12     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_contentblocking.js | the pref privacy.trackingprotection.pbmode.enabled remains as default value - 
10:41:12     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_contentblocking.js | the pref network.cookie.cookieBehavior remains as default value - 
10:41:12     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_contentblocking.js | browser.contentblocking.category has been set to custom - 
10:41:12     INFO - Buffered messages logged at 10:41:04
10:41:12     INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_contentblocking.js | browser.contentblocking.category has been set to custom - 
10:41:12     INFO - Buffered messages finished
10:41:12     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_contentblocking.js | Uncaught exception - undefined - timed out after 50 tries.
10:41:12     INFO - Leaving test bound testContentBlockingCustomCategory
...
10:41:36     INFO - GECKO(2099) | [Parent 2099, Main Thread] WARNING: NS_FAILED internal_GetScalarByEnum for CHILD: file /builds/worker/workspace/build/src/toolkit/components/telemetry/core/TelemetryScalar.cpp, line 2161
10:41:40     INFO - GECKO(2099) | --DOMWINDOW == 17 (0x118a36400) [pid = 2099] [serial = 388] [outer = 0x0] [url = about:preferences#privacy]
10:41:40     INFO - GECKO(2099) | --DOMWINDOW == 16 (0x11a367800) [pid = 2099] [serial = 396] [outer = 0x0] [url = about:preferences#privacy]
10:42:23     INFO - Longer timeout required, waiting longer...  Remaining timeouts: 1
10:43:53     INFO - Not taking screenshot here: see the one that was previously logged
10:43:53     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_contentblocking.js | Test timed out - 
10:43:54     INFO - GECKO(2099) | MEMORY STAT | vsize 4552MB | residentFast 514MB | heapAllocated 109MB
10:43:54     INFO - TEST-OK | browser/components/preferences/in-content/tests/browser_contentblocking.js | took 180361ms
10:43:54     INFO - Not taking screenshot here: see the one that was previously logged
10:43:54     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_contentblocking.js | Found a tab after previous test timed out: about:blank - 
10:43:54     INFO - Not taking screenshot here: see the one that was previously logged
10:43:54     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_contentblocking.js | Found a tab after previous test timed out: about:preferences#privacy - 
10:43:54     INFO - GECKO(2099) | ++DOCSHELL 0x114941800 == 1 [pid = 2104] [id = {1f7ee5b0-39e3-e641-b63a-6371c31e36c1}]
10:43:54     INFO - GECKO(2099) | ++DOMWINDOW == 1 (0x11e505c00) [pid = 2104] [serial = 22] [outer = 0x0]
10:43:54     INFO - GECKO(2099) | ++DOMWINDOW == 2 (0x11e5cd400) [pid = 2104] [serial = 23] [outer = 0x11e505c00]
10:43:54     INFO - GECKO(2099) | ++DOMWINDOW == 3 (0x11e5d2000) [pid = 2104] [serial = 24] [outer = 0x11e505c00]
10:43:54     INFO - checking window state
Duplicate of this bug: 1516185
Assignee: amarchesini → ehsan
Flags: needinfo?(ehsan)

Comment 18

5 months ago
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/724652219657
network.cookie.cookieBehavior default to 0 in release, r=tanvi
The reason why this broke tests was this change that Andrea had added to the patch after review which was breaking the test: https://hg.mozilla.org/mozilla-central/rev/12f70c0d3d1e#l2.1

That change was effectively making the test only work when the default value of the cookieBehavior is 0.  The test needs to be rewritten properly to be able to handle both cases.

Comment 21

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/724652219657
Status: REOPENED → RESOLVED
Last Resolved: 5 months ago5 months ago
Resolution: --- → FIXED
Depends on: 1516540
improvements from landing:
== Change summary for alert #18406 (as of Fri, 21 Dec 2018 03:46:59 GMT) ==

Improvements:

 27%  raptor-tp6-yandex-firefox windows10-64 pgo           155.06 -> 112.90
 25%  raptor-tp6-yandex-firefox windows7-32 pgo            150.87 -> 113.54
 25%  raptor-tp6-yandex-firefox windows10-64 opt           161.43 -> 121.53
 25%  raptor-tp6-yandex-firefox windows7-32 opt            162.41 -> 122.49
 24%  raptor-tp6-yandex-firefox osx-10-10 opt              307.22 -> 233.68
 20%  raptor-tp6-yandex-firefox windows10-64-qr opt        141.85 -> 113.03
 18%  raptor-tp6-yandex-firefox linux64 pgo                143.57 -> 117.93
 18%  raptor-tp6-yandex-firefox linux64-qr opt             147.22 -> 121.20
 17%  raptor-tp6-yandex-firefox linux64 opt                151.57 -> 125.17
 17%  raptor-tp6-wikia-firefox windows7-32 opt             179.14 -> 148.62
 11%  raptor-tp6-amazon-firefox windows10-64 opt           493.46 -> 437.83
 10%  raptor-tp6-amazon-firefox linux64-qr opt             524.60 -> 470.30
  9%  raptor-tp6-amazon-firefox linux64 opt                485.78 -> 443.42
  7%  raptor-tp6-amazon-firefox windows10-64-qr opt        486.10 -> 450.56
  7%  raptor-tp6-amazon-firefox osx-10-10 opt              1,248.22 -> 1,158.77
  7%  raptor-tp6-amazon-firefox linux64 pgo                429.34 -> 400.56
  5%  raptor-tp6-amazon-firefox windows7-32 opt            455.54 -> 432.88
  5%  raptor-tp6-amazon-firefox windows7-32 pgo            411.96 -> 391.55
  5%  raptor-tp6-amazon-firefox windows10-64 pgo           431.05 -> 410.50
  4%  raptor-tp6-facebook-firefox linux64 pgo              369.34 -> 354.88
  4%  raptor-tp6-facebook-firefox windows10-64 opt         401.36 -> 386.06
  4%  raptor-tp6-facebook-firefox linux64-qr opt           416.77 -> 401.58
  3%  raptor-tp6-facebook-firefox windows7-32 pgo          372.93 -> 360.84
  3%  raptor-tp6-facebook-firefox linux64 opt              394.29 -> 381.67
  3%  raptor-tp6-facebook-firefox windows10-64-qr opt      412.12 -> 399.20
  3%  raptor-tp6-facebook-firefox windows10-64 pgo         377.01 -> 366.28

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=18406

regression for backing out is in bug 1516540
As noted on that bug, this patch was relanded so there is no regression here per se, but the real regression is of course from cookieBehavior=0 -> cookieBehavior=4 which is what bug 1516540 is really tracking.
You need to log in before you can comment on or make changes to this bug.