Files
obsidian-vault/work/wiki/apple-browsers/chat-path-onboarding-tracker-blocking.md
T

675 lines
40 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# iOS: Chat-Path Onboarding — Tracker Blocking Demo
> **Asana:** [Ship Review task 1214147157456478](https://app.asana.com/1/137249556945/task/1214147157456478)
> **Branch stack:** `demo-tracker-blocking-onboarding` → `ui-polish` → `chat-path-dialog-polish` → `sr-feedback` → `uti-flow`
> **Due:** 2026-05-15
---
## PR Stack
| PR | Branch | Review status |
|---|---|---|
| [#4544](https://github.com/duckduckgo/apple-browsers/pull/4544) | `demo-tracker-blocking-onboarding` | Open — review required (Alessandro + Rachel left comments) |
| [#4584](https://github.com/duckduckgo/apple-browsers/pull/4584) | `demo-tracker-blocking-onboarding-eoj-dax-animation` | **Merged** ✅ 2026-05-18 |
| [#4591](https://github.com/duckduckgo/apple-browsers/pull/4591) | `demo-tracker-blocking-onboarding-ui-polish` | **Merged** ✅ 2026-05-15 |
| [#4664](https://github.com/duckduckgo/apple-browsers/pull/4664) | `demo-tracker-blocking-onboarding-chat-path-dialog-polish` | **Merged** ✅ 2026-05-15 |
| [#4855](https://github.com/duckduckgo/apple-browsers/pull/4855) | `demo-tracker-blocking-onboarding-sr-feedback` | Open — no review yet |
| [#4668](https://github.com/duckduckgo/apple-browsers/pull/4668) | `demo-tracker-blocking-onboarding-uti-flow` | Open — no review yet |
---
## Open Items
---
### ✅ SR — GJ2/ Remove X button from chat-path dialogs
**Source:** [Gary Apr 29](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214409112744728)
**Fix:** X (manual dismiss) button removed from "Try visiting a site!", trackers-blocked, and "Oh, before I forget…" dialogs on the chat path.
**State:** Done. Committed in PR #4664.
---
### ✅ SR — GJ4/ Copy: "Next, try visiting a site!" after trying AI
**Source:** [Gary Apr 29](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214409112744728)
**Fix:** "Try visiting a site!" dialog title updated to "Next, try visiting a site!" for chat-path context.
**State:** Done. Committed in PR #4664.
---
### 🟠 SR — GJ3/ Hide address bar and toolbar for "Try visiting a site!" step (chat-path only)
**Source:** [Gary Apr 29](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214409112744728), confirmed by [Gary May 7](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214615822543276) and [Costas May 8](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214627423981810): hide address bar + toolbar for AI-chat flow only so user is forced to choose from presets. Without this: user could type a search (repeating the same dialog) or switch to Duck.ai. Asana task: [1214828145455337](https://app.asana.com/1/137249556945/project/1202406491309510/task/1214828145455337) "Try Visiting a Site: Remove Browser Toolbar".
**Fix:** `chromeDelegate?.setBarsHidden(true)` on show, `setBarsHidden(false)` on dismiss. Tracked as PR #4855 — Alessandro (line 477) item below.
**State:** `sr-feedback / uncommitted` — in working tree, NOT committed.
---
### ✅ SR — Design: "Try a Search or AI chat" dialog — Remove pointer/tail
**Source:** [Asana task 1214846580751519](https://app.asana.com/1/137249556945/project/1202406491309510/task/1214846580751519) (Ship Review: Design, May 15). Remove the bubble pointer/tail from the bottom of the "Try a Search or AI chat" dialog container since Dax is not present in this dialog.
**State:** ✅ Done. `tailPosition: nil` in `RebrandedContextualOnboardingDialogs+TrySearch.swift`. Committed in merged PR.
---
### ✅ SR — Design: "You've Got This!" dialog — Remove pointer/tail
**Source:** [Asana task 1214828145455345](https://app.asana.com/1/137249556945/project/1202406491309510/task/1214828145455345) (Ship Review: Design, May 15). Remove the bubble pointer/tail from the bottom of the "You've Got This!" EOJ dialog container since Dax is not present.
**State:** ✅ Done. `tailPosition: nil` in `RebrandedContextualOnboardingDialogs+EndOfJourney.swift`. In uncommitted working tree on `demo-tracker-blocking-onboarding`.
---
### 🟡 SR — Design: "Try Visiting a Site" screen — Add background
**Source:** [Asana task 1214828145455342](https://app.asana.com/1/137249556945/project/1202406491309510/task/1214828145455342) (Ship Review: Design, May 15). After hiding address bar + toolbar (GJ3), the screen is very empty. Gary suggests switching in an existing background for delight. **Not a 100% blocker for the experiment.**
**State:** 🟡 Nice-to-have. Not implemented. Not in any PR.
---
### 🟠 SR — GJ7/ Exclude sync-restore / returning users from Duck.ai experiment step
**Source:** [Alessandro May 7](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214595287457513): if user restored data from sync, Dax dialogs are disabled — enrolling them would break the flow. Alex confirmed addressed in May 13 build comment ("currently commented out in PR").
**Fix:** Uncommented `guard case .introDialog(isReturningUser: false) = introSteps.first` in `insertExperimentStepIfNeeded()`. Tracked as PR #4855 — Bugbot "Returning-user guard commented out" item below.
**State:** `sr-feedback / uncommitted` — in working tree, NOT committed.
---
### 🟠 SR — GJ8/ Change "Duck.ai" to "Ask AI" on toggle choice screen (onboarding only)
**Source:** [Gary May 6](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214592278234254): "Let's also change 'Duck.ai' to 'Ask AI' in the toggle screen in onboarding. Only change it in onboarding though, not the standard toggle."
**Fix:** DuckAI query toggle label updated to "Ask AI" in `OnboardingView+DuckAIExperimentSearchContent.swift` and `UserText.swift`. Tracked as PR #4855 — Alessandro (UserText.swift:2375) item below.
⚠️ Alessandro's review comment says "Ask.ai" (with dot) vs GJ8 "Ask AI" (no dot) — needs product confirmation.
**State:** `sr-feedback / uncommitted` — in working tree, NOT committed.
---
### ✅ SR — GJ6b/ "You've got this" EOJ fails after tapping "Got it" on tracker blocking step
**Source:** [Gary Apr 29](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214409112744728): "This dialogue fails to appear after tapping 'Got it' on the tracker blocking step. As a result, we also skip the subscription upsell dialogue."
**Fix:** `if contextualOnboardingLogic.chatPathPhase == .trackerToEOJ { delegate?.tabDidRequestNewTab(self) }` added to `didTapDismissContextualOnboardingAction` in `TabViewController.swift`.
**State:** ✅ Fix landed in `alex/demo-tracker-blocking-onboarding` via PR #4584 merge (2026-05-18). GitHub thread on #4544 still open — needs resolving.
---
### 🔴 SR — GJ1/ Animated toggle thumbnail
**Source:** [Gary Apr 29 + May 6](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214592278234254) confirmed in scope. [Costas May 8](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214627423981810): task [1214627423981798](https://app.asana.com/1/137249556945/project/1212535659026889/task/1214627423981798) created for animated assets. If not in time, defer to follow-up project. [Alessandro May 10](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214683268207876): nice-to-have, defer if needed.
**State:** 🔴 NOT implemented. Nice-to-have — defer to follow-up if time doesn't allow.
---
### 🔴 SR — GJ6a/ UTI: keyboard appears after choosing AI, hiding response and fire button
**Source:** [Gary Apr 29](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214409112744728). [Pete May 7](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214600831548835): UTI experiment participants will be excluded from this experiment; UTI issues can be fixed in follow-up before June 1. [Alessandro May 7](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214595287457513): follow-up task [1214592943696234](https://app.asana.com/1/137249556945/project/1212535659026889/task/1214592943696234).
**State:** 🔴 NOT implemented here. Scoped to separate follow-up project (UTI + experiment winner work).
---
### 🔴 SR — GJ6/ UTI new breakage (visit-site dialog, bar state)
**Source:** [Alex May 13](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214741014878191): "new breakage since my last fixes, will do next." Tracked in PR #4668.
**State:** 🔴 In progress. PR #4668 has open comment threads (see aataraxiaa + Bugbot items below).
---
### SR — GJ5/ Add new Dax brand assets to onboarding
**Source:** [Gary Apr 29](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214409112744728).
**State:** Deferred. Costas/Alessandro agreed: out of scope for this project, addressed in follow-up.
---
### SR — CB1/ Welcome screen copy update
**Source:** [Costas May 8](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214627423981810). [Alessandro May 10](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214683268207876): comes for free once task [1213967368170629](https://app.asana.com/1/137249556945/project/1142021229838617/task/1213967368170629) merges.
**State:** Covered by separate task. No action needed here.
---
### ✏️ COMMIT-1 — All working-tree changes on `sr-feedback` are uncommitted
**State:** All of the items below marked `sr-feedback / uncommitted` are local-only on `alex/demo-tracker-blocking-onboarding-sr-feedback`. None have been committed or pushed.
- `iOS/DuckDuckGo/MainViewController.swift`
- `iOS/DuckDuckGo/NewTabPageControllerDelegate.swift`
- `iOS/DuckDuckGo/NewTabPageViewController.swift`
- `iOS/DuckDuckGo/OnboardingFlow/LinearOnboarding/OnboardingIntroViewModel.swift` — also needs `!restorePromptHandler.isEligibleForRestorePrompt()` added (see PR #4855 Alessandro line 450)
- `iOS/DuckDuckGo/OnboardingView+DuckAIExperimentSearchContent.swift` — label currently "Ask AI", needs correction to **"Ask.ai"** (with dot)
- `iOS/DuckDuckGo/UserText.swift` + all 26 lproj files
**Action:** Fix "Ask.ai" copy + add `isEligibleForRestorePrompt` guard, then commit and push before #4855 can be reviewed.
> **Color legend:** 🟠 fix in working tree, uncommitted — 🟡 fix in another PR in stack, not merged to this one — 🔴 not fixed / blocked — ✅ done
---
## PR #4544 Comments
> Threads in GitHub order. Resolved threads grouped into tables.
### ✅ [Resolved] PR #4544 — Bugbots (outdated): hardcoded cohort / promo override / missing pixels / commented-out redirect
**Source:** Four Bugbot comments on early commits of PR #4544.
| Thread | Fix | Code state |
|---|---|---|
| Hardcoded `return .treatmentA` in `OnboardingIntroViewModel` | Removed in later commit on #4544 | ✅ Fixed (Bugbot commit is outdated) |
| `shouldDisplay` always returns `true` in `OnboardingSubscriptionPromotionHelper` | Reverted in commit `f8d5ba2` on #4544 | ✅ Fixed |
| Missing pixel definitions in `PixelEvent.swift` | Added in commit `ec42fd8` on #4544 | ✅ Fixed |
| `isChatPathSubscriptionPromo` commented out, variable unused in `RebrandedNewTabDaxDialogFactory` | Removed in commit `e682475` on #4544 | ✅ Fixed |
**State:** All four fixed in code. All four GitHub threads are **NOT resolved** (outdated Bugbot comments never closed). → Resolve all four threads.
---
### ✅ [Resolved] PR #4544 — Bugbot: Chat-path completion dialog shown twice via competing paths
**Source:** Bugbot on PR #4544, `MainViewController.swift:4711`
**Issue:** Two independent paths both call `presentChatPathOnboardingCompletionIfNeeded()``tabDidRequestNewTab` dispatch and a direct call — potentially showing the completion dialog twice.
**State:** `presentChatPathOnboardingCompletionIfNeeded()` has a guard on `chatPathPhase == .trackerToEOJ` — once the EOJ fires, phase advances and the second call is a no-op. GitHub thread is **[Resolved]**.
---
### ✅ [Resolved] PR #4544 — Bugbots + Alessandro (additional resolved threads)
All resolved on GitHub. Fixed in later commits on the `#4544` branch.
| Thread | Fix |
|---|---|
| `DefaultDaxDialogsSettings.chatPathPhase` missing `isChatFirstPath` guard | Added in later commit |
| Chat-path tests miss `isChatFirstPath` setup | Fixed in tests |
| Debug reset omits clearing `isChatFirstPath` flag | Fixed in `OnboardingDebugView` |
| Duplicated `chatPathPhase` logic across `DaxDialogs` + `ContextualDaxDialogsFactory` | Consolidated into `DaxDialogsSettings` |
| Alessandro: pixels `@rachelmcr` fyi (`PixelEvent.swift:268`) | Informational; no action required |
| Alessandro: tests replicating real `DaxDialogsSettings` logic | Simplified tests |
| `TrackerToEOJ` phase test also needs browsing dialog flag | Fixed in tests |
| Missing fallback when AI Chat disabled during chat path (`MainViewController.swift:4712`) | Fallback to `.final` added |
| `isChatPath` logic in `createFinalDialog` is unreachable | Dead branching removed |
---
### 🔴 PR #4544 — Alessandro (DaxDialogs.swift:125): `DaxDialogProvider` refactoring suggestion
**Source:** [Alessandro May 7 review on PR #4544](https://github.com/duckduckgo/apple-browsers/pull/4544), `DaxDialogs.swift:125`
**Issue:** Alessandro asked whether having a `DaxDialogProvider` that internally delegates to either `DaxDialogs` or a new `DuckAIDaxDialogs` would be a significant change — to separate responsibilities since the chat-path flow doesn't need search dialogs.
**State:** 🔴 [OPEN] on GitHub. Alessandro's own follow-up (May 10): "No time for this, let's re-assess once the experiment is live." → Deferred to post-experiment follow-up.
---
### 🟠 PR #4544 — Alessandro (ContextualOnboardingNewTabDialogFactoryTests.swift:172): Tests are for the legacy factory
**Source:** [Alessandro May 7 review on PR #4544](https://github.com/duckduckgo/apple-browsers/pull/4544), `ContextualOnboardingNewTabDialogFactoryTests.swift:172`
**Issue:** "Just bear in mind that these tests are for the legacy factory." Tests target `NewTabDaxDialogFactory`, not `RebrandedNewTabDaxDialogFactory` (active under rebranding flag).
**State:** 🟠 Partially addressed — `createFinalDialog` chat-path branching removed (both factories now symmetric), so the asymmetry this comment highlighted is gone. `createSubsequentDialog` chat-path logic still only tested via legacy factory. GitHub thread still open.
---
### ✅ PR #4544 — Bugbot (r3234446893): Dead `isChatPath` branch in `NewTabDaxDialogFactory.createFinalDialog`
**Source:** Bugbot on PR #4544, `NewTabDaxDialogFactory.swift:192` ([r3234446893](https://github.com/duckduckgo/apple-browsers/pull/4544#discussion_r3234446893))
**Issue:** `NewTabDaxDialogFactory.createFinalDialog` had chat-path conditional logic (`chatPathPhase == .trackerToEOJ && isAIChatEnabled`), but `RebrandedNewTabDaxDialogFactory.createFinalDialog` did not. `.final` is unreachable for chat-path + AI-enabled users (EOJ driven by `presentChatPathOnboardingCompletionIfNeeded`), so both branches were dead.
**Fix:** Removed `isChatPath` conditional from `NewTabDaxDialogFactory.createFinalDialog` — both factories now always use standard EOJ message + `daxDialogsEndOfJourneyNewTabUnique` pixel. Deleted test `testWhenChatPathEOJState_AndFinalDialogAppears_ThenFiresDuckAIExperimentFinalDialogPixel` (tested dead code); simplified remaining final-dialog tests.
**State:** ✅ Fixed and committed (2026-05-18). GitHub thread resolved on GitHub.
---
### ✅ PR #4544 — Bugbot (r3234645543) / SR GJ6b/: Chat-path EOJ not shown after tapping tracker dialog CTA
**Source:** [Bugbot comment r3234645543 on PR #4544](https://github.com/duckduckgo/apple-browsers/pull/4544#discussion_r3234645543), `TabViewController.swift:4214`. Also reported as SR GJ6b/ by [Gary Apr 29](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214409112744728) — same bug.
**Issue:** When user taps "Got it" on the tracker-blocked dialog, the code goes through `didTapDismissContextualOnboardingAction`, which does not call `tabDidRequestNewTab`. The EOJ new tab is never opened.
**Fix:** `if contextualOnboardingLogic.chatPathPhase == .trackerToEOJ { delegate?.tabDidRequestNewTab(self) }` in `didTapDismissContextualOnboardingAction` (`TabViewController.swift:4206`).
**State:** ✅ Fix landed via PR #4584 merge. GitHub thread **resolved** on GitHub.
---
### ✅ PR #4544 — Bugbot: Rebranded factory's final dialog missing chat-path branching (`RebrandedNewTabDaxDialogFactory.swift:202`)
**Source:** Bugbot on PR #4544, `RebrandedNewTabDaxDialogFactory.swift:202`
**Issue:** `NewTabDaxDialogFactory.createFinalDialog` had chat-path branching, `RebrandedNewTabDaxDialogFactory.createFinalDialog` did not. Both factories are selected by the `onboardingRebranding` feature flag.
**Fix:** Resolved by removing the dead branch from `NewTabDaxDialogFactory` (see r3234446893 entry above). Both factories now symmetric.
**State:** ✅ Fixed and committed (2026-05-18). GitHub thread resolved on GitHub.
---
### ✅ PR #4544 — Bugbot (r3257196584): Chat-path subscription promo unreachable after EOJ dismissal
**Source:** [Bugbot r3257196584 on PR #4544](https://github.com/duckduckgo/apple-browsers/pull/4544#discussion_r3257196584), `NewTabPageViewController.swift:355-374` and `DaxDialogs.swift:407-419`.
**Issue:** In `finishDismissal` (inside `showDuckAIOnboardingCompletionDialog`), `nextHomeScreenMessageNew()` was called while `browsingFinalDialogShown` was still `false`. `peekNextHomeScreenMessageExperiment()` took the chat-path branch (not the `finalDaxDialogSeen` branch) and returned `nil`. The else branch then called `daxDialogsManager.dismiss()` which atomically set `browsingFinalDialogShown = true` and `isDismissed = true`, making `isEnabled = false`. Any subsequent call returned `nil` unconditionally — subscription promo was unreachable.
**Fix:** Call `setFinalOnboardingDialogSeen()` before `nextHomeScreenMessageNew()` in `finishDismissal`. Added `setFinalOnboardingDialogSeen()` to `NewTabDialogSpecProvider` protocol; added stub to `MockNewTabDialogSpecProvider`.
**State:** ✅ Fixed and committed (2026-05-18). GitHub thread still **open** — needs resolving.
---
### ✅ PR #4544 — Bugbot (r3257070868): Unused `UserDefaultsPropertyWrapper.Key` cases
**Source:** [Bugbot comment r3257070868 on PR #4544](https://github.com/duckduckgo/apple-browsers/pull/4544#discussion_r3257070868), `UserDefaultsPropertyWrapper.swift:59-61`
**Issue:** `daxChatPathVisitSiteSeen` and `daxIsChatFirstPath` added to the `Key` enum but never used — `DefaultDaxDialogsSettings` stores these via `KeyValueStoring` with its own `ChatPathKey` constants.
**Fix:** Removed both dead enum cases from `UserDefaultsPropertyWrapper.Key`.
**State:** ✅ Fixed in `alex/demo-tracker-blocking-onboarding` working tree (2026-05-18). Not yet committed.
---
## PR #4591 Comments
> PR #4591 (`demo-tracker-blocking-onboarding-ui-polish`) — **Merged** ✅ 2026-05-15. Threads documented for reference.
### ✅ PR #4591 — Bugbot (SyncSettingsViewController.swift:738): Experiment pixel skips `.connect` receiver
**Source:** Bugbot on PR #4591, `SyncSettingsViewController.swift:738`
**State:** ✅ Acknowledged upstream issue (PR #4575). Not in scope. PR merged — thread irrelevant.
---
### ✅ [Resolved] PR #4591 — Bugbot (SyncSettingsViewController.swift:567): Missing pixel when `shouldShowSyncEnabled`
**Source:** Bugbot on PR #4591, `SyncSettingsViewController.swift:567`
**State:** ✅ [Resolved] on GitHub.
---
### ✅ PR #4591 — Bugbot (SyncSettingsViewController.swift:588): Missing experiment success pixel in `controllerDidCreateSyncAccount`
**Source:** Bugbot on PR #4591, `SyncSettingsViewController.swift:588`
**State:** ✅ Acknowledged upstream issue (PR #4575). Not in scope. PR merged — thread irrelevant.
---
### ✅ PR #4591 — Bugbot (MainViewController.swift): Removed `performCancel` may leave omnibar in editing state
**Source:** Bugbot on PR #4591, `MainViewController.swift`
**State:** ✅ Acknowledged upstream issue (PR #4624). Not in scope. PR merged — thread irrelevant.
---
### ✅ PR #4591 — Bugbot (HistoryCleaner.swift:74): nil-coalescing
**Source:** Bugbot on PR #4591, `HistoryCleaner.swift:74`
**State:** ✅ Acknowledged upstream issue (PR #4625). Not in scope. PR merged — thread irrelevant.
---
### ✅ [Resolved] PR #4591 — Bugbot (NewTabPageViewController.swift): Duplicated editing-state embedding constraint logic
**Source:** Bugbot on PR #4591, `NewTabPageViewController.swift`
**State:** ✅ [Resolved] — `embedDialogInEditingState` removed in `sr-feedback`.
---
### ✅ PR #4591 — Alessandro (RebrandedNewTabDaxDialogFactory.swift): Closure sent via notification
**Source:** [Alessandro review on PR #4591](https://github.com/duckduckgo/apple-browsers/pull/4591), `RebrandedNewTabDaxDialogFactory.swift`
**State:** ✅ NIP — Alessandro acknowledged refactor not required. PR merged.
---
### ✅ PR #4591 — Alessandro (MainViewController.swift): `as? (() -> Void)` runtime cast
**Source:** [Alessandro review on PR #4591](https://github.com/duckduckgo/apple-browsers/pull/4591), `MainViewController.swift`
**State:** ✅ Fixed in `sr-feedback` (typed `SettingsDeepLinkCallback` struct, commit `482056566b`). Lands with PR #4855. PR #4591 merged.
---
### 🟡 PR #4591 — Alessandro (NewTabPageViewController.swift): GJ3 — hide address bar
**Source:** [Alessandro review on PR #4591](https://github.com/duckduckgo/apple-browsers/pull/4591), `NewTabPageViewController.swift`
**Issue:** Alessandro flagged the GJ3 requirement to hide the address bar for the visit-site step.
**State:** 🟡 [OPEN] on GitHub thread. Fix implemented in `sr-feedback` branch (PR #4855) via `setBarsHidden`. Will land when stack merges.
---
### ✅ [Resolved] PR #4591 — Bugbot (RebrandedNewTabDaxDialogFactory.swift): `onPresented` passes `false` but sheet was shown
**Source:** Bugbot on PR #4591, `RebrandedNewTabDaxDialogFactory.swift`
**Issue (Bugbot):** `onPresented` closure calls `onDismiss(false)` despite the subscription sheet having been shown.
**Why it's a false positive:** The `Bool` parameter is `activateSearch`, not a "was shown" flag. `false` = "don't activate the search bar after dismissal" (user is heading to the subscription flow). `true` = "activate search" (user tapped Skip). The legacy `NewTabDaxDialogFactory` does the same in its proceed path. The `onPresented` deferral is intentional — avoids an NTP flash by keeping the promo visible until the Settings sheet fully covers it.
**Fix:** Added inline comment `// activateSearch: false — user is navigating to subscription, not returning to search.` to make the parameter intent explicit.
**State:** ✅ False positive. Comment added to suppress future confusion.
---
### ✅ [Resolved] PR #4591 — Bugbots (additional resolved threads)
| Thread | State |
|---|---|
| Verbose multi-line comments explain standard patterns (`NewTabPageViewController.swift`) | [Resolved] |
| Missing `?? parent` fallback when finding editing controller (`NewTabPageViewController.swift`) | [Resolved] — editing state approach removed |
| Private method `embedDialogInEditingState` is never called (`NewTabPageViewController.swift`) | [Resolved] — method removed in `sr-feedback` |
---
## PR #4664 Comments
> Threads in GitHub order.
### ✅ [Resolved] PR #4664 — Bugbot: NTP dialog title changed for all paths, not just chat-path
**Source:** Bugbot + [Alessandro review on PR #4664](https://github.com/duckduckgo/apple-browsers/pull/4664), `RebrandedNewTabDaxDialogFactory.swift`
**Fix:** `isChatPath` check added — title now conditional on `chatPathPhase == .visitSite`.
**State:** ✅ [Resolved] on GitHub. Fixed in PR #4664.
---
### ✅ [Resolved] PR #4664 — Bugbot (RebrandedContextualOnboardingDialogs+SubscriptionPromo.swift:101): Dismiss button appears on chat path
**Source:** Bugbot on PR #4664, `RebrandedContextualOnboardingDialogs+SubscriptionPromo.swift:101`
**Issue:** X dismiss button rendered unconditionally on subscription promo dialog; should be hidden on chat path (user should use the dedicated CTA, not escape via X).
**Fix:** `onManualDismiss` is `(() -> Void)?` in `OnboardingSubscriptionPromoDialog` (line 40, comment: "When `nil` the X dismiss button is hidden (e.g. chat-path onboarding)"). `RebrandedNewTabDaxDialogFactory` passes `nil` when `isChatPath`. Already in code — Bugbot is outdated.
**State:** ✅ Already fixed. GitHub thread status unknown.
---
## PR #4668 Comments
> Threads in GitHub order.
### ⭕ PR #4668 — Bugbot (OnboardingIntroViewModel.swift:473): Hardcoded debug `return`
**Source:** Bugbot on PR #4668, `OnboardingIntroViewModel.swift:473`
**Issue:** Hardcoded `return .treatmentA` bypasses feature flag in `resolveDuckAIQueryExperimentCohortID()`.
**State:** ⭕ Removed and pushed on #4668 branch. GitHub thread still open — needs resolving.
---
### ⭕ PR #4668 — Bugbot (FeatureFlag.swift:635): Commented-out `Config(...)` line
**Source:** Bugbot on PR #4668, `FeatureFlag.swift:635`
**Issue:** `defaultValue: .enabled` override was added to `unifiedToggleInput` for testing, plus the old `// Config(...)` line still sits underneath as dead commented-out code. Both need to be removed before merging.
**State:** ⭕ TODO — still in code on `#4668` branch. Deferred until main PR (#4544) is done.
---
### 🔴 PR #4668 — Bugbot (MainViewController.swift:3457): Protocol method `embedInUnifiedInputEditingAreaIfActive` never called
**Source:** Bugbot on PR #4668, `MainViewController.swift:3457`
**Issue:** Method added to `BrowserChromeDelegate` protocol and implemented, but no caller exists.
**State:** 🔴 [OPEN] on GitHub. Deferred until #4544 is done — see BrowserChromeManager item below for full detail.
---
### ⭕ PR #4668 — aataraxiaa (FeatureFlag.swift:635): Commented-out config line
**Source:** [aataraxiaa review comment on PR #4668](https://github.com/duckduckgo/apple-browsers/pull/4668), `FeatureFlag.swift:635`
**Issue:** Same commented-out `Config(...)` dead code flagged by Bugbot above. aataraxiaa also requests it be removed.
**State:** ⭕ TODO — GitHub thread [OPEN]. Deferred until #4544 is done.
---
### 🔴 PR #4668 — Bugbot (BrowserChromeManager.swift:43) `r3234513817`: New protocol method no callers
**Source:** Bugbot on PR #4668, `BrowserChromeManager.swift:43`
**Issue:** `embedInUnifiedInputEditingAreaIfActive` added to `BrowserChromeDelegate` protocol and implemented, but no caller exists. The NTP's `embedDialogInEditingState` and `showDuckAIOnboardingCompletionDialogInUTI` both embed by casting `parent as? MainViewController` directly instead of going through this protocol method. Additionally the **implementations differ**: the protocol method pins to `navigationBarContainer.bottomAnchor`, while actual NTP embedding pins to `coordinator.viewController.view.bottomAnchor`. So simply adding a caller is not enough — implementations must be aligned first.
**State:** 🔴 [OPEN] on GitHub. Deferred until #4544 is done. Fix = align implementations + wire caller, or remove the protocol method entirely.
---
## PR #4855 Comments
> Threads in GitHub order.
### 🟠 [Resolved] PR #4855 — Bugbot: Nav bar not restored when NTP dismissed during visit-site dialog
**Source:** Bugbot on PR #4855, `NewTabPageViewController.swift:281`
**Fix:** `dismiss()` now checks `didHideBarsForChatPathVisitSiteDialog` and calls `setBarsHidden(false)` before removing the view. Previously only `dismissHostingController()` restored the bars, but `dismiss()` (called when the NTP is removed via `removeHomeScreen()`) did not.
**Note on animation:** `dismiss()` uses `animated: false` while `dismissHostingController()` uses `animated: true`. This is intentional — `dismiss()` is the teardown path (e.g. user switches tabs), where the NTP view is being removed anyway, so animating bars in is pointless. `dismissHostingController()` is the success path (user picks a site), where the bars slide in as part of the navigation transition.
**State:** `sr-feedback / uncommitted` — fix is in working tree, NOT committed. GitHub thread NOT resolved.
---
### 🟠 PR #4855 — Bugbot: Guard early return aborts entire dialog presentation (`NewTabPageViewController.swift:473`)
**Source:** Bugbot on PR #4855, `NewTabPageViewController.swift:473`
**Issue:** `guard (parent as? MainViewController)?.currentTab?.isLoading != true else { return }` was a standalone guard that aborted the whole function. When the tab was loading, the guard returned early before any dialog or bar logic ran, leaving an orphaned `hostingController` and a blank NTP.
**Fix:** Removed the loading check entirely. The standalone guard (`(parent as? MainViewController)?.currentTab?.isLoading`) was deleted — no delegate method was introduced. The `if` block now only checks `spec == .subsequent` and `chatPathPhase == .visitSite`. Dialog always presents; no loading condition gates it.
**State:** `sr-feedback / uncommitted` — fix is in working tree, NOT committed. GitHub thread NOT resolved.
---
### 🟠 PR #4855 — Bugbot: Hardcoded `return .treatmentB` bypasses feature flag in `OnboardingIntroViewModel`
**Source:** Bugbot on PR #4855, `OnboardingIntroViewModel.swift:473`
**Fix:** The original `return .treatmentB` Bugbot flagged was already removed in an earlier commit. The override was subsequently **restored** to `return .treatmentA` (with `// TODO: Remove this before shipping`) for development testing purposes. The GitHub thread predates that restore and refers to `.treatmentB`; the `.treatmentA` override is intentional and tracked under PRE-SHIP-2 below.
**State:** `sr-feedback / uncommitted` — override present in working tree, NOT committed. GitHub thread NOT resolved (refers to outdated `.treatmentB`).
---
### 🟠 PR #4855 — Bugbot: Returning-user guard commented out in `insertExperimentStepIfNeeded`
**Source:** Bugbot on PR #4855, `OnboardingIntroViewModel.swift:451`. Also: [Alessandro May 7 review on #4544](https://github.com/duckduckgo/apple-browsers/pull/4544), Asana task [1214682678629767](https://app.asana.com/1/137249556945/task/1214682678629767).
**Fix:** Uncommented `guard case .introDialog(isReturningUser: false) = introSteps.first` in `insertExperimentStepIfNeeded()`. Returning users (including sync-restore users) are now excluded from the Duck.ai query experiment step. The `isReturningUser` value comes from `OnboardingManager.isNewUser` (line 250 in `OnboardingIntroViewModel`: `introDialog(isReturningUser: !isNewUser)`).
**State:** `sr-feedback / uncommitted` — fix is in working tree, NOT committed. GitHub thread NOT resolved.
---
### 🟠 PR #4855 — Alessandro (line 280): Use `NewTabPageControllerDelegate` instead of `(parent as? MainViewController)` cast
**Source:** [#4855 review comment, line 280](https://github.com/duckduckgo/apple-browsers/pull/4855)
**Fix:** The loading check was the only reason for the `(parent as? MainViewController)` cast, so the cast was removed by eliminating the loading check entirely (see "Guard early return" item above). No delegate method was added. Zero parent casts remain in `NewTabPageViewController`.
**State:** `sr-feedback / uncommitted` — fix is in working tree, NOT committed. GitHub thread NOT resolved.
---
### 🟠 PR #4855 — Alessandro (line 477): Hide both bars via `setBarsHidden` (Costas preference)
**Source:** [#4855 review comment, line 477](https://github.com/duckduckgo/apple-browsers/pull/4855/files#r3238549269)
**Fix:** Replaced `chromeDelegate?.setNavigationBarHidden(true/false)` + `(parent as? MainViewController)?.setChatPathVisitSiteControlsLocked(true/false)` with `chromeDelegate?.setBarsHidden(true/false, animated: false, customAnimationDuration: nil)` in `NewTabPageViewController` — both the show path and `dismiss()` / `dismissHostingController()`. This removes all `(parent as?)` casts from `NewTabPageViewController`.
**Why `setChatPathVisitSiteControlsLocked` calls were removed:** Alessandro's review requested `setBarsHidden` via `chromeDelegate` (no parent cast). `setBarsHidden` hides both address bar and toolbar in one call, making `setChatPathVisitSiteControlsLocked` unreachable. The method definition still sits in `MainViewController+DuckAIExperiment.swift:166` as dead code — see DEAD-1 below.
**State:** `sr-feedback / uncommitted` — fix is in working tree, NOT committed. GitHub thread NOT resolved.
---
### 🟠 [Resolved] PR #4855 — Alessandro (UserText.swift:2375): Revert `searchAndDuckAIOption` copy
**Source:** [#4855 review comment, UserText.swift:2375](https://github.com/duckduckgo/apple-browsers/pull/4855)
**Fix:** Reverted `searchAndDuckAIOption` back to `"Toggle between\nSearch and Duck.ai"` in `UserText.swift` and all 26 lproj files. (Was incorrectly changed to "Ask AI" — that rename applies only to the DuckAI query toggle label, not the Search Experience screen.)
**State:** GitHub thread **[Resolved]**. Fix is in working tree, NOT committed.
---
### ✅ PR #4855 — Alessandro (line 450): Add `!restorePromptHandler.isEligibleForRestorePrompt()` to the returning-user guard
**Source:** [Alessandro May 15 review on PR #4855](https://github.com/duckduckgo/apple-browsers/pull/4855), `OnboardingIntroViewModel.swift:450`. Also referenced in [Alessandro's May 15 Asana comment](https://app.asana.com/1/137249556945/task/1214348462471176/comment/1214794564106665).
**Fix:** `!restorePromptHandler.isEligibleForRestorePrompt()` is the second condition in the guard at `OnboardingIntroViewModel.swift:451`.
**State:** ✅ Committed in `alex/demo-tracker-blocking-onboarding` via PR #4584 merge (commit `4e626d3bdc`, 2026-05-18). GitHub thread on PR #4855 still open — needs resolving.
---
### 🟠 PR #4855 — Legacy factory: dismiss button shown unconditionally on visit-site dialog
**Source:** Observed during SR feedback pass. Parity issue with `RebrandedNewTabDaxDialogFactory`, which already suppresses the X button on chat path.
**Issue:** `NewTabDaxDialogFactory.createSubsequentDialog` always passed `manualDismissAction` as a non-optional closure to `OnboardingTryVisitingSiteDialog`, so `DaxDialogView` always rendered the X dismiss button — including on the chat path where the user must pick a site and should not be able to escape.
**Fix:** `manualDismissAction` changed to `(() -> Void)? = isChatPath ? nil : { … }`. `OnboardingTryVisitingSiteDialog.onManualDismiss` made `(() -> Void)?` so `DaxDialogView`'s `ifLet(onManualDismiss)` skips the button when `nil`. `ContextualDaxDialogsFactory` (in-browser overlay, always correct to show dismiss) unaffected.
**State:** 🟠 Fix in working tree, NOT committed.
---
### ✅ DEAD-1 — `setChatPathVisitSiteControlsLocked` in `MainViewController+DuckAIExperiment.swift` deleted
**Source:** Callers removed as part of PR #4855 Alessandro (line 477) fix above.
**State:** Method deleted. Not committed (tracked in `MainViewController+DuckAIExperiment.swift` working-tree change).
---
### ✅ GJ8 / Copy: "Ask.ai" confirmed (with dot)
**Source:** GJ8 said "Ask AI". [Alessandro's May 14 Asana comment](https://app.asana.com/1/137249556945/task/1214348462471176/comment/1214794310099718) explicitly says **"Ask.ai"** (with dot) — resolves the conflict.
**State:** ✅ Implemented — `UserText.DuckAIQueryExperiment.toggleAILabel` is `"Ask.ai"` (with dot) in working tree. Not committed.
---
### ✅ PRE-SHIP-1 — Privacy config: add US/EN targeting
**Source:** [Task 1214682678629768](https://app.asana.com/1/137249556945/task/1214682678629768). Without `targets`, experiment enrolls all locales worldwide.
**What:** In `privacy-configuration/overrides/ios-override.json`, add `"targets": [{ "localeLanguage": "en", "localeCountry": "US" }]` to the new `onboardingDuckAIQueryTrackersDemoExperiment` entry.
**State:** ✅ Done — `targets` included in the new experiment entry in privacy-config PR. Done together with PRE-SHIP-2.
---
### ✅ PRE-SHIP-2 — Privacy config: rename the experiment
**Source:** [Task 1214682678629768](https://app.asana.com/1/137249556945/task/1214682678629768). Internal testers already enrolled under `onboardingDuckAIQueryExperiment`; reusing it would complete NA analysis immediately (already at sample size).
**Name decided:** [Alessandro May 15](https://app.asana.com/1/137249556945/task/1214348462471176/comment/1214794564106665): use **`onboardingDuckAIQueryTrackersDemoExperiment`** as the new `AIChatSubfeature` case.
**What (three parts):**
1. **BSK** — new case `onboardingDuckAIQueryTrackersDemoExperiment` in `AIChatSubfeature` enum ✅
2. **App** — new `FeatureFlag.onboardingDuckAIQueryTrackersDemoExperiment` added; all flag references (`MainViewController+DuckAIExperiment.swift`, `MainViewController.swift`, `OnboardingIntroViewModel.swift`, `OnboardingPixelReporter.swift`, tests) updated to new name ✅
3. **`privacy-configuration` repo** — new entry `onboardingDuckAIQueryTrackersDemoExperiment` with `targets` added; old `onboardingDuckAIQueryExperiment` set to `"state": "disabled"`
**State:** ✅ Fully implemented. Privacy-config PR merged. App code updated on `alex/demo-tracker-blocking-onboarding`.
---
### 🔴 PRE-SHIP-3 — Tell UTI team the final experiment name
**Source:** [Pete Apr 30 comment](https://app.asana.com/1/137249556945/task/1214147157456478/1214600831548835). Pete's team needs the name to exclude enrolled users from the UTI feature rollout.
**What:** Fill `onboardingDuckAIQueryTrackersDemoExperiment` into [O-J <> O-N Coordination](https://app.asana.com/1/137249556945/project/1214157224317277/task/1214288645859692) and [O-N Live Onboarding Experiment Details](https://app.asana.com/1/137249556945/task/1214601039604921).
**State:** 🔴 Name now known and finalized — needs filling into both Asana tasks.
---
## Status Summary (mirrors deleted Asana comment, updated to current state)
> These were the TBD items called out in the ship review. Status reflects current state as of May 14.
**GJ6c/** ✅ No longer required. The address bar will be hidden for the "Try visiting a site" step (GJ3 / `setBarsHidden`). Costas confirmed a search performed after the trackers-blocked dialog is acceptable, so no additional guard is needed here.
**GJ7/ Sync-restore exclusion** 🟠 Implemented, not yet committed. `guard case .introDialog(isReturningUser: false) = introSteps.first` is now active in `insertExperimentStepIfNeeded()`. Users who selected "Restore My Stuff" (sync restore) will not be enrolled in the experiment. Working tree only — needs committing to `sr-feedback`.
**GJ3/ Hide address bar + toolbar** 🟠 Implemented via `chromeDelegate?.setBarsHidden(true/false)`, not yet committed. Replaces earlier `setNavigationBarHidden` + `setChatPathVisitSiteControlsLocked` approach.
**GJ8/ "Ask.ai" toggle label** 🟠 Copy confirmed as **"Ask.ai"** (with dot) by [Alessandro May 14](https://app.asana.com/1/137249556945/task/1214348462471176/comment/1214794310099718). Currently implemented as "Ask AI" (no dot) in working tree — needs correction to "Ask.ai" before committing.
**Experiment rename in privacy config** ✅ Done. `onboardingDuckAIQueryTrackersDemoExperiment` implemented in BSK, app code updated, privacy-config PR merged with new entry + old disabled.
**Privacy config `targets` payload** ✅ Done. `"targets": [{ "localeLanguage": "en", "localeCountry": "US" }]` added to `onboardingDuckAIQueryTrackersDemoExperiment` in privacy-config PR.
**O-N Live Onboarding Experiment Details ([task 1214601039604921](https://app.asana.com/1/137249556945/task/1214601039604921))** 🔴 Still needs filling: add `onboardingDuckAIQueryTrackersDemoExperiment` into [O-N Live Onboarding Experiment Details](https://app.asana.com/1/137249556945/task/1214601039604921) and [O-J <> O-N Coordination](https://app.asana.com/1/137249556945/project/1214157224317277/task/1214288645859692).
**Alessandro's May 15 QA** — Tested PR #4855 against test cases in [task 1214683268207880](https://app.asana.com/1/137249556945/task/1214683268207880). Test run: [task 1214794564106677](https://app.asana.com/1/137249556945/project/1206329551987282/task/1214794564106677). Left notes in test run and new comments in PR (see PR #4855 — Alessandro line 450 item above).
**Merge approved PRs** — Alessandro recommends merging #4591 and #4664 (both approved) into the main feature branch (#4544) to avoid propagating main-branch merges through each PR separately.
---
## Experiment Config (current, privacy-config PR merged)
`privacy-configuration/overrides/ios-override.json` under `aiChat` subfeatures:
```json
"onboardingDuckAIQueryExperiment": {
"state": "disabled"
},
"onboardingDuckAIQueryTrackersDemoExperiment": {
"state": "enabled",
"targets": [{ "localeLanguage": "en", "localeCountry": "US" }],
"cohorts": [
{ "name": "control", "weight": 1 },
{ "name": "treatmentA", "weight": 1 }
]
}
```
> Feature flag: `FeatureFlag.onboardingDuckAIQueryTrackersDemoExperiment` in `iOS/Core/FeatureFlag.swift`
> Cohort type: `FeatureFlag.DuckAIQueryExperimentCohort`