jingram/cpm-stats-on-ntp: Update Protections Report on NTP to include CPM stats #2039
Conversation
✅ Deploy Preview for content-scope-scripts ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Temporary Branch UpdateThe temporary branch has been updated with the latest changes. Below are the details:
Please use the above install command to update to the latest version. |
[Beta] Generated file diffTime updated: Fri, 21 Nov 2025 23:03:21 GMT Apple
File has changed Integration
File has changed Windows
File has changed |
|
@jsoningram amazing. This approach is really good because when both platforms support it, we can delete a bunch of code and we're not left with overly-configurable components. Excellent work - sorry it creates a much larger PR, but this will pay off for certain! |
jsoningram
force-pushed
the
jingram/cpm-stats-on-ntp
branch
from
fc51da1 to
c8ad930
Compare
jsoningram
requested review from
a team,
mgurgel and
shakyShane
as code owners
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
jsoningram
force-pushed
the
jingram/cpm-stats-on-ntp
branch
from
7ea3c94 to
e596630
Compare
special-pages/pages/new-tab/app/components/TickPill/TickPill.module.css
Outdated
Show resolved
Hide resolved
special-pages/pages/new-tab/app/components/TickPill/TickPill.module.css
Outdated
Show resolved
Hide resolved
special-pages/pages/new-tab/app/activity/NormalizeDataProvider.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Really good work @jsoningram! I like how you've made it really easy to delete legacy code in the future :)
Mostly minor issues to flag:
- Potentially blocking translation key mismatch (see Cursor's comment)
- Commented-out line that should either be removed or explained
- Couple questions about the use of CSS variables, but only for consistency - no visible impact
- A tiny lint error for an unused Fire icon that's breaking CI
jsoningram
force-pushed
the
jingram/cpm-stats-on-ntp
branch
from
210d0ea to
66366e3
Compare
Bug: Tracker Status UI: No Message for Zero Trackers.
The TrackerStatus function computes totalTrackersPillText for the
zero-trackers case but never displays it. When totalTrackersBlocked ===
0, the condition {totalTrackersBlocked > 0 && <TickPill .../>} prevents
rendering any tracker status message. The legacy TrackerStatusLegacy
displays "No trackers blocked" or "No trackers found" in this scenario,
but the new implementation shows nothing, breaking the expected UI
behavior for sites with no blocked trackers.
#2039 (comment)
special-pages/pages/new-tab/app/protections/integrations-tests/protections.page.js
Outdated
Show resolved
Hide resolved
| - img | ||
| - heading /\\d+ – tyle prób śledzenia zablokowano/ [level=3] | ||
| - paragraph: Ostatnie 7 dni | ||
| - heading /\\d+ {count} – tyle prób śledzenia zablokowano/ [level=3] |
There was a problem hiding this comment.
Bug: Literal Placeholder Breaks Regex Pattern
The aria snapshot regex pattern /\\d+ {count} – tyle prób śledzenia zablokowano/ includes a literal {count} placeholder that should not appear in rendered text. This pattern would only match if the translation string wasn't properly interpolated, which suggests a test bug. The pattern should likely be /\\d+ .* – tyle prób śledzenia zablokowano/ or similar to match the actual rendered Polish text after interpolation.
| onKeyDown={handleKeyDown} | ||
| > | ||
| {children} | ||
| {isVisible && <div id={tooltipId} class={styles.tooltip} role="tooltip" dangerouslySetInnerHTML={{ __html: content }} />} |
There was a problem hiding this comment.
Bug: Tooltip uses dangerouslySetInnerHTML without sanitization
The Tooltip component accepts arbitrary HTML content via dangerouslySetInnerHTML. While the current usage passes a hardcoded translation string with minimal HTML (just a <span> tag), this pattern creates a security vulnerability for potential future misuse. The component should either sanitize the content or use a safer rendering approach like rendering JSX children with proper HTML escaping.
…ration (#2442) Task/Issue URL: https://app.asana.com/1/137249556945/task/1208880731399772 Tech Design URL: - https://app.asana.com/1/137249556945/project/481882893211075/task/1211708480324720 - https://app.asana.com/1/137249556945/project/481882893211075/task/1211627657442469 CC: ### Description This PR implements extends history data model with info if cookie popup was blocked on a domain and integrates with the FE to show the stats on a widget. Feature functionality is behind a feature flag with the exception of data model changes. Additionally the NTP changes are not yet released so for purpose of testing fetching custom C-S-S is required (details in test steps below) ### Testing Steps **Smoke history with the data base changes / current C-S-S* (both iOS and macOS)* 1. Build the app from the branch with disabled `newTabPageAutoconsentStats` feature flag, and current version of C-S-S. 2. Navigate to some pages and confirm if history works as expected. - Above needs to be performed for both iOS and macOS as the changes were impacting shared core data part for both) **Locally update the C-S-S to one supporting CPM widget** Use C-S-S from this PR -> duckduckgo/content-scope-scripts#2039 - Bind the latest version from branch via `npm i github:duckduckgo/content-scope-scripts#65866a0d18cbbb27e6ed07194dfa18b8dfd4b75d` (or check PR for latest) - Intall with `npm install` **Smoke test legacy NTP design with disabled feature flag** 1. Verify that `newTabPageAutoconsentStats` feature flag is disabled. 2. Open NTP and confirm that the "Protections Report" widget looks like the current production one (e.g. left side trackers blocked count and on details feed the tracker icons are displayed) <img width="528" height="383" alt="Screenshot 2025-11-19 at 18 58 47" src="https://github.com/user-attachments/assets/946a9a96-f191-4366-b278-3304c4b017bd" /> **Test update NTP design with enabled feature flag** 1. Enable `newTabPageAutoconsentStats` feature flag from the debug menu. 2. Perform some navigations that will result in trackers being blocked or cookie prompts being managed (to force NTP stats update). 4. Make sure at least 1 cookie popup has been managed. 5. Open NTP and confirm that the "Protections Report" widget has updated look (e.g. right side blocked cookie popup count and details feed lacks tracker icons but includes tag if cookie pop-up was blocked on domain) <img width="525" height="562" alt="Screenshot 2025-11-19 at 19 02 59" src="https://github.com/user-attachments/assets/e460624e-e165-4fd9-b3d5-53d81c1db387" /> **Test that disabling CPM hides NTP widget** 1. Confirm that the new cookie widget is being shown on NTP. 2. Open Settings -> Cookie Pop-up Protection and disable "Automatically handle cookie pop-ups" 4. Perform some navigations that will result in trackers being blocked or cookie prompts being managed (to force NTP stats update). 5. Open NTP and confirm that the "Protections Report" widget has updated look but the blocked cookie popups part is hidden. 6. Re-enable "Automatically handle cookie pop-ups" in settings 7. Perform some navigations. 8. Open NTP and confirm that the "Protections Report" widget has updated look and now the blocked cookie popups part is shown again. **Test that having 0 cookie popups blocked hides NTP widget** 1. Perform full data clearing via Fire Button. 2. Perform some navigations that will result in trackers being blocked but NOT cookie prompts being managed. 3. Open NTP and confirm that the "Protections Report" widget has updated look and only blocked trackers count is shown. 4. Perform a navigation to a website where cookie prompts is managed. 6. Open NTP and confirm that the "Protections Report" widget has updated look and the blocked cookie popups counter shows "1". ### Impact and Risks Medium: Could disrupt specific features or user flows #### What could go wrong? - Issues with "Protections Report" widget on NTP - Small change to Core Data data model but impacts shared part between and iOS and macOS, can potentiall result in broken or lost browsing history -Remaining part of functionality is hidden behind a feature flag and awaits future C-S-S release for data updates --- ###### Internal references: [Definition of Done](https://app.asana.com/0/1202500774821704/1207634633537039/f) | [Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) | [Tech Design Template](https://app.asana.com/0/59792373528535/184709971311943) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds cookie popup blocked tracking to history and integrates async Autoconsent usage stats into the New Tab Page, wiring events from user script to history, clearing on burn, and updating UI/data models accordingly. > > - **History (shared + macOS Core Data)**: > - Add `cookiePopupBlocked` to history models (`BrowsingHistory 2`, `History 7`) and `BrowsingHistoryEntryManagedObject`/`HistoryEntry`. > - Extend `HistoryCoordinator` with `cookiePopupBlocked(on:)` and `resetCookiePopupBlocked(for:tld:)` and persist flag; logging updated. > - **Autoconsent Stats (shared)**: > - Refactor `AutoconsentStats` to async API, add `statsUpdatePublisher` and `isEnabled()` callback; remove direct `FeatureFlagger` dep. > - Record clicks/time, fetch totals, clear stats; fire updates via Combine. > - **Tab Extensions**: > - `AutoconsentTabExtension` publishes managed events and records stats. > - `HistoryTabExtension` listens for managed events to mark `cookiePopupBlocked`. > - **New Tab Page integration**: > - Add `AutoconsentStats` dependency; extend Protections Report data with optional `totalCookiePopUpsBlocked` and respect feature/settings. > - Wire `NewTabPageProtectionsReportModel/Client` to stats (including live updates); include `cookiePopUpBlocked` in Recent Activity items. > - **Fire clearing**: > - Clear Autoconsent stats during burns; when clearing cookies without history, reset `cookiePopupBlocked` for affected domains. > - **App wiring (macOS)**: > - Instantiate `AutoconsentStats` with feature flag, pass to NTP coordinator and actions manager. > - **Removals/Cleanup**: > - Remove legacy `AutoconsentDailyStats` code, feature flag usage, and pixel (`popup-managed-count`). > - **iOS stubs**: > - Update `HistoryManager`/tests to include new cookie popup APIs. > - **Tests**: > - Add/extend unit and integration tests for history flagging, reset behavior, Autoconsent stats API, and NTP data paths. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit a8f631a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Dominik Kapusta <dkapusta@duckduckgo.com>
| await ntp.openPage({ additional: { ...defaultPageParams } }); | ||
| await ap.didRender(); | ||
| await ap.hidesCookiePopupIndicatorWhenNotBlocked(); | ||
| }); |
There was a problem hiding this comment.
Bug: Test uses legacy UI instead of new UI
The test hides cookie popup indicator when not blocked doesn't pass the cpm parameter, causing it to use the legacy UI where cookie popup indicators don't exist at all. The test passes because the indicator isn't visible in the legacy UI, but it's not actually testing the intended behavior of the new UI hiding the indicator when cookiePopUpBlocked is false. The test should pass cpm: 'true' in the openPage call to use the new UI and properly verify that items with cookiePopUpBlocked: false don't display the cookie popup indicator.
There was a problem hiding this comment.
Bug: Shared HistoryItem uses wrong styles for legacy UI
The HistoryItem component always uses styles from Activity.module.css (lines 107-117), but it's shared by both HistoryItems and HistoryItemsLegacy. When HistoryItemsLegacy renders this component, the history items will have incorrect styling because they should use stylesLegacy from ActivityLegacy.module.css instead. The parent HistoryItemsLegacy correctly uses stylesLegacy.history for the container, but the child items use the wrong stylesheet, creating a styling mismatch in the legacy UI.
special-pages/pages/new-tab/app/activity/components/HistoryItems.js#L106-L117
jsoningram
force-pushed
the
jingram/cpm-stats-on-ntp
branch
from
9cd24f0 to
38cdcef
Compare
There was a problem hiding this comment.
Bug: HistoryItem uses wrong styles for legacy UI
The HistoryItem component always uses styles from Activity.module.css (via the styles import), but it's called by both HistoryItems and HistoryItemsLegacy. When HistoryItemsLegacy calls HistoryItem, the history items will have incorrect styling because they should use stylesLegacy instead. The parent components correctly use different style imports (styles.history vs stylesLegacy.history), but the shared child component doesn't receive or use the appropriate styles, causing visual inconsistencies in the legacy UI.
special-pages/pages/new-tab/app/activity/components/HistoryItems.js#L106-L117
| <Trans str={t('activity_countBlockedAdsAndTrackersPlural', { count: String(status.value.totalCount) })} values={{}} /> | ||
| ) : ( | ||
| <Trans str={t('activity_countBlockedPlural', { count: String(status.value.totalCount) })} values={{}} /> | ||
| <Trans str={t('activity_countBlockedPluralLegacy', { count: String(status.value.totalCount) })} values={{}} /> |
There was a problem hiding this comment.
Bug: Missing legacy translation string for ad blocking
The TrackerStatusLegacy component uses activity_countBlockedAdsAndTrackersPlural when ad blocking is enabled, but this translation key lacks a legacy variant. The non-ad-blocking path correctly uses activity_countBlockedPluralLegacy on line 332, creating an inconsistency. Either activity_countBlockedAdsAndTrackersPlural needs a legacy variant added to the strings file, or the code should use the existing stats_countBlockedAdsAndTrackersPluralLegacy key instead. This was flagged in the PR discussion but appears unresolved.
Include missing styles for infoIcon
Bug: Tracker Status UI: No Message for Zero Trackers.
The TrackerStatus function computes totalTrackersPillText for the
zero-trackers case but never displays it. When totalTrackersBlocked ===
0, the condition {totalTrackersBlocked > 0 && <TickPill .../>} prevents
rendering any tracker status message. The legacy TrackerStatusLegacy
displays "No trackers blocked" or "No trackers found" in this scenario,
but the new implementation shows nothing, breaking the expected UI
behavior for sites with no blocked trackers.
#2039 (comment)
…r messages
The TrackerStatus component always displays a checkmark icon in the
TickPill component, even when showing zero-tracker messages like "No
trackers blocked" or "No trackers found". The TickPill component has a
displayTick prop that defaults to true, but the code doesn't pass
displayTick={false} when totalTrackersBlocked === 0. This causes a
checkmark to appear next to messages indicating no trackers were found
or blocked, which is semantically incorrect and doesn't match the test
expectations.
Update strings to (further) clarify intent
Address screenshot issues
Details: [integration] › pages/new-tab/app/protections/integrations-tests/protections.spec.js:61:5 › protections report › displays cookie popup blocking stats when enabled and counts > 0
jsoningram
force-pushed
the
jingram/cpm-stats-on-ntp
branch
from
792836d to
ff9568f
Compare
There was a problem hiding this comment.
Bug: History items use wrong styles in legacy mode
The HistoryItem component is hardcoded to use styles (new styles) but is called by both HistoryItems and HistoryItemsLegacy. When HistoryItemsLegacy renders the legacy UI, the child HistoryItem components still use new styles (styles.historyItem, styles.historyLink, styles.time, styles.historyBtn) instead of legacy styles (stylesLegacy.*). This causes visual inconsistencies where the parent uses legacy styling but children use new styling. The component needs to accept a styles prop or be split into separate legacy and new versions like other components in this PR.
special-pages/pages/new-tab/app/activity/components/HistoryItems.js#L106-L117
3 participants
Asana Task/Github Issue: FE/XP: implement CPM stats in NTP
Figma: O-E CPM
Description
This PR introduces cookie pop-up blocking stats to the New Tab Page (NTP), along with a redesigned activity tracker details display. The changes include support for legacy platforms through derived feature flagging to ensure backwards compatibility during the rollout.
New UI Components
special-pages/pages/new-tab/app/components/TickPill/): A reusable pill component with optional checkmark icon for displaying status informationspecial-pages/pages/new-tab/app/components/Tooltip/): A tooltip component for displaying additional contextual informationLegacy Platform Support
To support platforms not yet ready for the new protections report, legacy versions of components were created:
ActivityItemLegacy: Legacy activity item componentTrackerStatusLegacy: Legacy tracker status display (icon-based)ProtectionsHeadingLegacy: Legacy protections headingActivityLegacy.module.css: Legacy styling for activity componentsPrivacyStatsLegacy.module.css: Legacy styling for privacy statsComponents now accept a
shouldDisplayLegacyActivityprop to toggle between new and legacy UI. This allows for gradual rollout across platforms. When all platforms are ready, we can remove the ‘legacy’ support files. Search for@todo legacyProtectionsto locate usage. NOTE Also see activity_ and stats_ translation strings for legacy supportTesting Steps
Append the following query parameters to the Base URL to simulate…
?cpm=none?stats=none(sets trackers blocked to 0, hiding summary and details)?cpm=undefinedChecklist
Please tick all that apply:
Note
Adds CPM (cookie pop-up) stats to the Protections Report with a new UI (pills + tooltip), updates Activity to show tracker/cookie indicators, and introduces legacy fallbacks, schema/types, tests, and i18n updates.
totalCookiePopUpsBlockedwith new heading layout and info tooltip.TickPillpills and optional “Cookie pop-up blocked” per item.shouldDisplayLegacyActivity), legacy components/styles.cookiePopUpBlockedin data context.components/TickPill/(pill with optional check),components/Tooltip/(hover/focus tooltip), newFireIcon.protections-data.jsonandactivity.json; regenerate TS types to include CPM and cookie flags.Written by Cursor Bugbot for commit 7bd10fc. This will update automatically on new commits. Configure here.