Closed Bug 1800102 Opened 2 years ago Closed 1 year ago

Dispatch "cookiebannerhandled" events when nsICookieInjector handles cookie banners

Categories

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

task

Tracking

()

RESOLVED FIXED
110 Branch
Tracking Status
firefox110 --- fixed

People

(Reporter: pbz, Assigned: pbz)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

This is split off from Bug 1798960.

Dispatching events for the cookie injector is particularly tricky because we don't have a valid WindowContext at the time of injection. Perhaps we can store something on loadInfo and dispatch the event on location change.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1798960#c0 for more details.

Jared, is this a requirement for the front-end? As I understand it we need the "cookiebannerhandled" event for the UI to hightlight that we handled a banner. Where does that show?

The click component already dispatches this, however without this bug when we handle a banner by cookie injection there won't be an event. Is that a blocker for any of your work?

Flags: needinfo?(jhirsch)
Assignee: nobody → pbz
Status: NEW → ASSIGNED

I think we'll want to get some input from Product, so I've needinfo'd :RT, but I don't think we strictly need this event at all.

Background

For v1 / Firefox 110, when the cookie banner handling code fires, we plan to show this in the UI by glowing the shield in the toolbar.

Note that the shield may already be glowing because ETP is enabled for the site, or the shield may be disabled (crossed out with a diagonal line) because the user disabled ETP for the site. (If ETP is disabled, I don't think we should glow the shield, but I need to follow up with UX/Product to confirm.)

From the UI perspective, by glowing the shield, we are giving the user an indication that something in Firefox is acting on their behalf for the current site, which could be ETP, or CBH, or both.

Option 1: just check the CBH list to decide whether to glow the shield, whether or not we successfully block the banner

Strictly speaking, to decide whether to glow the shield, we don't need the cookiebannerhandled event. Instead, at page load time, we can just check if the site is in the CBH rules list and is not in the exceptions list, and glow the shield accordingly.

This may sometimes be wrong, but if ETP is enabled for the site, the shield will be glowing anyway, so it won't be clear to the user that we guessed wrong about CBH.

This is easier for engineering, because the rules list and exceptions list can already be checked from the UI code. We might be able to just rip out the code that fires the cookiebannerhandled event altogether.

Option 2: ensure we block the banner before we glow the shield

On the other hand, if we only want to glow the shield after the banner has been handled, then we would need to ensure the cookiebannerhandled event fires in all cases. Note that, if ETP is enabled for a site, the shield will already be glowing, so this won't add anything to the user experience.

This is harder for engineering, because keeping track of cookie injection would require some additional work, as described above by :pbz.

If the plans for CBH indicator UI are significantly different for v2, we might later need the cookiebannerhandled event, but that's a separate question.

Flags: needinfo?(jhirsch) → needinfo?(rtestard)

I think some UI change to indicate that a banner has been handled would be good, and IMO it should be based on the actual handled state, otherwise we end up in situations where we say we handled the banner but didn't actually do it. Just having a rule for a site is too weak of a signal.

I've raised this in #cookie-banners on matrix already, but I think it would be good to have a more pronounced, standalone indicator showing that we handled a banner, e.g. like the one the screenshot in Bug 1795893 shows.

I think we should go with Option 1. My rationale is there even if there are scenarios where this could be an issue (delay to clear the banner or clearing the banner incorrectly / not clearing the banner), we designed our solution around the assumption that these will be very rare (QA and regression testing should identify these issues ideally) and low user impact.
I also think that getting user attention to the shield sounds like a good thing in this case - hopefully the user attention is drawn there and users report the issue with the feature we have under the follow-on items.

Flags: needinfo?(rtestard)

Something that came up on Matrix: Relying on the event rather than the rule presence means we can show the indicator when the banner has been handled for the first time and we no longer need to show it on consecutive visits. This is much less intrusive IMO.

Attachment #9307619 - Attachment description: WIP: Bug 1800102 - Dispatch cookiebannerdetected and cookiebannerhandled events after nsICookieInjector handled a banner. r=timhuang! → Bug 1800102 - Dispatch cookiebannerdetected and cookiebannerhandled events after nsICookieInjector handled a banner. r=timhuang!
Attachment #9307620 - Attachment description: WIP: Bug 1800102 - Add a cookiebanner event test for the cookie injector. r=timhuang! → Bug 1800102 - Add a cookiebanner event test for the cookie injector. r=timhuang!
Pushed by pzuhlcke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/27fea4eb98cd
Dispatch cookiebannerdetected and cookiebannerhandled events after nsICookieInjector handled a banner. r=timhuang,necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/d284f149218f
Add a cookiebanner event test for the cookie injector. r=timhuang
Regressions: 1806079
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 110 Branch
Duplicate of this bug: 1801084
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: