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)
Tracking
()
RESOLVED
FIXED
Firefox 66
People
(Reporter: tanvi, Assigned: ehsan.akhgari)
References
Details
(Whiteboard: [privacy65][triage])
Attachments
(1 file)
(deleted),
patch
|
tanvi
:
review+
RyanVM
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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•6 years ago
|
Target Milestone: --- → Firefox 65
Reporter | ||
Updated•6 years ago
|
Priority: -- → P1
Comment 1•6 years ago
|
||
Assignee: nobody → amarchesini
Attachment #9032480 -
Flags: review?(tanvi)
Reporter | ||
Comment 2•6 years 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)
Assignee | ||
Comment 3•6 years ago
|
||
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 4•6 years ago
|
||
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•6 years ago
|
||
No; are we already past early beta 65? I will investigate.
Reporter | ||
Comment 6•6 years 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•6 years ago
|
Attachment #9032480 -
Flags: review?(tanvi) → review+
Assignee | ||
Comment 7•6 years ago
|
||
(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
Comment 9•6 years ago
|
||
bugherder |
Comment 11•6 years ago
|
||
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?
Updated•6 years ago
|
status-firefox65:
--- → affected
tracking-firefox65:
--- → +
tracking-firefox66:
--- → +
Target Milestone: Firefox 65 → Firefox 66
Comment 12•6 years ago
|
||
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+
Comment 13•6 years ago
|
||
bugherder uplift |
Assignee | ||
Comment 14•6 years ago
|
||
(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 */);
Comment 15•6 years ago
|
||
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)
Updated•6 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•6 years ago
|
Comment 16•6 years ago
|
||
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 | ||
Updated•6 years ago
|
Assignee: amarchesini → ehsan
Flags: needinfo?(ehsan)
Comment 18•6 years 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
Assignee | ||
Comment 19•6 years ago
|
||
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.
Assignee | ||
Comment 20•6 years ago
|
||
bugherder uplift |
Comment 21•6 years ago
|
||
bugherder |
Status: REOPENED → RESOLVED
Closed: 6 years ago → 6 years ago
Resolution: --- → FIXED
Comment 22•6 years ago
|
||
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
Assignee | ||
Comment 23•6 years ago
|
||
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.
Description
•