Extend hit logger to be able to listen to requests across all tabs

[greiner]

Background

In #6402 we introduced the hitLogger module that allows any UI to listen to requests for any given tab. In #6404 we'd like to introduce a stand-alone UI that's similar to our DevTools panel but that shows requests across all tabs.

What to change

• Extend HitLogger to allow listeners to choose to listen to requests across all open tabs.
• Allow listeners to differentiate the data by tab.

[kzar]

Can't you use the browser.tabs APIs in combination with the existing HitLogger implementation to do this?

[greiner]

Yes, we could implement such a logic in adblockplusui. Do you think we should?

At least I'd expect the implementation to be much simpler within HitLogger given that logRequest() already receives and handles all requests without having to maintain a list of all tabs.

[kzar]

Well we previously left quite a bit of logic out of the HitLogger API since it was only needed by the devtools code. So it seems consistent that we should do the same here too and leave this logic that only one consumer of the API needs out of the API. Then on the other hand if you're right and it's simpler to include the logic in the API in practice perhaps we should just do it.

Do you have an opinion Sebastian?

[sebastian]

FWIW, I'm not convinced that it's worth it to add yet another user interface to show blocked/blockable items, in the first place. It seems redundant with the functionality provided by the devtools panel, but less useful if items from all tabs are listed in a combined view shown in a separate tab, not to mention the challenge (and resulting complexity) to abstract the common functionality properly.

[greiner]

Well we previously left quite a bit of logic out of the HitLogger API since it was only needed by the devtools code. So it seems consistent that we should do the same here too and leave this logic that only one consumer of the API needs out of the API.

I totally see your point and I agree that functionality that's only used by the UI doesn't belong into adblockpluschrome. Therefore I'm open for ideas.

Fortunately, there's no pressure since we don't even have a design ready yet so if you two want to wait for that, that'd be fine with me. What's important for me is that we identify any problems beforehand, which could make such a feature impossible/improbable to implement.

It seems redundant with the functionality provided by the devtools panel

Showing more data on the same page does offer some added value, depending on how we arrange it. Also note that, unlike the DevTools window, we'd be able to open this page programmatically (e.g. through UI interactions or keyboard shortcut).

See also #6404 and https://issues.adblockplus.org/ticket/6402#comment:7.

but less useful if items from all tabs are listed in a combined view shown in a separate tab

The UI complexity is likely the biggest challenge here but let's entrust that to Product.

not to mention the challenge (and resulting complexity) to abstract the common functionality properly.

That part seems manageable because, fortunately, there's not a lot going on in that UI.

[kzar]

Replying to sebastian:

FWIW, I'm not convinced that it's worth it to add yet another user interface to show blocked/blockable items, in the first place. It seems redundant with the functionality provided by the devtools panel, but less useful if items from all tabs are listed in a combined view shown in a separate tab, not to mention the challenge (and resulting complexity) to abstract the common functionality properly.

Well I will defend the idea a bit, when the carnage was unfolding from our transition to the Firefox WebExtension APIs I spent a bunch of time reading through the forums and any other places I could find where users provided feedback. This idea of splitting our devtools pane out into a separate page got quite a lot of support. For example users mentioned that the Firefox developer tools are so slow on their computer that they'd rather avoid using them.

[sebastian]

Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the ​new issue tracker.