Skip to content

[iOS] Ship review changes for for fire mode#4436

Open
hassaanelgarem wants to merge 3 commits intomainfrom
hassaan/fire-mode/ship-review-1
Open

[iOS] Ship review changes for for fire mode#4436
hassaanelgarem wants to merge 3 commits intomainfrom
hassaan/fire-mode/ship-review-1

Conversation

@hassaanelgarem
Copy link
Copy Markdown
Contributor

@hassaanelgarem hassaanelgarem commented Apr 10, 2026

Task/Issue URL: https://app.asana.com/1/137249556945/task/1214011006395194?focus=true
cc @SabrinaTardio

Description

This PR addresses the following issues:

  • Issue 1: Sidebar button in chats wasn't working in fire tabs. Addressed by adjusting the logic of isFrontendReady to not depend on sync being enabled.
  • Issue 2: Burning the last open chat in normal mode didn't open a new chat

Testing Steps

  1. Make sure both fireMode and fireButtonRefinements flags are enabled.
  2. Verify that the sidebar button works on fire tabs
  3. Have 1 tab open in normal mode and have it be a chat. Burn it. Verify a new chat is opened.

Impact and Risks

Low: Minor visual changes, small bug fixes, improvement to existing features

What could go wrong?

N/A

Quality Considerations

N/A

Notes to Reviewer

Issue 2 was happening because burning a chat triggers a replacement of the current tab with a new chat tab. The old logic has an optimization that was the root cause. The optimization assumes that when there's only 1 normal tab, the auto-created blank tab "serves the same purpose" as newTab. This is true when newTab is a blank/home tab, but not when newTab has a link (like an AI chat URL). The newTab is silently discarded.


Internal references:

Definition of Done | Engineering Expectations | Tech Design Template


Note

Medium Risk
Changes tab replacement behavior for the last normal tab and tweaks AI Chat frontend-ready signaling; regressions could affect tab lifecycle (extra/blank tabs) or sidebar toggle responsiveness.

Overview
Fixes AI Chat sidebar toggling in fire tabs by marking the frontend as ready on .getAIChatNativeConfigValues (instead of waiting on .setAIChatHistoryEnabled), and updates tests accordingly.

Adjusts TabManager.replace(tab:withNewTab:) so that when replacing the last normal tab, a non-empty newTab (e.g. AI Chat URL) is still inserted rather than being dropped due to the auto-created blank tab optimization.

Reviewed by Cursor Bugbot for commit 728e804. Bugbot is set up for automated code reviews on this repo. Configure here.

@hassaanelgarem hassaanelgarem requested a review from dus7 April 10, 2026 22:52
@hassaanelgarem hassaanelgarem self-assigned this Apr 10, 2026
@hassaanelgarem hassaanelgarem added the do-not-autoclose Prevents auto-closing by stale PR workflow. label Apr 10, 2026
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2dc9127. Configure here.

Comment thread iOS/DuckDuckGo/AIChat/AIChatContentHandler.swift
}

if message == .setAIChatHistoryEnabled {
if message == .getAIChatNativeConfigValues {
Copy link
Copy Markdown
Contributor

@SabrinaTardio SabrinaTardio Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this does not work because the FE is not fully set up yet when we receive this message, that is why I was using the next message, but I thought it was only about showing chat history so I assumed it always happened.
However if we do if we do message != .getAIChatNativeConfigValues (so that it use the first useful message aka any after this)
seems to work in both cases.
It’s not ideal but we could use it while I get the FE to have the url parameter for toggling the page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-autoclose Prevents auto-closing by stale PR workflow.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants