Opened 8 months ago

Closed 5 months ago

Last modified 3 months ago

#6386 closed change (fixed)

Add Requests and Filters data to the Report data [Issue reporter]

Reported by: saroyanm Assignee: saroyanm
Priority: P2 Milestone:
Module: User-Interface Keywords:
Cc: wspee, greiner, agiammarchi, kzar, sebastian, sergz Blocked By: #6402
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://gitlab.com/eyeo/adblockplus/adblockplusui/merge_requests/6

Description (last modified by saroyanm)

Background

Currently the report data doesn't contain information about the Requests and Filters.

What to change

Add Requests and Filters to the report data as specified here.

Change History (22)

comment:1 Changed 8 months ago by saroyanm

  • Description modified (diff)

comment:2 follow-up: Changed 8 months ago by saroyanm

  • Cc kzar added

@Dave maybe you can help us here:
In order to implement this issue, UI team needs to have ability to subscribe, or somehow be notified about the Blocked Request and Element Hiding Event for a specific tab.

Having look into the implementation, it seems like the only safe and easy way to do that currently is to pretend of being devtools and listen to the Add Record event, which seems to be the easiest way to implement this with the least code duplication, but this is still a Hack.

Ideally we would be able to generalize the implementation, to avoid similar Hacks, but still before starting the implementation it definitely make sense to align with the platform team.

Question: Is there any other better/preferred (existing) way to achieve this you can think of ?

Let's start discussion and let's see how we gonna proceed here.

comment:3 in reply to: ↑ 2 ; follow-up: Changed 8 months ago by kzar

  • Cc sebastian added

Replying to saroyanm:

In order to implement this issue, UI team needs to have ability to subscribe, or somehow be notified about the Blocked Request and Element Hiding Event for a specific tab.

So first of all have I got this right?

You want it so that when the user clicks to report an issue, a list of all requests that the main page (and any frames?) made, and a list of all filter hits both element hiding and request blocking for the page are included in the report data that's sent to us.

My first concern is privacy, I assume we're very clear about all this data that we're sharing? I worry a full list of requests the page made could be quite intrusive.

Having look into the implementation, it seems like the only safe and easy way to do that currently is to pretend of being devtools and listen to the Add Record event, which seems to be the easiest way to implement this with the least code duplication, but this is still a Hack.

My second concern is performance, to do this without having the user somehow refresh the page we'd have to be recording all the filter hit information at all times. We are very careful not to do this for the developer tools pane for performance reasons, instead we only do it when the developers tool pane is open.

