Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling ReportBrokenSiteShown message #1167

Merged

Conversation

mgurgel
Copy link
Collaborator

@mgurgel mgurgel commented Jan 16, 2025

Required:

Task/Issue URL: https://app.asana.com/0/1206594217596623/1209173355503842/f
iOS PR: duckduckgo/iOS#3822
macOS PR: duckduckgo/macos-browser#3744
What kind of version bump will this require?: Minor

Description:

Implements Privacy Dashboard message ReportBrokenSiteShown which will trigger a pixel on macOS and iOS

This PR contains a fix for #1160 which is why it is stacked on it. Please let me know if this is not the best way to have this code reviewed.

Steps to test this PR:

  1. Invoke the breakage form in the two possible ways:
    Open the Privacy Dashboard and click on "Report a problem with this site"
    Open the app menu ••• and click on "Report Broken Site” (iOS)

  2. Confirm that the pixel m_report-broken-site_shown was fired

  3. Submit a report

  4. Confirm that the pixels epbf (iOS) / epbf_macos_desktop (macOS) and m_report-broken-site_sent were fired

OS Testing:

  • iOS 14
  • iOS 15
  • iOS 16
  • macOS 10.15
  • macOS 11
  • macOS 12

Internal references:

Software Engineering Expectations
Technical Design Template

@@ -368,10 +368,13 @@ extension PrivacyDashboardController: PrivacyDashboardUserScriptDelegate {
}

func userScriptDidRequestShowReportBrokenSite(_ userScript: PrivacyDashboardUserScript) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a legacy message that will isn’t currently being fired and is likely to be removed in the future.

@mgurgel mgurgel force-pushed the mgurgel/improved-breakage-form-pixel branch from 6a2da92 to f85a419 Compare January 17, 2025 14:37
@mgurgel mgurgel marked this pull request as ready for review January 17, 2025 15:16
@afterxleep afterxleep self-requested a review January 17, 2025 16:33
Copy link
Collaborator

@afterxleep afterxleep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.- Both iOS and macOS PR working as described.

mgurgel added a commit to duckduckgo/macos-browser that referenced this pull request Jan 17, 2025
Task/Issue URL:
https://app.asana.com/0/1206594217596623/1209173355503842/f

**Description**:

Set ups Privacy Dashboard EventMapping as to align with iOS. Each mapped
event fires a pixel when invoked by the Privacy Dashboard. See
duckduckgo/BrowserServicesKit#1167

Note: not all events are currently used by the Dashboard. Some are being
deprecated, which is why they might not be fired at all even though they
still exist in the mapping.

**Steps to test this PR**:
1. Invoke the breakage form in the two possible ways:
Open the Privacy Dashboard and click on "Report a problem with this
site"
Open the app menu ••• and click on "Report Broken Site” (iOS)

2. Confirm that the pixel `m_report-broken-site_shown` was fired
3. Submit a report
4. Confirm that the pixels `epbf_macos_desktop` and
`m_report-broken-site_sent` were fired
mgurgel added a commit to duckduckgo/iOS that referenced this pull request Jan 17, 2025
<!--
Note: This checklist is a reminder of our shared engineering
expectations. Feel free to change it, although assigning a GitHub
reviewer and the items in bold are required.

⚠️ If you're an external contributor, please file an issue first before
working on a PR, as we can't guarantee that we will accept your changes
if they haven't been discussed ahead of time. Thanks!
-->

Task/Issue URL:
https://app.asana.com/0/1206594217596623/1209173355503842/f

**Description**:

Removes automatic firing of `BrokenSiteReportShown` pixel to align with
macOS. The firing of that pixel is now actively requested by the Privacy
Dashboard (see
duckduckgo/BrowserServicesKit#1167)

This PR contains a fix for on
#3802 which is why it is stacked
on it. Please let me know if this is not the best way to have this code
reviewed.

**Steps to test this PR**:

1. Invoke the breakage form in the two possible ways:
Open the Privacy Dashboard and click on "Report a problem with this
site"
Open the app menu ••• and click on "Report Broken Site” (iOS)

2. Confirm that the pixel `m_report-broken-site_shown` was fired
3. Submit a report
4. Confirm that the pixels `epbf` and `m_report-broken-site_sent` were
fired

**Definition of Done (Internal Only)**:

* [ ] Does this PR satisfy our [Definition of
Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)?

**Copy Testing**:

* [ ] Use of correct apostrophes in new copy, ie `’` rather than `’`

**Orientation Testing**:

* [ ] Portrait
* [ ] Landscape

**Device Testing**:

* [ ] iPhone SE (1st Gen)
* [ ] iPhone 8
* [ ] iPhone X
* [ ] iPhone 14 Pro
* [ ] iPad

**OS Testing**:

* [ ] iOS 15
* [ ] iOS 16
* [ ] iOS 17

**Theme Testing**:

* [ ] Light theme
* [ ] Dark theme

—
###### Internal references:
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)

---------

Co-authored-by: Jacek Łyp <[email protected]>
@mgurgel mgurgel merged commit ecd7c2d into mgurgel/improved-breakage-form Jan 17, 2025
8 checks passed
@mgurgel mgurgel deleted the mgurgel/improved-breakage-form-pixel branch January 17, 2025 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants