1803601 - Allow to persist cookie banner handling exceptions in private mode

Status: RESOLVED FIXED

(Core :: Privacy: Anti-Tracking, task, P2)

Description

[Arturo Mejia [:amejia]]

At the moment the cookie banner exceptions are only session scoped in private mode (No persistent), Focus for Android is always in private mode, and users would expected that exceptions will persist after app restarts similar to tracking protection exceptions.

See more details on the matrix matrix.

Comment 1

[Emma Zühlcke [:emz]]

Tim, do you think we could add a flag to the setter method to persist the pref in private browsing for specific callers? Focus could then pass that flag into this method https://searchfox.org/mozilla-central/rev/2fc2ccf960c2f7c419262ac7215715c5235948db/toolkit/components/cookiebanners/nsICookieBannerService.idl#89
We would probably need a separate bucket for PBM exceptions that are persistent?

Comment 2

[Tim Huang[:timhuang]]

I think we can just use the disk version of the setting if the permanent PBM is enabled. Maybe this is easier.

Comment 3

[Emma Zühlcke [:emz]]

This might be fine for Focus, but if we use this flag in other cases in the future we may run into issues with PBM overwriting normal browsing prefs.

Comment 4

[Tim Huang[:timhuang]]

Yeah, this is a good concern. But we can use a different content pref key for permanent PBM mode. Then, it won't mess up with normal browsing prefs.

Comment 5

[[:owlish] 🦉 PST]

Gabriel, what is the need re: the API? Do you have any requirements, what would be convenient to use to implement this feature? (AC or GV API)

Comment 6

[Iorga Gabriel]

(In reply to [:owlish] 🦉 PST from comment #5)

Gabriel, what is the need re: the API? Do you have any requirements, what would be convenient to use to implement this feature? (AC or GV API)

Hi , I think it should be something like this method from ac ,with a different name like addPresistentPrivateException.
override suspend fun addException(
uri: String,
privateBrowsing: Boolean,
) {
setGeckoException(uri, DISABLED, privateBrowsing)
}

Comment 7

[Tim Huang[:timhuang]]

One question from me that Arturo might be able to answer. Does Focus rely on the pref browser.privatebrowsing.autostart to enable permanent private mode? If so, do you think we can use the same pref to control the behavior here, or should we introduce a separate pref for this purpose? Thanks.

Comment 8

[Tim Huang[:timhuang]]

I've discovered how Geckoview persists permission for the private mode. There is a separate API to allow Geckoview to persist permission for a private session. Maybe we can also follow the same pattern here.

Comment 9

[Arturo Mejia [:amejia]]

(In reply to Tim Huang[:timhuang] from comment #7)

One question from me that Arturo might be able to answer. Does Focus rely on the pref browser.privatebrowsing.autostart to enable permanent private mode? If so, do you think we can use the same pref to control the behavior here, or should we introduce a separate pref for this purpose? Thanks.

We don't use it in GeckoView, we don't expose it to apps. But if it's convenient for you we can do it, the flag is true and we call

      Services.cookieBanners.setDomainPref(  uri,  mode, true);

right?

Comment 10

[Arturo Mejia [:amejia]]

(In reply to Tim Huang[:timhuang] from comment #8)

I've discovered how Geckoview persists permission for the private mode. There is a separate API to allow Geckoview to persist permission for a private session. Maybe we can also follow the same pattern here.

Having a separate API will be ideally to be more explicit and have consistency with other APIs, but not sure how much work does that generates for you.

Comment 11

[Tim Huang[:timhuang]]

I am adding a Services.cookieBanners.setDomainPrefAndPersistInPrivateBrowsing() to allow GeckoView to persist settings for private windows.

Comment 12

[Arturo Mejia [:amejia]]

That sounds fantastic 🙌!
Thanks!

Comment 13

[Tim Huang[:timhuang]]

Attachment: Bug 1803601 - Part 1: Adding a Services.cookieBanners.setDomainPrefAndPersistInPrivateBrowsing() to allow persist cookie banner domain pref for the private browsing. r?pbz!

This patch adds a new function setDomainPrefAndPersistInPrivateBrowsing()
to nsICookieBannerService. This function allows persisting domain pref
in the private browsing mode.

Comment 14

[Tim Huang[:timhuang]]

Attachment: Bug 1803601 - Part 2: Add tests for nsICookieBannerService.setDomainPrefAndPersistInPrivateBrowsing(). r?pbz!

Depends on D164797

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a9679be52b5b
Part 1: Adding a Services.cookieBanners.setDomainPrefAndPersistInPrivateBrowsing() to allow persist cookie banner domain pref for the private browsing. r=pbz
https://hg.mozilla.org/integration/autoland/rev/fff79aec4a22
Part 2: Add tests for nsICookieBannerService.setDomainPrefAndPersistInPrivateBrowsing(). r=pbz

Comment 16

[Cristian Tuns]

Backed out for causing build bustages on nsCookieBannerService.cpp

• Backout link
• Push with failures
• Failure Log
• Failure line: /builds/worker/checkouts/gecko/toolkit/components/cookiebanners/nsCookieBannerService.cpp(640,24): error: function declared 'stdcall' here was previously declared without calling convention

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ed716ee573f9
Part 1: Adding a Services.cookieBanners.setDomainPrefAndPersistInPrivateBrowsing() to allow persist cookie banner domain pref for the private browsing. r=pbz
https://hg.mozilla.org/integration/autoland/rev/3f2bdf32df53
Part 2: Add tests for nsICookieBannerService.setDomainPrefAndPersistInPrivateBrowsing(). r=pbz

Comment 18

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/ed716ee573f9
https://hg.mozilla.org/mozilla-central/rev/3f2bdf32df53