Closed Bug 1514853 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox :: Protections UI, defect, P1)

65 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 66
Tracking Status
firefox65 + fixed
firefox66 + fixed

People

(Reporter: tanvi, Assigned: ehsan.akhgari)

References

Details

(Whiteboard: [privacy65][triage])

Attachments

(1 file)

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 */);
Target Milestone: --- → Firefox 65
Priority: -- → P1
Attached patch pref.patch (deleted) — Splinter Review
Assignee: nobody → amarchesini
Attachment #9032480 - Flags: review?(tanvi)
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)
No; are we already past early beta 65? I will investigate.
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)
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. :-)
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/12f70c0d3d1e network.cookie.cookieBehavior default to 0 in release, r=tanvi
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
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
Assignee: amarchesini → ehsan
Flags: needinfo?(ehsan)
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.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
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.

Attachment

General

Created:
Updated:
Size: