1659335 - When the box 'Accept cookies from sites' is not checked, the cookies 'Exceptions' button is greyed out

Status: RESOLVED FIXED

(Thunderbird :: Preferences, defect)

Description

[Christian Riechers]

Attachment: cookie_exceptions.png

TB78.2.0-pre build1 on Linux

Preferences - Privacy & Security
When the box 'Accept cookies from sites' is not checked, the cookies 'Exceptions' button is greyed out - see attached screenshot.
This isn't the case with TB68. The idea is not to allow cookies for RSS feeds, but create exceptions to allow cookies for OAuth2 providers.
The problem is also present in TB81.0 daily.

Comment 1

[Jorg K (CEST = GMT+2)]

Alice, can you find the regression for us, please. Thanks in advance.

Comment 2

[Alice0775 White]

Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=0c5827184c460f2052547c481288f3088d657344&tochange=aa0ea60ab031095e06721e9dcd08c0cdd2766469
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=173e2e6757ae7c0f3c7b48836c7aea01bd5ba33c&tochange=2ea5446878716fe02bb5624a2740d7c82f8de13e

Comment 3

[Jorg K (CEST = GMT+2)]

Thanks Alice. The regression isn't quite clear here, maybe bug 1622703. Or would it have been something in M-C? Unlikely since this is TB UI.

Comment 4

[Jorg K (CEST = GMT+2)]

Christian, can you please clarify the issue a bit. Usually one would think that if "Accept cookies from sites" is checked, it makes sense to define list of exceptions where not cookies are accepted. What exceptions do you want to define when cookies aren't accepted generally? What am I missing?

EDIT: Ah, I see it now, this is bad UI, but the exceptions panel says "always" or "never", so if you generally don't accept cookies, you can add some "always" exceptions.

Comment 5

[Magnus Melin [:mkmelin]]

Lasana, can you investigate+fix this.

Comment 6

[Lasana Murray]

Appears to be introduced by the fix for bug 1622703 at this location specifically:
https://searchfox.org/comm-central/diff/6fca3093634e3bb2ccb3e946be59c4033f9cf332/mail/components/preferences/privacy.js#140

Comment 7

[Lasana Murray]

Attachment: bug1659335-fix.patch

Fix and modified tests to check for this.

Comment 8

[Geoff Lankow (:darktrojan)]

You may need to fix the enterprise policy tests too.

Comment 9

[Lasana Murray]

Attachment: bug1659335-fixv2.patch

I updated the browser_policy_cookie_settings tests as well.

Comment 10

[Magnus Melin [:mkmelin]]

Comment on attachment 9171975 [details] [diff] [review]
bug1659335-fixv2.patch

Review of attachment 9171975 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/preferences/test/browser/head.js
@@ +191,5 @@
>      for (let test of tests) {
>        info(`Checking ${test.checkboxID}`);
>  
> +      let unaffectedSelectors = test.unaffectedElements || [];
> +      let unaffectedStates = unaffectedSelectors.map(getDisableState);

would just inline this to

.map(sel => prefsDocument.querySelector(sel).disabled);

Comment 11

[Magnus Melin [:mkmelin]]

Please update for comment 10 first

Comment 12

[Lasana Murray]

Attachment: bug1659335fixv3.patch

getDisableState replaced with arrow function.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d44db91db09e
Do not disable the cookie exceptions button when accept cookies is unchecked. r=mkmelin DONTBUILD

Comment 14

[Magnus Melin [:mkmelin]]

Please request uplifts to beta and 78.

Comment 15

[Magnus Melin [:mkmelin]]

Comment on attachment 9172663 [details] [diff] [review]
bug1659335fixv3.patch

[Approval Request Comment]
Regression caused by (bug #): bug 1622703
User impact if declined: can't as easily add cookie exceptions

Comment 16

[Wayne Mery (:wsmwk)]

Comment on attachment 9172663 [details] [diff] [review]
bug1659335fixv3.patch

[Triage Comment]
Approved for esr78

Comment 17

[Rob Lemley [:rjl]]

Thunderbird 78.3.2:
https://hg.mozilla.org/releases/comm-esr78/rev/76c14838e7c3