539 lines
33 KiB
Markdown
539 lines
33 KiB
Markdown
# 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) |
|
||
| [#4591](https://github.com/duckduckgo/apple-browsers/pull/4591) | `demo-tracker-blocking-onboarding-ui-polish` | Approved |
|
||
| [#4664](https://github.com/duckduckgo/apple-browsers/pull/4664) | `demo-tracker-blocking-onboarding-chat-path-dialog-polish` | Approved |
|
||
| [#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.
|
||
|
||
**Fix:** `chromeDelegate?.setBarsHidden(true)` on show, `setBarsHidden(false)` on dismiss. Tracked as PR #4855 — Alessandro (line 477) item above.
|
||
|
||
**State:** `sr-feedback / uncommitted` — in working tree, NOT committed.
|
||
|
||
---
|
||
|
||
### 🟠 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 above.
|
||
|
||
**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 above.
|
||
|
||
⚠️ 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."
|
||
|
||
This is the same bug as **PR #4544 — Bugbot r3234645543** below. Fix is in `didTapDismissContextualOnboardingAction` on sr-feedback branch, not yet on #4544 branch.
|
||
|
||
**State:** 🔴 Fix exists in PR #4855 (`sr-feedback`), NOT in PR #4544. GitHub thread on #4544 NOT resolved.
|
||
|
||
---
|
||
|
||
### 🔴 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 above).
|
||
|
||
---
|
||
|
||
### ➖ 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 #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:** Added `newTabPageControllerCurrentTabIsLoading(_ :) -> Bool` to `NewTabPageControllerDelegate` protocol; implemented in `MainViewController.swift`; removed the `(parent as? MainViewController)?` cast from `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.
|
||
|
||
---
|
||
|
||
### 🟠 PR #4855 — Alessandro (line 473): Fold loading check into `if` condition
|
||
|
||
**Source:** [#4855 review comment, line 473](https://github.com/duckduckgo/apple-browsers/pull/4855)
|
||
|
||
**Fix:** Replaced `guard (parent as? MainViewController)?.currentTab?.isLoading != true else { return }` (which was a standalone guard that aborted the whole function, leaving an orphaned `hostingController`) with `delegate?.newTabPageControllerCurrentTabIsLoading(self) != true` as a third condition in the outer `if`. The dialog still presents; only the bar-hiding is skipped when loading.
|
||
|
||
**State:** `sr-feedback / uncommitted` — fix is in working tree, NOT committed. GitHub thread NOT 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:** `sr-feedback / uncommitted` — fix is in working tree, NOT committed. GitHub thread NOT resolved.
|
||
|
||
---
|
||
|
||
### 🟠 [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.
|
||
|
||
**State:** `sr-feedback / uncommitted` — fix is in working tree, NOT committed. GitHub thread NOT resolved.
|
||
|
||
---
|
||
|
||
### 🟠 PR #4855 — Bugbot: Guard early return leaves orphaned `hostingController` and blank NTP
|
||
|
||
**Source:** Bugbot on PR #4855, `NewTabPageViewController.swift:473`
|
||
|
||
**Fix:** Same as the Alessandro (line 473) item above — guard replaced with `if` condition using delegate method.
|
||
|
||
**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:** Removed `return .treatmentB` (and its `// TODO: Remove this` comment) from `resolveDuckAIQueryExperimentCohortID()`. The method now always goes through `featureFlagger.resolveCohort()`.
|
||
|
||
**State:** `sr-feedback / uncommitted` — fix is in working tree, NOT committed. GitHub thread NOT resolved.
|
||
|
||
---
|
||
|
||
### 🔴 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).
|
||
|
||
**Issue:** The current guard `guard case .introDialog(isReturningUser: false) = introSteps.first` excludes returning users and those eligible to restore data during onboarding. But if the experiment is later remotely enrolled for all users (e.g. post-experiment analysis phase), returning users who are eligible for restore will bypass the guard and still see the Duck.ai experiment step.
|
||
|
||
**Fix:** Add `!restorePromptHandler.isEligibleForRestorePrompt()` as an additional condition to the guard.
|
||
|
||
**State:** 🔴 NOT yet implemented. Needs to be added to the uncommitted working-tree change in `OnboardingIntroViewModel.swift`. GitHub thread NOT resolved.
|
||
|
||
---
|
||
|
||
### 🟠 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 #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:** Added `if contextualOnboardingLogic.chatPathPhase == .trackerToEOJ { delegate?.tabDidRequestNewTab(self) }` to `didTapDismissContextualOnboardingAction`. (Same block was already in `didNavigateAwayFromContextualOnboardingDialog`.)
|
||
|
||
**State:** Fix exists in commit `6c3774e4f8` on `sr-feedback` branch (PR #4855). **NOT present on the `#4544` branch itself.** Fix will land in `main` when the full PR stack merges. GitHub thread on PR #4544 is **NOT resolved**.
|
||
|
||
---
|
||
|
||
### ✅ [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 (outdated): hardcoded cohort / promo override / missing pixels / commented-out redirect
|
||
|
||
**Source:** Four Bugbot comments on early commits of PR #4544.
|
||
|
||
| Comment | 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 are fixed in code. All four GitHub threads are **NOT resolved** (outdated Bugbot comments never closed). → Resolve all four threads.
|
||
|
||
---
|
||
|
||
### 🔴 PR #4544 — Bugbot: Dead property `pendingCompletionDialogMessage` never set non-nil (`NewTabDaxDialogFactory.swift:192`)
|
||
|
||
**Source:** Bugbot on PR #4544, `NewTabDaxDialogFactory.swift:192`
|
||
|
||
**Issue:** `NewTabDaxDialogFactory.createFinalDialog` was updated with chat-path conditional logic (`chatPathPhase == .trackerToEOJ && isAIChatEnabled` choosing a different message + pixel), but `RebrandedNewTabDaxDialogFactory.createFinalDialog` was not updated to match — despite its section mark being renamed to "Chat-Path Completion". The `.final` spec is currently unreachable for chat-path + AI-enabled users (the chat EOJ is driven by `presentChatPathOnboardingCompletionIfNeeded` instead), so `pendingCompletionDialogMessage` is always nil.
|
||
|
||
**State:** 🔴 [OPEN] on GitHub. The dead branching in `NewTabDaxDialogFactory` was removed in a later commit; verify `RebrandedNewTabDaxDialogFactory` is consistent.
|
||
|
||
---
|
||
|
||
### 🔴 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` has chat-path branching (`chatPathPhase == .trackerToEOJ && isAIChatEnabled`), but `RebrandedNewTabDaxDialogFactory.createFinalDialog` does not — both are selected by the `onboardingRebranding` feature flag. While `.final` is currently unreachable for chat-path + AI-enabled users, divergence could cause the wrong message + pixel if the flow changes.
|
||
|
||
**State:** 🔴 [OPEN] on GitHub. Needs symmetry check between the two factories.
|
||
|
||
---
|
||
|
||
### 🔴 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 added under `// MARK: - Chat Path – Subsequent Dialog` target `NewTabDaxDialogFactory` (legacy), not `RebrandedNewTabDaxDialogFactory` (active under the rebranding flag).
|
||
|
||
**State:** 🔴 [OPEN] on GitHub. Rebranded factory coverage may be insufficient.
|
||
|
||
---
|
||
|
||
### ✅ [Resolved] PR #4544 — Bugbots (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` |
|
||
| `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 |
|
||
| Alessandro: tests replicating real `DaxDialogsSettings` logic | Simplified tests |
|
||
|
||
---
|
||
|
||
## Open Items — PR #4591
|
||
|
||
> PR #4591 (`demo-tracker-blocking-onboarding-ui-polish`) is **Approved**. Open threads still need resolving before merge.
|
||
|
||
### 🔴 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`
|
||
|
||
**Issue:** “Not a big fan of sending this closure in a notification, but I guess this would require some refactoring to change it.” Settings deep-link callback dispatched via `NotificationCenter` with a closure payload.
|
||
|
||
**State:** 🔴 [OPEN] on GitHub. Alessandro acknowledged it’s a refactor; no fix required to merge.
|
||
|
||
---
|
||
|
||
### 🔴 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`
|
||
|
||
**Issue:** `as? (() -> Void)` cast is only checked at runtime — if the factory’s closure signature ever changes, it will silently fail.
|
||
|
||
**State:** 🔴 [OPEN] on GitHub. This was fixed in commit `482056566b` on `sr-feedback` (wrapped in a typed `SettingsDeepLinkCallback` struct). Fix is NOT on #4591 branch; will land when stack merges.
|
||
|
||
---
|
||
|
||
### 🟡 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.
|
||
|
||
---
|
||
|
||
### 🔴 PR #4591 — Bugbot (SyncSettingsViewController.swift:738): Experiment pixel skips `.connect` receiver
|
||
|
||
**Source:** Bugbot on PR #4591, `SyncSettingsViewController.swift:738`
|
||
|
||
**Issue:** The experiment success pixel fires for `.recovery` sync path but skips `.connect`. mallexxx acknowledged ([comment](https://github.com/duckduckgo/apple-browsers/pull/4591)): pixel originates in upstream PR #4575, not this PR.
|
||
|
||
**State:** 🔴 [OPEN] on GitHub. Acknowledged as upstream issue.
|
||
|
||
---
|
||
|
||
### 🔴 PR #4591 — Bugbot (SyncSettingsViewController.swift:588): Missing experiment success pixel in `controllerDidCreateSyncAccount`
|
||
|
||
**Source:** Bugbot on PR #4591, `SyncSettingsViewController.swift:588`
|
||
|
||
**State:** 🔴 [OPEN] on GitHub. Needs verification / fix.
|
||
|
||
---
|
||
|
||
### 🔴 PR #4591 — Bugbot (MainViewController.swift): Removed `performCancel` may leave omnibar in editing state
|
||
|
||
**Source:** Bugbot on PR #4591, `MainViewController.swift`
|
||
|
||
**Issue:** Removing `performCancel` call may leave the omnibar stuck in editing state in certain paths.
|
||
|
||
**State:** 🔴 [OPEN] on GitHub. Needs verification.
|
||
|
||
---
|
||
|
||
### 🔴 PR #4591 — Bugbot (RebrandedNewTabDaxDialogFactory.swift): `onPresented` passes `false` but sheet was shown
|
||
|
||
**Source:** Bugbot on PR #4591, `RebrandedNewTabDaxDialogFactory.swift`
|
||
|
||
**Issue:** `onPresented` closure is called with `false` despite the subscription sheet having been shown, leading to incorrect state tracking.
|
||
|
||
**State:** 🔴 [OPEN] on GitHub. Needs fix.
|
||
|
||
---
|
||
|
||
### ✅ [Resolved] PR #4591 — Bugbots (resolved threads)
|
||
|
||
| Thread | State |
|
||
|---|---|
|
||
| Missing experiment success pixel when `shouldShowSyncEnabled` is true (`SyncSettingsViewController.swift:567`) | [Resolved] |
|
||
| Duplicated editing-state embedding constraint logic (`NewTabPageViewController.swift`) | [Resolved] — `embedDialogInEditingState` removed in `sr-feedback` |
|
||
| Verbose multi-line comments explain standard patterns | [Resolved] |
|
||
| Missing `?? parent` fallback when finding editing controller | [Resolved] — editing state approach removed |
|
||
| Private method `embedDialogInEditingState` is never called | [Resolved] — method removed in `sr-feedback` |
|
||
|
||
---
|
||
|
||
### ✅ [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.
|
||
|
||
---
|
||
|
||
### 🔴 PR #4668 — aataraxiaa: Commented-out `Config(...)` line in `FeatureFlag.swift:635`
|
||
|
||
**Source:** [aataraxiaa review comment on PR #4668](https://github.com/duckduckgo/apple-browsers/pull/4668), `FeatureFlag.swift:635`
|
||
|
||
**Issue:** `defaultValue: .enabled` override was added to `unifiedToggleInput` for testing, plus the old `// Config(...)` line is still sitting underneath it as dead commented-out code. Both need to be removed before merging.
|
||
|
||
**State:** 🔴 Still in code on the `#4668` branch. NOT fixed. GitHub thread NOT resolved.
|
||
|
||
---
|
||
|
||
### 🔴 PR #4668 — Bugbot: `embedInUnifiedInputEditingAreaIfActive` added to protocol but never called
|
||
|
||
**Source:** Bugbot on PR #4668, `BrowserChromeManager.swift:43` and `MainViewController.swift:3457`
|
||
|
||
**Issue:** Method added to `BrowserChromeDelegate` protocol and implemented in `BrowserChromeManager`, but no caller exists anywhere.
|
||
|
||
**State:** On `sr-feedback` branch commit `82bb0f71e3` removed `embedDialogInEditingState` dead code; the protocol method may now be entirely unreachable. **Needs verification on #4668 branch.** GitHub thread NOT resolved. → Either add a caller or remove the method and its implementation.
|
||
|
||
---
|
||
|
||
### 🟠 DEAD-1 — `setChatPathVisitSiteControlsLocked` in `MainViewController+DuckAIExperiment.swift:166` is dead code
|
||
|
||
**Source:** Callers removed as part of PR #4855 Alessandro (line 477) fix above.
|
||
|
||
**State:** Method definition still exists in `MainViewController+DuckAIExperiment.swift:166`, no callers. Needs deletion. Not committed.
|
||
|
||
---
|
||
|
||
### ✅ 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:** 🟠 Currently implemented as "Ask AI" (no dot) in working tree. Needs to be changed to "Ask.ai" before committing.
|
||
|
||
---
|
||
|
||
### 🔴 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 `onboardingDuckAIQueryExperiment` entry.
|
||
|
||
**State:** Local edit exists on wrong branch (`alex/remove-autocomplete-tabs`) in `privacy-configuration` repo, not committed. Should be done together with PRE-SHIP-2 in one PR to that repo.
|
||
|
||
---
|
||
|
||
### 🔴 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** — update `FeatureFlag.onboardingDuckAIQueryExperiment` to reference the new subfeature case; update experiment pixel names accordingly
|
||
3. **`privacy-configuration` repo** — add new entry `onboardingDuckAIQueryTrackersDemoExperiment` with `targets`; set old `onboardingDuckAIQueryExperiment` to `"state": "disabled"`
|
||
|
||
**State:** 🔴 Name decided, NOT yet implemented.
|
||
|
||
---
|
||
|
||
### 🔴 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 — can be filled in once PRE-SHIP-2 is done.
|
||
|
||
---
|
||
|
||
## 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** 🔴 Name decided: **`onboardingDuckAIQueryTrackersDemoExperiment`** ([Alessandro May 15](https://app.asana.com/1/137249556945/task/1214348462471176/comment/1214794564106665)). Reason: reusing the old name would complete NA analysis immediately (already at sample size). Still needs implementing: new `AIChatSubfeature` case in BSK, update `FeatureFlag`, new entry in `privacy-configuration/overrides/ios-override.json` (disable old one).
|
||
|
||
**Privacy config `targets` payload** 🔴 TBD. `"targets": [{ "localeLanguage": "en", "localeCountry": "US" }]` must be added to the experiment entry. Local edit exists on wrong branch in `privacy-configuration` repo. Do together with rename above.
|
||
|
||
**O-N Live Onboarding Experiment Details ([task 1214601039604921](https://app.asana.com/1/137249556945/task/1214601039604921))** 🔴 Name now known (`onboardingDuckAIQueryTrackersDemoExperiment`). Fill 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) once PRE-SHIP-2 is implemented.
|
||
|
||
**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, local override only)
|
||
|
||
`privacy-configuration/overrides/ios-override.json` under `aiChat` subfeatures:
|
||
|
||
```json
|
||
"onboardingDuckAIQueryExperiment": {
|
||
"state": "enabled",
|
||
"targets": [{ "localeLanguage": "en", "localeCountry": "US" }],
|
||
"cohorts": [
|
||
{ "name": "control", "weight": 1 },
|
||
{ "name": "treatmentA", "weight": 1 },
|
||
{ "name": "treatmentB", "weight": 1 }
|
||
]
|
||
}
|
||
```
|
||
|
||
> Feature flag: `FeatureFlag.onboardingDuckAIQueryExperiment` in `iOS/Core/FeatureFlag.swift`
|
||
> Cohort type: `FeatureFlag.DuckAIQueryExperimentCohort`
|