(With the legacy Firefox extension there was a hack which allowed us to record element hiding hits quite cheaply, but that's not the case on Chrome or with the Firefox WebExtension unfortunately.)

Ideally we would be able to generalize the implementation, to avoid similar Hacks, but still before starting the implementation it definitely make sense to align with the platform team.

Yes, it certainly sounds like a good idea to think it through together. Sebastian is probably the best person to offer some ideas, I've copied him in as well.

comment:4 in reply to: ↑ 3 Changed 8 months ago by saroyanm

Replying to kzar:

Replying to saroyanm:

In order to implement this issue, UI team needs to have ability to subscribe, or somehow be notified about the Blocked Request and Element Hiding Event for a specific tab.

So first of all have I got this right?

You want it so that when the user clicks to report an issue, a list of all requests that the main page (and any frames?) made, and a list of all filter hits both element hiding and request blocking for the page are included in the report data that's sent to us.

Right

My first concern is privacy, I assume we're very clear about all this data that we're sharing? I worry a full list of requests the page made could be quite intrusive.

The data can be viewed before sending by clicking on Show report data button which opens current overlay, before clicking Send Report.

Having look into the implementation, it seems like the only safe and easy way to do that currently is to pretend of being devtools and listen to the Add Record event, which seems to be the easiest way to implement this with the least code duplication, but this is still a Hack.

My second concern is performance, to do this without having the user somehow refresh the page we'd have to be recording all the filter hit information at all times. We are very careful not to do this for the developer tools pane for performance reasons, instead we only do it when the developers tool pane is open.
(With the legacy Firefox extension there was a hack which allowed us to record element hiding hits quite cheaply, but that's not the case on Chrome or with the Firefox WebExtension unfortunately.)

There were couple of suggestions/ideas how to do that:

  • Reload the page (which you mentioned and how currently specified)
  • Open page in the iframe in issue reporter page

Ideally we would be able to generalize the implementation, to avoid similar Hacks, but still before starting the implementation it definitely make sense to align with the platform team.

Yes, it certainly sounds like a good idea to think it through together. Sebastian is probably the best person to offer some ideas, I've copied him in as well.

comment:5 Changed 8 months ago by agiammarchi

There were couple of suggestions/ideas how to do that:

Three, actually:

  • open the page in a non active tab (preserve top/parent behavior) with tracking enabled and close it after N seconds (or onload)

Security concerns are legit, but shouldn't we report only things that have been filtered?

I don't think we'd be interested in other resources, unless we want to investigate if it's a third parts issue (but I don't think that's our responsibility) ... thoughts?

Last edited 8 months ago by agiammarchi (previous) (diff)

comment:6 Changed 8 months ago by greiner

RE: Privacy
I'd like to add that all we're trying to accomplish here is restore functionality that was already present in the legacy Firefox version.

RE: Performance
One misunderstanding I'd like to clarify is that (a) it's the issue reporter who'd register itself as a devtools panel and (b) it would only register itself as a devtools panel when the user opens it.

comment:7 follow-up: Changed 8 months ago by kzar

One misunderstanding I'd like to clarify is that (a) it's the issue reporter who'd register itself as a devtools panel and (b) it would only register itself as a devtools panel when the user opens it.

I'm glad to hear it, but in that case by that point the user reports a problem we've already missed the relevant filter hits and requests. So the plan is to also go with one of Manvel or Andrea's suggestions e.g. refreshing the website?

comment:8 in reply to: ↑ 7 Changed 8 months ago by greiner

Replying to kzar:

I'm glad to hear it, but in that case by that point the user reports a problem we've already missed the relevant filter hits and requests. So the plan is to also go with one of Manvel or Andrea's suggestions e.g. refreshing the website?

Exactly.

comment:9 Changed 8 months ago by kzar

Alright then I think we're on the same page.

I think Manvel's right about splitting off the request / filter hit watching logic into a a separate API which can be used by both our devtools pane and the issue reporter. I'll open a blocking issue for that tomorrow.

comment:10 Changed 8 months ago by kzar

  • Blocked By 6402 added

comment:11 Changed 8 months ago by sergz

  • Cc sergz added

comment:12 Changed 7 months ago by saroyanm

  • Description modified (diff)

updated the specs link

comment:13 Changed 7 months ago by saroyanm

  • Priority changed from Unknown to P2
  • Ready set

comment:14 Changed 7 months ago by saroyanm

  • Description modified (diff)

comment:15 Changed 7 months ago by saroyanm

  • Owner set to saroyanm

comment:16 Changed 6 months ago by saroyanm

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:17 Changed 6 months ago by saroyanm

  • Blocked By 6582 added

comment:18 Changed 6 months ago by sebastian

  • Blocked By 6582 removed

comment:19 Changed 5 months ago by saroyanm

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:20 Changed 5 months ago by abpbot

A commit referencing this issue has landed:
Issue 6386 - Add Requests and Filters data to the Report data

comment:21 Changed 5 months ago by greiner

See also #5892.

comment:22 Changed 3 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. This appears to be working as expected.

ABP 3.1.0.2069
Chrome 67 / 64 / 49 / Windows 7
Firefox 60 / 55 / 51 / Windows 7
Opera 52 / 45 / 38 / Windows 7

Note: See TracTickets for help on using tickets.