[2026-05-22] taiga sync: .obsidian/community-plugins.json .obsidian/graph.json .obsidian/hotkeys.json .obsidian/plugins/obsidian-git/data.json .obsidian/plugins/obsidian-git/main.js
This commit is contained in:
@@ -1,753 +0,0 @@
|
||||
# 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` | **Merged** ✅ 2026-05-15 |
|
||||
| [#4668](https://github.com/duckduckgo/apple-browsers/pull/4668) | `demo-tracker-blocking-onboarding-uti-flow` | **Open — fixes applied (2026-05-21)** — bottom-bar blocker fixed; B1/B2/B3 cleanups done; merged main |
|
||||
|
||||
---
|
||||
|
||||
## Open Items
|
||||
|
||||
### D1 — Subscription upsell dismissed: Dax background gone + UTI address bar not activated *(2026-05-21)*
|
||||
**Status:** ✅ Fixed *(2026-05-21)*
|
||||
**Repro:** UTI + experiment flags enabled → complete fire step → NTP shows "You've got this" completion dialog → subscription promo appears → dismiss promo → NTP is blank (no Dax background) and UTI address bar stays collapsed and inactive.
|
||||
**Root cause:** `showDuckAIOnboardingCompletionWithActiveAddressBar` calls `chromeDelegate?.omniBar.beginEditing(animated: true)` before showing the completion dialog. In non-UTI mode this creates an `OmniBarEditingStateViewController` (the modal editing state). In UTI mode there is no editing state VC; instead, `omniBar.beginEditing()` may present a conflicting modal OR trigger the UTI to enter `.aiTab(.expanded)` state, causing `coordinator.isOmnibarSession` to return `false` on the next run loop. `showDuckAIOnboardingCompletionDialog` then falls through to the `OmniBarEditingStateViewController` guard, fails, leaves `view.alpha = 0`, and shows no dialog. Additionally, after the subscription promo is dismissed, `launchNewSearch()` calls `omniBar.beginEditing()` which does not expand the UTI pill.
|
||||
**Fix:** In `launchNewSearch()` (called after subscription promo dismissal), detect `coordinator.isOmnibarSession` and call `coordinator.activateInput()` to re-show the keyboard without resetting the omnibar session. (The completion dialog activation path — `showDuckAIOnboardingCompletionWithActiveAddressBar` — was separately fixed in D2's follow-up: always use `omniBar.beginEditing()` which is correctly intercepted by the UTI.)
|
||||
**Files:** `NewTabPageViewController.swift`
|
||||
|
||||
### D2 — Bottom bar: completion dialog invisible + tab switcher broken after trackers-blocked "Got it" *(2026-05-21)*
|
||||
**Status:** ✅ Fixed *(2026-05-21)*
|
||||
**Repro:** UTI + experiment flags enabled + **bottom bar** position → fire step → visit site → trackers-blocked dialog → tap "Got it" → new NTP opens → completion ("You've got this!") dialog blinks and disappears; tab switcher button stops working permanently.
|
||||
**Root cause (two parts):**
|
||||
1. **Zero-height dialog.** `showDuckAIOnboardingCompletionDialogInUTI` constrained the dialog with `topAnchor = coordinator.viewController.view.bottomAnchor` and `bottomAnchor = container.bottomAnchor`. In bottom bar mode both of those Y-positions equal `toolbar.topAnchor`, so the dialog has ~0 pt height and is never visible.
|
||||
2. **Tab switcher permanently blocked.** `showUnifiedToggleInputOmnibar` sets `navigationBarCollectionView.isUserInteractionEnabled = false` (standard omnibar transition). It is restored by `finishUnifiedToggleInputOmnibarDismiss()` which is called when the dismiss animation completes via `deactivateToOmnibar()`. With the zero-height dialog the user can never tap "Got it", so `collapseUTI()` → `dismissUnifiedToggleInputToOmnibar` is never called → `finishUnifiedToggleInputOmnibarDismiss` never runs → tab switcher stays non-interactive.
|
||||
|
||||
**Fix:**
|
||||
- `showDuckAIOnboardingCompletionDialogInUTI`: detect `coordinator.cardPosition.isBottom`; in bottom bar mode use `topAnchor = container.topAnchor` / `bottomAnchor = coordinator.viewController.view.topAnchor` so the dialog fills the content area **above** the UTI bar.
|
||||
- `showDuckAIOnboardingCompletionWithActiveAddressBar`: removed the special `coordinator.activateInput()` branch (which only fired when `isOmnibarSession = true`, never the case here since `refreshNonAITab` hid the coordinator). Always use `omniBar.beginEditing()` — in UTI mode this is intercepted by `activateFromOmnibarIfNeeded` → `coordinator.activateFromOmnibar(cardPosition:)`, which correctly sets `cardPosition = .bottom` before the async dialog-setup hop.
|
||||
|
||||
**Files:** `NewTabPageViewController.swift`
|
||||
|
||||
---
|
||||
|
||||
### C1 — Intro onboarding background lingers during UTI duck.ai transition *(2026-05-21)*
|
||||
**Status:** ✅ Fixed *(2026-05-21)*
|
||||
**Repro:** New install, UTI + experiment flags enabled → complete intro onboarding choosing duck.ai mode → the gradient/decorative background from the intro onboarding is still visible during the hand-off to the UTI duck.ai first-request state.
|
||||
**Root cause:** In UTI mode `onboardingCompletedWithExperimentTransition` called `setBarsVisibility(0, animated: true)` which tears down the UTI session mid-animation. The snapshot fade was not enough because the keyboard / UTI input bar dismissal raced with the transition.
|
||||
**Fix:** In `MainViewController+DuckAIExperiment.swift`, detect UTI active mode and skip the `setBarsVisibility` call entirely; instead let the UTI session handle its own dismissal cleanly. The intro background snapshot fades out independently without fighting the bar-visibility animation.
|
||||
**Files:** `MainViewController+DuckAIExperiment.swift`
|
||||
|
||||
### C2 — Chats + Tabs buttons not disabled during AI onboarding *(2026-05-21)*
|
||||
**Status:** ✅ Fixed *(2026-05-21)*
|
||||
**Repro:** UTI + experiment flags enabled → advance to duck.ai first-request step → the Chat-list button and Tab-switcher button in the AI chat header remain tappable and visually undimmed.
|
||||
**Root cause:** `setOnboardingLocked(_ locked:)` in `AIChatTabChatHeaderView.swift` only disabled `appMenuButton`, `newChatButton`, and `titleContainer`. `chatListButton` and `tabSwitcherButton` were not included. `TabSwitcherStaticButton` also does not auto-dim on `isEnabled = false` (unlike standard `UIButton`), so explicit alpha was needed.
|
||||
**Fix:** Added `chatListButton.isEnabled`, `chatListButton.alpha`, `tabSwitcherButton.isEnabled`, and `tabSwitcherButton.alpha = locked ? 0.5 : 1` to `setOnboardingLocked`.
|
||||
**Files:** `AIChatTabChatHeaderView.swift`
|
||||
|
||||
### C3 — Fire button invisible + no blinking circle during duck.ai fire dialog *(2026-05-21)*
|
||||
**Status:** ✅ Fixed *(2026-05-21)*
|
||||
**Repro:** UTI + experiment flags enabled → complete duck.ai first request → fire dialog appears but fire button is disabled/faded and has no `ViewHighlighter` pulse circle (or pulse appears in wrong position, below screen).
|
||||
**Root cause (multi-part):**
|
||||
1. In UTI AI-tab mode the fire button is `UnifiedToggleInputView.aiTabCollapsedFireButton` (the flanking pill button to the left of the input bar), NOT the traditional toolbar fire button. The toolbar is hidden by `reconcileToolbarVisibilityForCurrentTab()`.
|
||||
2. `showFireButtonPulse()` only checked `toolbar.isHidden` for the iPad path; on iPhone with UTI it returned `nil` (no `tabsBarController`) and exited without highlighting.
|
||||
3. `setOnboardingControlsLocked(true)` set `viewController.view.isUserInteractionEnabled = false`, which disabled the entire view tree including the fire button — child `isUserInteractionEnabled = true` cannot override a parent's `false` in UIKit.
|
||||
4. `viewController.view.alpha = 0.5` multiplied down the tree, dimming the fire button even after attempting `fireButton.alpha = 1`.
|
||||
5. The text entry (`SwitchBarTextEntryView`) directly becomes first responder on tap, bypassing all `isOnboardingLocked` guards.
|
||||
|
||||
**Fix:**
|
||||
- Exposed `aiTabCollapsedFireButton` through the chain: `UnifiedToggleInputView.aiTabFireButton` → `UnifiedToggleInputViewController.aiTabFireButton` → `UnifiedToggleInputCoordinator.aiTabFireButton`.
|
||||
- `showFireButtonPulse()` in `MainViewController`: if UTI coordinator is active and `aiTabFireButton.isHidden == false`, use that button for `ViewHighlighter.showIn`.
|
||||
- Replaced view-level `isUserInteractionEnabled = false` + `alpha` in `setOnboardingControlsLocked` with a new `setOnboardingDimmed(_ dimmed:)` method on `UnifiedToggleInputView` that: (a) sets `alpha = 0.5` on every direct subview *except* `aiTabCollapsedFireButton` (siblings, so no multiplicative alpha issue), and (b) sets `textEntryView.isUserInteractionEnabled = false` to block direct first-responder activation.
|
||||
- Added `guard !isOnboardingLocked else { return }` to `unifiedToggleInputVCDidTapVoice` (previously missing) so the voice button is also blocked.
|
||||
- Rolled back the earlier wrong fix (`viewCoordinator.toolbar.isHidden = false` in `applyExperimentDuckAIFireChromeState`).
|
||||
|
||||
**Files:** `UnifiedToggleInputView.swift`, `UnifiedToggleInputViewController.swift`, `UnifiedToggleInputCoordinator.swift`, `MainViewController.swift`, `MainViewController+DuckAIExperiment.swift`
|
||||
|
||||
### ✅ 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()`. Code at line 473: `// TODO: Remove this` + `return .treatmentA` — the `guard` and actual feature-flag lookup below are dead code.
|
||||
|
||||
**Additional:** The code below the dead `return` still references the OLD flag name `onboardingDuckAIQueryExperiment` instead of the renamed `onboardingDuckAIQueryTrackersDemoExperiment`. The rename (PRE-SHIP-2) landed on `alex/demo-tracker-blocking-onboarding` but has NOT been propagated to `uti-flow` branch yet.
|
||||
|
||||
**State:** 🔴 Still in code on `#4668` branch. Must be fixed: (1) remove the `return .treatmentA` + `// TODO` comment, (2) update flag reference to `onboardingDuckAIQueryTrackersDemoExperiment` after rebasing the branch stack.
|
||||
|
||||
---
|
||||
|
||||
### ⭕ 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 — aataraxiaa + Pete (Asana): Bottom bar position — onboarding stops after AI chat dismissed
|
||||
|
||||
**Source:** [aataraxiaa PR comment](https://github.com/duckduckgo/apple-browsers/pull/4668#issuecomment-4358732539) ("tested with bottom bar position and it didn't work as expected") + [Pete Asana comment on PR task 1214425079182350](https://app.asana.com/1/137249556945/task/1214425079182350) ("when I tested after selecting bottom position during onboarding, after I buried the chat, nothing, no further onboarding was displayed"). This is why the PR is currently **[ON HOLD]**.
|
||||
|
||||
**Issue:** After the user selects the **bottom bar** position during onboarding and completes/dismisses the AI chat, no further onboarding dialogs appear. The visit-site dialog or tracker-blocking dialog flow breaks. Top bar position works; bottom bar position does not.
|
||||
|
||||
**State:** 🔴 Root cause not yet identified. SHIP-BLOCKER — must fix before unhold.
|
||||
|
||||
---
|
||||
|
||||
### 🔴 PR #4668 — Pete (Asana): Blue contextual dialogs look bad against dark mode UTI background
|
||||
|
||||
**Source:** [Pete Asana comment on PR task 1214425079182350](https://app.asana.com/1/137249556945/task/1214425079182350): blue contextual onboarding dialogs look poor against the dark-coloured unified input background in dark mode (screenshots attached in Asana).
|
||||
|
||||
**State:** 🔴 Not fixed. Cosmetic — lower priority than bottom-bar blocker, but should be addressed.
|
||||
|
||||
---
|
||||
|
||||
### 🔴 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).
|
||||
|
||||
**UTI bottom bar position (SHIP-BLOCKER)** 🔴 After selecting bottom bar position in onboarding and dismissing AI chat, no further onboarding dialogs appear. Blocks unhold of PR #4668. Root cause TBD.
|
||||
|
||||
**UTI dark mode dialogs (cosmetic)** 🔴 Blue dialogs look poor against dark UTI background. Lower priority; should address before ship.
|
||||
|
||||
**PR #4668 pre-ship cleanups** 🔴 Three items must be fixed before #4668 can merge: (1) remove `return .treatmentA` hardcode + update flag name to `onboardingDuckAIQueryTrackersDemoExperiment`, (2) restore `unifiedToggleInput` FeatureFlag config (remove `defaultValue: .enabled` + dead comment), (3) deal with unused `embedInUnifiedInputEditingAreaIfActive` protocol method.
|
||||
|
||||
**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`
|
||||
Reference in New Issue
Block a user