Bug 1567268 Comment 8 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to :Ehsan Akhgari from comment #6)
> > and depends on the barely supported `nsIClassifiedChannel.matchedList` which doesn't give us the full range of today's content blocking categories.
> 
> Um, why do you think that?  `nsIClassifiedChannel.matchedList` is as supported as it ever was before, it gives you the matched safebrowsing table that was used to block an element.  It is currently supported for the following content blocking back-ends:
> 
>   * Tracking protection
>   * Safe browsing
>   * Fingerprinting blocking
>   * Cryptocurrency mining blocking
>   * Social tracking protection
> 
> Note that this is only supported for back-ends which block the loading of resources.

That's just inaccurately worded from my side. There have been bugs that affected GV specifically around `matchedList` before, but we haven't had any issue for a long time. I'm happy to hear that `matchedList` will continue to be supported.

On the other hand, it is only useful for blocked elements, not for potential loads (e.g., `STATE_LOADED_TRACKING_CONTENT`) for bug 1567611.

> > We should refactor the GV implementation to use `nsIWebProgressListener.onContentBlockingEvent` which would enable the API to report a wider range of block event types, including cookie-based events as triggered by some cookie behavior settings (ETP).
> 
> Well, why do these two APIs need to be tied together?  Currently GV manages to use `STATE_BLOCKED_TRACKING_CONTENT` together with the `matchedList` API without any issues, so why is using this API together with another content blocking code like `STATE_COOKIES_BLOCKED_TRACKER` an issue?

Combining `STATE_*` and `matchedList` works for us, but makes block reporting more complicated than it needs to be. This is definitely not a high priority item, just a nice-to-have.

> > For the refactoring, we will have to decouple `BlockEvent.categories` from the `Category` constants, since those only range from anti-tracking to safe browsing. The refactoring should allow for `BlockEvent` to report the complete range of content blocking events, including cookies.
> 
> Not sure if I understand this part...

GV API specifics, feel free to ignore this comment.

> (In reply to Eugen Sawin [:esawin] from comment #1)
> > The issue with `nsIWebProgressListener.STATE_BLOCKED_TRACKING_CONTENT` is that it does not allow us to differentiate between ad, analytic, social and content trackers. We will either have to split up `STATE_BLOCKED_TRACKING_CONTENT` or continue mapping the GV categories based on `nsIClassifiedChannel.matchedList`.
> 
> Is there a problem with the second option?

It's not an actual problem, but would make for a cleaner mapping between Gecko and the GV API.
 
> If that problem is what we're trying to solve here, why not just provide the `matchedList` data there?

Knowing that GV getting category info out of `matchedList` is not considered a "barely supported" hack, I think we're OK with continuing using `matchedList` data for GV block type mapping.
 
> I think we can have our cake and eat it too.  :-)

And we can share it with our users! :)
(In reply to :Ehsan Akhgari from comment #6)
> > and depends on the barely supported `nsIClassifiedChannel.matchedList` which doesn't give us the full range of today's content blocking categories.
> 
> Um, why do you think that?  `nsIClassifiedChannel.matchedList` is as supported as it ever was before, it gives you the matched safebrowsing table that was used to block an element.  It is currently supported for the following content blocking back-ends:
> 
>   * Tracking protection
>   * Safe browsing
>   * Fingerprinting blocking
>   * Cryptocurrency mining blocking
>   * Social tracking protection
> 
> Note that this is only supported for back-ends which block the loading of resources.

That's just inaccurately worded from my side. There have been bugs that affected GV specifically around `matchedList` before, but we haven't had any issue for a long time. I'm happy to hear that `matchedList` will continue to be supported.

On the other hand, it is only useful for blocked elements, not for potential blocks (e.g., `STATE_LOADED_TRACKING_CONTENT`) for bug 1567611.

> > We should refactor the GV implementation to use `nsIWebProgressListener.onContentBlockingEvent` which would enable the API to report a wider range of block event types, including cookie-based events as triggered by some cookie behavior settings (ETP).
> 
> Well, why do these two APIs need to be tied together?  Currently GV manages to use `STATE_BLOCKED_TRACKING_CONTENT` together with the `matchedList` API without any issues, so why is using this API together with another content blocking code like `STATE_COOKIES_BLOCKED_TRACKER` an issue?

Combining `STATE_*` and `matchedList` works for us, but makes block reporting more complicated than it needs to be. This is definitely not a high priority item, just a nice-to-have.

> > For the refactoring, we will have to decouple `BlockEvent.categories` from the `Category` constants, since those only range from anti-tracking to safe browsing. The refactoring should allow for `BlockEvent` to report the complete range of content blocking events, including cookies.
> 
> Not sure if I understand this part...

GV API specifics, feel free to ignore this comment.

> (In reply to Eugen Sawin [:esawin] from comment #1)
> > The issue with `nsIWebProgressListener.STATE_BLOCKED_TRACKING_CONTENT` is that it does not allow us to differentiate between ad, analytic, social and content trackers. We will either have to split up `STATE_BLOCKED_TRACKING_CONTENT` or continue mapping the GV categories based on `nsIClassifiedChannel.matchedList`.
> 
> Is there a problem with the second option?

It's not an actual problem, but would make for a cleaner mapping between Gecko and the GV API.
 
> If that problem is what we're trying to solve here, why not just provide the `matchedList` data there?

Knowing that GV getting category info out of `matchedList` is not considered a "barely supported" hack, I think we're OK with continuing using `matchedList` data for GV block type mapping.
 
> I think we can have our cake and eat it too.  :-)

And we can share it with our users! :)

Back to Bug 1567268 Comment 8