1586934 - [Protections Panel] Cookies subview says "cross-site tracking cookies" for all cookieBehaviors when there's a site exception

Status: RESOLVED FIXED

(Firefox :: Protections UI, defect, P3)

Description

[(inactive) Nihanth Subramanya [:nhnt11]]

Set cookieBehavior=1, and disable TP for the site. Open the cookies subview. The header string says "cross-site tracking cookies" instead of "third-party cookies"

Comment 1

[Tanvi Vyas[:tanvi]]

We may also want to consider what the strings should look like fro cookie behavior 2 and 3. If we block all cookies, what does the subview show? all cookies?

Comment 2

[Tanvi Vyas[:tanvi]]

Adding Betsy and Eric since we may need new strings.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Comment 4

[Johann Hofmann [:johannh]]

Hmph

Comment 5

[Betsy Mikel [:betsymi]]

New strings:

Blocking Third-Party Cookies
Blocking Cookies

Not Blocking Third-Party Cookies
Not Blocking Cookies

Comment 6

[Meena Murthy]

Please assign this bug to me.

Comment 7

[(inactive) Nihanth Subramanya [:nhnt11]]

The logic for setting this string is here: https://searchfox.org/mozilla-central/rev/6cd54550a27e2f6ca0755a25328f769e41e524f4/browser/base/content/browser-siteProtections.js#749

We need to improve the logic to handle each cookieBehavior case.

Comment 8

[Meena Murthy]

(In reply to Tanvi Vyas[:tanvi] from comment #1)

We may also want to consider what the strings should look like fro cookie behavior 2 and 3. If we block all cookies, what does the subview show? all cookies?

With Site Exception for cookie behavior 2 = BEHAVIOR_REJECT which is "All cookies"
Content blocking label shows "All Cookies"
On clicking "All Cookies", subview opens the Protections title shows as "Not Blocking Cross-Site Tracking Cookies"
without the site exception for same cookie behavior = 2 Protections title shows as "All Cookies Blocked"

With Site Exception for cookie behavior 3 = BEHAVIOR_LIMIT_FOREIGN which is about Unvisted site cookies
Content blocking label shows "Cross-site Tracking Cookies"
On clicking "Cross-site Tracking Cookies", subview opens the Protections title shows as "Not Blocking Cross-Site Tracking Cookies"
without the site exception for same cookie behavior = 3 Protections title still shows as "Not Blocking Cross-Site Tracking Cookies"
In updateCategoryItem() and updateSubView() under ThirdPartyCookies in browser-siteProtections.js there is logic for setting these, but needs to be checked if it is hitting this point.

Comment 9

[Meena Murthy]

(In reply to Meena Murthy from comment #8)

(In reply to Tanvi Vyas[:tanvi] from comment #1)

We may also want to consider what the strings should look like fro cookie behavior 2 and 3. If we block all cookies, what does the subview show? all cookies?

With Site Exception for cookie behavior 2 = BEHAVIOR_REJECT which is "All cookies"
Content blocking label shows "All Cookies"
On clicking "All Cookies", subview opens the Protections title shows as "Not Blocking Cross-Site Tracking Cookies"
without the site exception for same cookie behavior = 2 Protections title shows as "All Cookies Blocked"

With Site Exception for cookie behavior 3 = BEHAVIOR_LIMIT_FOREIGN which is about Unvisted site cookies
Content blocking label shows "Cross-site Tracking Cookies"
On clicking "Cross-site Tracking Cookies", subview opens the Protections title shows as "Not Blocking Cross-Site Tracking Cookies"
without the site exception for same cookie behavior = 3 Protections title still shows as "Not Blocking Cross-Site Tracking Cookies"
In updateCategoryItem() and updateSubView() under ThirdPartyCookies in browser-siteProtections.js there is logic for setting these to "Unvisted sites", but needs to be checked if it is hitting this point.

P.S. Did not know how to edit so adding as another comment

Comment 10

[(inactive) Nihanth Subramanya [:nhnt11]]

I had a chat with Meena about the Unvisited Sites issue on Matrix.

Our title-setting code is handling this cookieBehavior value, but it will never be executed because we don't consider cookie blocking enabled for this case - so we hit an early return.

I'm not aware of any plans to change the semantics of ThirdPartyCookies.enabled, so I think we can remove that case.

Comment 11

[Meena Murthy]

(In reply to Betsy Mikel [:betsymi] from comment #5)

New strings:

Blocking Third-Party Cookies
Blocking Cookies

Not Blocking Third-Party Cookies
Not Blocking Cookies

Betsy,

Under which scenarios would "Blocking Cookies" and "Not Blocking Cookies" be displayed in subview?

Comment 12

[Meena Murthy]

(In reply to Nihanth Subramanya [:nhnt11] from comment #10)

I had a chat with Meena about the Unvisited Sites issue on Matrix.

Our title-setting code is handling this cookieBehavior value, but it will never be executed because we don't consider cookie blocking enabled for this case - so we hit an early return.

I'm not aware of any plans to change the semantics of ThirdPartyCookies.enabled, so I think we can remove that case.

Nihanth, do we remove the "unvisited site" related code in the title-setting switch block here? https://searchfox.org/mozilla-central/source/browser/base/content/browser-siteProtections.js#645

Comment 13

[Meena Murthy]

Attachment: Bug 1586934 Fixed cookie subview for all cookieBehaviors when thers's a site exception. r=nhnt11

Depends on D67178

Comment 14

[Meena Murthy]

Attachment: Bug 1586934 Fixed cookie subview for all cookieBehaviors when thers's a site exception

Depends on D68038

Comment 15

[(inactive) Nihanth Subramanya [:nhnt11]]

(In reply to Meena Murthy from comment #11)

(In reply to Betsy Mikel [:betsymi] from comment #5)

New strings:

Blocking Third-Party Cookies
Blocking Cookies

Not Blocking Third-Party Cookies
Not Blocking Cookies

Betsy,

Under which scenarios would "Blocking Cookies" and "Not Blocking Cookies" be displayed in subview?

This is for the cookie behavior where we block all cookies.

(In reply to Meena Murthy from comment #12)

(In reply to Nihanth Subramanya [:nhnt11] from comment #10)

I had a chat with Meena about the Unvisited Sites issue on Matrix.

Our title-setting code is handling this cookieBehavior value, but it will never be executed because we don't consider cookie blocking enabled for this case - so we hit an early return.

I'm not aware of any plans to change the semantics of ThirdPartyCookies.enabled, so I think we can remove that case.

Nihanth, do we remove the "unvisited site" related code in the title-setting switch block here? https://searchfox.org/mozilla-central/source/browser/base/content/browser-siteProtections.js#645

I don't mind removing it in this patch but we can also file a follow-up bug for it. Please do one of those things. :)

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4610dabc0fc0
Fixed cookie subview for all cookieBehaviors when thers's a site exception. r=nhnt11

Comment 17

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/4610dabc0fc0