Closed Bug 1399987 Opened 7 years ago Closed 6 years ago

Report what tracking protection list (if any) was in use, and whether mixed content was blocked, when reporting new site issue

Categories

(Web Compatibility :: Tooling & Investigations, enhancement, P1)

Product:
Web Compatibility
Component:
Tooling & Investigations
Type:
enhancement

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: miketaylr, Assigned: miketaylr)

References

Details

Attachments

(1 file)

Bug 1399987. Send tracking and mixed content blocking status as part of browser details.
59 bytes, text/x-review-board-request
Gijs
: review+
Details 
Then we can add a label for it.
Since we'd be passing this flag into /issues/new?, we could potentially display a message to the user as well.
Note: it's not enough to just check pref status, as the user may have whitelisted a site with the pref enabled. We can figure that else here tho: https://searchfox.org/mozilla-central/search?q=useTrackingProtection&case=true&path=
Priority: -- → P2
Changing the scope slightly here because it seems useful.
Assignee: nobody → miket
Priority: P2 → P1
Summary: Tell if user have tracking protection enabled when reporting new site issue → Report if user have tracking or mixed content was blocked when reporting new site issue
I've got a WIP patch based on Bug 1399985, so let's land that first.
Depends on: 1399985
Comment on attachment 8970239 [details]
Bug 1399987. Send tracking and mixed content blocking status as part of browser details.

https://reviewboard.mozilla.org/r/239036/#review244830

Tentative r=me. Do we want to also include the specific TP list if it's one of the builtin ones?
Attachment #8970239 - Flags: review?(gijskruitbosch+bugs) → review+
(In reply to :Gijs (he/him) from comment #6)
> Comment on attachment 8970239 [details]
> Bug 1399987. Send tracking and mixed content blocking status as part of
> browser details.
> 
> https://reviewboard.mozilla.org/r/239036/#review244830
> 
> Tentative r=me. Do we want to also include the specific TP list if it's one
> of the builtin ones?

Oh yeah... I think at least knowing "basic" vs "strict" would be helpful for us. I have a 1:1 with Panos today, I'll ask him for some pointers. ^__^
(I think maybe mozreview doesn't ping on r+'d... would you mind taking a look at https://reviewboard.mozilla.org/r/239036/diff/1-2/ please? Thanks!)
Flags: needinfo?(gijskruitbosch+bugs)
Comment on attachment 8970239 [details]
Bug 1399987. Send tracking and mixed content blocking status as part of browser details.

https://reviewboard.mozilla.org/r/239036/#review246926

::: browser/extensions/webcompat-reporter/content/WebCompatReporter.jsm:109
(Diff revisions 1 - 2)
>  
>      // Grab the relevant tab environment details that might change per site
>      details["mixed active content blocked"] = tabData.hasMixedActiveContentBlocked;
>      details["mixed passive content blocked"] = tabData.hasMixedDisplayContentBlocked;
> -    details["tracking content blocked"] = tabData.useTrackingProtection;
> +    details["tracking content blocked"] = tabData.useTrackingProtection ?
> +      `true (${details.blockList})` : false;

Nit: Bit odd that this is a string in some cases, and a bool in others, but as long as the ingesting location copes, I guess it doesn't matter...

::: browser/extensions/webcompat-reporter/content/WebCompatReporter.jsm:122
(Diff revisions 1 - 2)
>  
>      if (details["gfx.webrender.all"] || details["gfx.webrender.enabled"]) {
>        params.append("label", "type-webrender-enabled");
>      }
>  
> +    if (details["tracking content blocked"]) {

Seems safer to just use  `tabData.useTrackingProtection` for this condition.
(In reply to Mike Taylor [:miketaylr] from comment #9)
> (I think maybe mozreview doesn't ping on r+'d...

It doesn't, no - it assumes r+ carries over. If you want another review you would normally re-request review in bugzilla. Anyway, this also worked. :-)
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to :Gijs (he/him; out until May 8) from comment #10)
> Comment on attachment 8970239 [details]
> Bug 1399987. Send tracking and mixed content blocking status as part of
> browser details.
> 
> https://reviewboard.mozilla.org/r/239036/#review246926
> 
> ::: browser/extensions/webcompat-reporter/content/WebCompatReporter.jsm:109
> (Diff revisions 1 - 2)
> >  
> >      // Grab the relevant tab environment details that might change per site
> >      details["mixed active content blocked"] = tabData.hasMixedActiveContentBlocked;
> >      details["mixed passive content blocked"] = tabData.hasMixedDisplayContentBlocked;
> > -    details["tracking content blocked"] = tabData.useTrackingProtection;
> > +    details["tracking content blocked"] = tabData.useTrackingProtection ?
> > +      `true (${details.blockList})` : false;
> 
> Nit: Bit odd that this is a string in some cases, and a bool in others, but
> as long as the ingesting location copes, I guess it doesn't matter...

Ah true. Let's make it stringy everywhere.
> 
> ::: browser/extensions/webcompat-reporter/content/WebCompatReporter.jsm:122
> (Diff revisions 1 - 2)
> >  
> >      if (details["gfx.webrender.all"] || details["gfx.webrender.enabled"]) {
> >        params.append("label", "type-webrender-enabled");
> >      }
> >  
> > +    if (details["tracking content blocked"]) {
> 
> Seems safer to just use  `tabData.useTrackingProtection` for this condition.

Cool.

Thanks! Will address nits and land this post-soft code freeze (May 7).
Pushed by mitaylor@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3f48d59f9c4e
Send tracking and mixed content blocking status as part of browser details. r=Gijs
https://hg.mozilla.org/mozilla-central/rev/3f48d59f9c4e
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Summary: Report if user have tracking or mixed content was blocked when reporting new site issue → Report what tracking protection list (if any) was in use, and whether mixed content was blocked, when reporting new site issue
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: