For #26224 - New TCP CFR popup

[Mugurell]

Pull Request checklist

• [x] Tests: This PR includes thorough tests or an explanation of why it does not
• [x] Screenshots in Jira: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
• [x] Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

QA

• [x] QA Needed

To download an APK when reviewing a PR (after all CI tasks finished running):

1. Click on Checks at the top of the PR page.
2. Click on the firefoxci-taskcluster group on the left to expand all tasks.
3. Click on the build-debug task.
4. Click on View task in Taskcluster in the new DETAILS section.
5. The APK links should be on the right side of the screen, named for each CPU architecture.

GitHub Automation

Fixes #26224

[Mugurell]

DATA REVIEW REQUEST

1. What questions will you answer with this data?

This data will help us understand how users interact with the Total Cookie Protection Contextual Feature Recommendation Popup.

2. Why does Mozilla need to answer these questions? Are there benefits for users? Do we need this information to address product or business requirements?

To better understand the usefulness of new features.

3. What alternative methods did you consider to answer these questions? Why were they not sufficient?

There are no other.

4. Can current instrumentation answer these questions?

No.

5. List all proposed measurements and indicate the category of data collection for each measurement, using the Firefox data collection categories found on the Mozilla wiki.

Measurement Name	Measurement Description	Data Collection Category	Tracking Bug
tracking_protection.tcp_cfr_shown	The Total Cookie Protection CFR was shown to the user.	Category 2 - interaction	https://github.com/mozilla-mobile/fenix/issues/26224
tracking_protection.tcp_cfr_implicit_dismissal	The Total Cookie Protection CFR was dismissed by the user by interacting with the outside of the popup.	Category 2 - interaction	https://github.com/mozilla-mobile/fenix/issues/26224
tracking_protection.tcp_cfr_explicit_dismissal	The Total Cookie Protection CFR was dismissed by the user by clicking on the "X" button to close the popup.	Category 2 - interaction	https://github.com/mozilla-mobile/fenix/issues/26224
tracking_protection.tcp_sumo_link_clicked	A user clicked the link in the Total Cookie Protection CFR to open a new SUMO tab detailing the Total Cookie Protection feature.	Category 2 - interaction	https://github.com/mozilla-mobile/fenix/issues/26224

6. Please provide a link to the documentation for this data collection which describes the ultimate data set in a public, complete, and accurate way.

This collection is Glean so is documented in the Glean Dictionary.

7. How long will this data be collected?

Up until version '119', with the option to renew at that point.

8. What populations will you measure?

All channels, countries, and locales. No filters.

9. If this data collection is default on, what is the opt-out mechanism for users?

These collections are Glean. The opt-out can be found in the product's preferences.

10. Please provide a general description of how you will analyze this data.

Glean and Amplitude.


11. Where do you intend to share the results of your analysis?

Only on Glean, Amplitude, and with mobile teams.


12. Is there a third-party tool (i.e. not Glean or Telemetry) that you are proposing to use for this data collection?

No.

[rocketsroger]

DATA REVIEW REQUEST

1. What questions will you answer with this data?

This data will help us understand how users interact with the Total Cookie Protection Contextual Feature Recommendation Popup.

2. Why does Mozilla need to answer these questions? Are there benefits for users?
   Do we need this information to address product or business requirements?

To better understand the usefulness of new features.

3. What alternative methods did you consider to answer these questions?
   Why were they not sufficient?

There are no other.

4. Can current instrumentation answer these questions?

No.

5. List all proposed measurements and indicate the category of data collection for each
   measurement, using the Firefox data collection categories found on the Mozilla wiki.

Measurement Name Measurement Description Data Collection Category Tracking Bug tracking_protection.tcp_cfr_shown The Total Cookie Protection CFR was shown to the user. Category 2 - interaction #26224 tracking_protection.tcp_cfr_implicit_dismissal The Total Cookie Protection CFR was dismissed by the user by interacting with the outside of the popup. Category 2 - interaction #26224 tracking_protection.tcp_cfr_explicit_dismissal The Total Cookie Protection CFR was dismissed by the user by clicking on the "X" button to close the popup. Category 2 - interaction #26224 tracking_protection.tcp_sumo_link_clicked A user clicked the link in the Total Cookie Protection CFR to open a new SUMO tab detailing the Total Cookie Protection feature. Category 2 - interaction #26224

6. Please provide a link to the documentation for this data collection which
   describes the ultimate data set in a public, complete, and accurate way.

This collection is Glean so is documented in the Glean Dictionary.

7. How long will this data be collected?

Up until version '119', with the option to renew at that point.

8. What populations will you measure?

All channels, countries, and locales. No filters.

9. If this data collection is default on, what is the opt-out mechanism for users?

These collections are Glean. The opt-out can be found in the product's preferences.

10. Please provide a general description of how you will analyze this data.

Glean and Amplitude.


11. Where do you intend to share the results of your analysis?

Only on Glean, Amplitude, and with mobile teams.


12. Is there a third-party tool (i.e. not Glean or Telemetry) that you
    are proposing to use for this data collection?

No.

Data Review

1. Is there or will there be documentation that describes the schema for the ultimate data set in a public, complete, and accurate way?

Yes, through the metrics.yaml file and the Glean Dictionary

2. Is there a control mechanism that allows the user to turn the data collection on and off?

Yes, through the "Send Usage Data" preference in the application settings

3. If the request is for permanent data collection, is there someone who will monitor the data over time?

N/A, collection set to end or be renewed by version 119

4. Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 2, Interaction data

5. Is the data collection request for default-on or default-off?

default-on

6. Does the instrumentation include the addition of any new identifiers?

No

7. Is the data collection covered by the existing Firefox privacy notice?

Yes

8. Does the data collection use a third-party collection tool?

No

Result

data-review+

[mergify[bot]]

This pull request has conflicts when rebasing. Could you fix it @Mugurell? 🙏

[mavduevskiy]

Could we rebase it, so the first commit which we merged in another PR, wouldn't be present here as well?

[Mugurell]

Adding @Amejia481 as a reviewer for the second commit which makes this CFR controllable through Nimbus - browserscreen.cfrs-enabled.tcp.

[mavduevskiy]

@Mugurell When you have time, could you take a look at this discussion🙏

[Mugurell]

@Mugurell When you have time, could you take a look at this discussion🙏

Responded in that thread.

[mergify[bot]]

This pull request has conflicts when rebasing. Could you fix it @Mugurell? 🙏

[Mugurell]

Oh, one more thing. When going through review cycles, it's very convenient if new patches are added as extra commits, so I could just review the small patch that was made. There might be a lot of them in the end, and then rebasing it and collapsing into one would be the way to go (github squashes comments into one string as well, and it becomes unreadable)

I remember asking @Amejia481 about this some time ago who said that squashed commits are okay. Looking through the commit list I think the general consensus is to avoid separate fixup issues. Separate fixup commits to me would break the entire commits structure and otherwise just pollute the git history with commits having no real use on heir own - in this case I made an effort to split the work in multiple commits that revolve around a specific usecase and then an effort to keep the commits clean following review. The end result should paint an accurate portrait of what was needed to resolve a particular issue. Later commits that change the logic or update documentation following review would mean the below commits are obsolete - when checking the history on how something was resolved we would only see a small part and would need to continue to investigate to get the entire picture. So avoiding one small effort in review would mean many bigger efforts later.

[Amejia481]

👋🏽 I agree with @Mugurell about having a single commit before merging (when ever it is possible) per feature. In term of reviews, but I think it's OK to push multiple commits to indicate new changes (as long as we squash before merging), there are also other ways to avoid that, in my case I rely on the GitHub UI for telling me what have been changed, since the last push you can read more about the feature here https://github.blog/2019-07-01-mark-files-as-viewed/, either way I think it's OK, so far I don't think we have been fixed on any specific approach.

Some examples on this PR:

[mergify[bot]]

This pull request has conflicts when rebasing. Could you fix it @Mugurell? 🙏

[Amejia481]

I think we just have to chat with the reviewer to know which method is easier :) I think it's just a matter of preference!

[gabrielluong]

Looking good!

It would be safer if wrong filtering would not only trigger in manual testing, but in the test as well – it looks like we have a test that does exactly that:GIVEN the TCP CFR should be shown WHEN the current tab is fully loaded THEN the TCP CFR is only shown once, but logging and manual testing might be enough.

Oh, one more thing. When going through review cycles, it's very convenient if new patches are added as extra commits, so I could just review the small patch that was made. There might be a lot of them in the end, and then rebasing it and collapsing into one would be the way to go (github squashes comments into one string as well, and it becomes unreadable)

@mavduevskiy I would actually suggest not doing fixup commits for review comments. When we move towards landing to mozilla-central, we will actually lose the ability to squash commits since we take more of a stack based approach to patches. See https://moz-conduit.readthedocs.io/en/latest/lando-user.html. In a review queue in phabricator, you will typically see multiple commits named Part 1, 2, 3, etc. If a problem was pointed out in Part 1, it would be expected that Part 1 would be amended since the changes in a commit should address any problems that are pointed instead of creating a Part 4 that might create linting changes for instance. The point would that a reader of Part 1 should see all the necessary changes to reflect the issue it is trying to resolve.

For that reasoning, I would actually suggest switching towards amending your commits to align with the rest of organization.

[mergify[bot]]

This pull request has conflicts when rebasing. Could you fix it @Mugurell? 🙏

[Mugurell]

Updated the patch to use the new Nimbus configuration from https://github.com/mozilla-mobile/fenix/pull/26616.

[mergify[bot]]

This pull request has conflicts when rebasing. Could you fix it @Mugurell? 🙏

[mergify[bot]]

This pull request has conflicts when rebasing. Could you fix it @Mugurell? 🙏