Closed
Bug 1451024
Opened 6 years ago
Closed 6 years ago
Cookies default settings are not greyed-out while lock a policy is used
Categories
(Firefox :: Enterprise Policies, defect)
Tracking
()
VERIFIED
FIXED
Firefox 61
People
(Reporter: Abe_LV, Assigned: bytesized)
References
Details
Attachments
(1 file, 1 obsolete file)
59 bytes,
text/x-review-board-request
|
jaws
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
Steps to reproduce: Screen capture- https://testing-1.tinytake.com/sf/MjQ4ODA1Nl83NDkyNjE0 1. Use this policy: { "policies": { "Cookies": { "Default": true, "ExpireAtSessionEnd": false, "AcceptThirdParty": "all", "Locked": true } } } 2. Go to about:preferences#privacy -> Cookies and Site Data 3. Check if "Keep until" and "Accept third-party cookie and site data" options are greyed-out. Actual result: They look active but are not changeable. Expected result: Locked options should be greyed out Note- this is not a blocking bug to the actual implementation. It is a nice to have for future developments or followups.
Reporter | ||
Updated•6 years ago
|
Flags: needinfo?(felipc)
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → ksteuber
Flags: needinfo?(ksteuber)
Comment 1•6 years ago
|
||
Yeah, this looks like a preference bug. Those are weird.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8965461 -
Flags: review?(jaws)
Comment 3•6 years ago
|
||
mozreview-review |
Comment on attachment 8965461 [details] Bug 1451024 - Cookie settings should be disabled when the related prefs are locked https://reviewboard.mozilla.org/r/234210/#review239834 ::: browser/components/preferences/in-content/privacy.js:857 (Diff revision 1) > - acceptThirdPartyLabel.disabled = acceptThirdPartyMenu.disabled = !acceptCookies; > + if (!acceptThirdPartyMenu.disabled) { > + acceptThirdPartyMenu.disabled = !acceptCookies; > + } Once this menu gets disabled, how does it ever get enabled again?
Assignee | ||
Comment 4•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8965461 [details] Bug 1451024 - Cookie settings should be disabled when the related prefs are locked https://reviewboard.mozilla.org/r/234210/#review239834 > Once this menu gets disabled, how does it ever get enabled again? As discussed on IRC, locking and unlocking prefs is not instantly synced with about:preferences the way that changes to the pref values are. The menu should not need to be enabled once disabled.
Comment 5•6 years ago
|
||
mozreview-review |
Comment on attachment 8965461 [details] Bug 1451024 - Cookie settings should be disabled when the related prefs are locked https://reviewboard.mozilla.org/r/234210/#review240724
Attachment #8965461 -
Flags: review?(jaws) → review+
Pushed by ksteuber@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0d049666e764 Cookie settings should be disabled when the related prefs are locked r=jaws
Comment 7•6 years ago
|
||
Backed out changeset 0d049666e764 (bug 1451024) for browser chrome failures on browser_privacypane_1.js Backout: https://hg.mozilla.org/integration/autoland/rev/085c62e04558eea099732b66fbbde71625cbb772 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=0d049666e764901e4e7a6ec915caa74f59bcc477&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success&filter-resultStatus=pending&filter-resultStatus=running&selectedJob=172733705 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=172733705&repo=autoland&lineNumber=6476 [task 2018-04-09T20:23:53.705Z] 20:23:53 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_privacypane_1.js | keepCookiesUntil should be disabled - [task 2018-04-09T20:23:53.706Z] 20:23:53 INFO - Buffered messages finished [task 2018-04-09T20:23:53.708Z] 20:23:53 INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_privacypane_1.js | acceptThirdPartyLabel should not be disabled - Got true, expected false [task 2018-04-09T20:23:53.709Z] 20:23:53 INFO - Stack trace: [task 2018-04-09T20:23:53.710Z] 20:23:53 INFO - chrome://mochikit/content/browser-test.js:test_is:1280 [task 2018-04-09T20:23:53.712Z] 20:23:53 INFO - chrome://mochitests/content/browser/browser/components/preferences/in-content/tests/privacypane_tests_perwindow.js:test_dependent_cookie_elements/expect_disabled/<:133 [task 2018-04-09T20:23:53.713Z] 20:23:53 INFO - chrome://mochitests/content/browser/browser/components/preferences/in-content/tests/privacypane_tests_perwindow.js:expect_disabled:132 [task 2018-04-09T20:23:53.715Z] 20:23:53 INFO - chrome://mochitests/content/browser/browser/components/preferences/in-content/tests/privacypane_tests_perwindow.js:test_dependent_cookie_elements:144 [task 2018-04-09T20:23:53.716Z] 20:23:53 INFO - chrome://mochitests/content/browser/browser/components/preferences/in-content/tests/privacypane_tests_perwindow.js:runTestOnPrivacyPrefPane:11 [task 2018-04-09T20:23:53.717Z] 20:23:53 INFO - Not taking screenshot here: see the one that was previously logged [task 2018-04-09T20:23:53.719Z] 20:23:53 INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_privacypane_1.js | acceptThirdPartyMenu should not be disabled - Got true, expected false
Flags: needinfo?(ksteuber)
Assignee | ||
Comment 8•6 years ago
|
||
I am now able to reproduce this bug. It appears to show up only in debug builds. Working on a fix now.
Flags: needinfo?(ksteuber)
Assignee | ||
Comment 9•6 years ago
|
||
My mistake. I can, in fact, reproduce this in a release build in addition to a debug build. Still working on a fix.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8965461 -
Attachment is obsolete: true
Assignee | ||
Updated•6 years ago
|
Attachment #8966770 -
Flags: review?(jaws)
Assignee | ||
Comment 11•6 years ago
|
||
jaws, it looks like you were right when you suspected that being unable to re-enable those controls would be a problem. As I said then, it does not result in any problems as far as enterprise policies are concerned, but it looks like it causes problems when cookies are fully disabled, then re-enabled. This patch should address that. I'm not crazy about basically adding an extra mechanism for disabling controls with locked prefs, but I think that this solution is acceptable.
Comment 12•6 years ago
|
||
mozreview-review |
Comment on attachment 8966770 [details] Bug 1451024 - Cookie settings should be disabled when the related prefs are locked https://reviewboard.mozilla.org/r/235454/#review241892 ::: browser/components/preferences/in-content/privacy.js:862 (Diff revision 1) > > - acceptThirdPartyLabel.disabled = acceptThirdPartyMenu.disabled = !acceptCookies; > + acceptThirdPartyLabel.disabled = acceptThirdPartyMenu.disabled = controlsDisabled; > > let privateBrowsing = Preferences.get("browser.privatebrowsing.autostart").value; > - keepUntil.disabled = menu.disabled = privateBrowsing || !acceptCookies; > + let cookieExpirationLocked = Services.prefs.prefIsLocked("network.cookie.lifetimePolicy"); > + controlsDisabled = privateBrowsing || !acceptCookies || cookieExpirationLocked; This changes the meaning of `controlsDisabled`. I'd prefer if you introduce a new variable so future code doesn't have to worry about the potentially changing value of this. Then you can declear `controlsDisabled` as a `const`.
Attachment #8966770 -
Flags: review?(jaws) → review+
Comment hidden (mozreview-request) |
Comment 14•6 years ago
|
||
Pushed by ksteuber@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/37ef105b62dc Cookie settings should be disabled when the related prefs are locked r=jaws
Comment 15•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/37ef105b62dc
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Reporter | ||
Comment 16•6 years ago
|
||
We tested this on latest nightly with adm and JSON policies and it is verified as fixed. Settings are now greyed out when the lock option is used in the policy. Test cases and runs are here- https://testrail.stage.mozaws.net/index.php?/plans/view/8760
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 17•6 years ago
|
||
Comment on attachment 8966770 [details] Bug 1451024 - Cookie settings should be disabled when the related prefs are locked Approval Request Comment [Feature/Bug causing the regression]: Enterprise Policy Engine [User impact if declined]: When cookie settings are locked, the controls will not correctly appear to be disabled [Is this code covered by automated tests?]: Included in patch [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: Testing being done by Abe [List of other uplifts needed for the feature/fix]: Related bugs already uplifted. [Is the change risky?]: Minimal Risk [Why is the change risky/not risky?]: Changes behavior of about:preferences control to the standard behavior - When related pref is locked, control should be disabled. [String changes made/needed]: None
Attachment #8966770 -
Flags: approval-mozilla-beta?
Comment 18•6 years ago
|
||
Comment on attachment 8966770 [details] Bug 1451024 - Cookie settings should be disabled when the related prefs are locked fix for looking cookie settings by policy, approved for 60.0b14
Attachment #8966770 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 19•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/c5734a8b1d7a
status-firefox60:
--- → fixed
Updated•6 years ago
|
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•