Edit after re-titling the bug: the need for improvement arises from isDetected being prone to false positives - we end up using it to enable and show the category item but when clicked, there turn out to be no items in the subview.

Talked to Johann about this. Currently the "detected" state of the cookies category just means that there was some kind of cookie detected - it doesn't go by the type of cookies that we're blocking. As for tracking content - we were showing the empty subview with the strict mode hint before, to communicate that there were possible trackers to block using strict. But now that we're removing that hint, we should just hide the category (or rather punt it to the "not found" section)

There are a few possible approaches to explore here, I will comment after investigating further.

OK, I've been racking my brains at this and I don't think we can solve this before the merge. I can't find a way to achieve all the logic with the available information, even with some compromises.

Here is a summary of the ideal truth state of ThirdPartyCookies.isDetected(contentBlockingEvent) where contentBlockingEvent is the bitfield with the blocked/loaded states:

1. isDetected controls whether the category item is clickable, to go into the subview.
2. The category item should not be clickable, if the subview is going to be empty when it's opened.
3. Therefore, isDetected means, we have cookies to show in the subview.

The subview shows either the cookies being blocked, or the cross-site tracking cookies we've allowed.

Logical details:

1. If we are set to block some type of cookie, isDetected should be true if we have blocked cookies, or we have loaded a cookie that we otherwise would have blocked due to an exception (impossible to know because the content blocking event doesn't seem to give us an origin! Please let me know if I'm wrong about this)
2. If we are set to allow cookies, we want to show the cross-site tracking cookies we WOULD HAVE blocked, in the subview. Therefore, isDetected should be true if we have loaded tracker cookies. This is impossible to know based on the web progress state alone. A compromise is to use ((we have loaded cookies) && (we have loaded trackers)) and while that's better than nothing, it's still not too great IMO.

All the information we need is in the content blocking log, which I don't want to fetch and parse every time we get a content blocking event. Is there a reason we don't pass all the information of a log entry to onContentBlockingEvent? How difficult would it be to extend the content blocking event notification system to share the origin of the content being blocked? That would help immensely. I couldn't find any ways to get anything useful from the nsIWebProgress instance that's available.

Johann, one hack I can think of to make this work to some extent is to be less lazy about updating the subview, i.e. do it either when the panel is opened and/or when the page load is complete, and from there determine the notFound state of the category item. What do you think of this?

The problem with reading the content blocking log is that it's async and slow, so it will cause flicker and reduce performance. The onContentBlockingEvent event is only fired for state changes not per actual blocking event, so I'm not sure what you'd do with the origin of the resource.

What exactly was wrong with this approach (as an interim solution):

isDetected = true, if:

For cookieBehavior=4: COOKIES_BLOCKED OR TRACKERS_LOADED
For other cases: COOKIES_BLOCKED OR COOKIES_LOADED

Yes, this will lead to edge cases where the panel is empty, but those will be quite rare, I believe. It's definitely much more workable than what we have right now. This buys us time to look into sending better content blocking events.

(In reply to Johann Hofmann [:johannh] from comment #6)

The problem with reading the content blocking log is that it's async and slow, so it will cause flicker and reduce performance. The onContentBlockingEvent event is only fired for state changes not per actual blocking event, so I'm not sure what you'd do with the origin of the resource.

What exactly was wrong with this approach (as an interim solution):

isDetected = true, if:

For cookieBehavior=4: COOKIES_BLOCKED OR TRACKERS_LOADED
For other cases: COOKIES_BLOCKED OR COOKIES_LOADED

Yes, this will lead to edge cases where the panel is empty, but those will be quite rare, I believe. It's definitely much more workable than what we have right now. This buys us time to look into sending better content blocking events.

Needed that sanity check, thanks. New patch coming up.

https://hg.mozilla.org/mozilla-central/rev/6e6f9cf6ea58

Empty categories are still shown in Firefox Nightly with the fix from this bug.

Yeah, this is expected. We realized that we don't have a perfect way to solve this bug and landed a workaround that covers many but not all cases. Platform changes are being made in bug 1576641 to try and help the situation. I expect this will take some time (and some more bugs) to fully fix.

In that light, maybe it's worth making all such bugs block this one, and re-title this one a bit to better indicate that the solution in the bug doesn't cover all cases.

Thanks for clarification! :)

https://hg.mozilla.org/releases/mozilla-beta/rev/6e6f9cf6ea58