marketing-shibata50/cmux
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: address browser profile review follow-ups

Browse source
This commit is contained in:
Lawrence Chen 2026-03-16 23:50:43 -07:00
parent e2ddf9214c
commit fdde470dcf
No known key found for this signature in database
8 changed files with 107 additions and 49 deletions
Download patch file Download diff file Expand all files Collapse all files

8
Resources/Localizable.xcstrings
View file

@ -4797,7 +4797,7 @@
"ja": {
"stringUnit": {
"state": "translated",
"value": "ブラウザ: %@"
"value": "ブラウザー: %@"
}
}
}
@ -37876,7 +37876,7 @@
"ja": {
"stringUnit": {
"state": "translated",
"value": "ブラウザから取り込む…"
"value": "ブラウザーから取り込む…"
}
}
}
@ -50658,7 +50658,7 @@
"ja": {
"stringUnit": {
"state": "translated",
"value": "ブラウザデータを取り込む"
"value": "ブラウザーデータを取り込む"
}
}
}
@ -50788,7 +50788,7 @@
"ja": {
"stringUnit": {
"state": "translated",
"value": "ブラウザから取り込む"
"value": "ブラウザーから取り込む"
}
}
}

54
Sources/Panels/BrowserPanel.swift
View file

@ -2354,11 +2354,41 @@ final class BrowserPanel: Panel, ObservableObject {
webView.onContextMenuOpenLinkInNewTab = { [weak self] url in
self?.openLinkInNewTab(url: url)
}
configureNavigationDelegateCallbacks()
webView.navigationDelegate = navigationDelegate
webView.uiDelegate = uiDelegate
setupObservers(for: webView)
}
private func configureNavigationDelegateCallbacks() {
guard let navigationDelegate else { return }
let boundWebViewInstanceID = webViewInstanceID
let boundHistoryStore = historyStore
navigationDelegate.didFinish = { [weak self] webView in
Task { @MainActor [weak self] in
guard let self, self.isCurrentWebView(webView, instanceID: boundWebViewInstanceID) else { return }
boundHistoryStore.recordVisit(url: webView.url, title: webView.title)
self.refreshFavicon(from: webView)
self.applyBrowserThemeModeIfNeeded()
// Keep find-in-page open through load completion and refresh matches for the new DOM.
self.restoreFindStateAfterNavigation(replaySearch: true)
}
}
navigationDelegate.didFailNavigation = { [weak self] failedWebView, failedURL in
Task { @MainActor in
guard let self, self.isCurrentWebView(failedWebView, instanceID: boundWebViewInstanceID) else { return }
// Clear stale title/favicon from the previous page so the tab
// shows the failed URL instead of the old page's branding.
self.pageTitle = failedURL.isEmpty ? "" : failedURL
self.faviconPNGData = nil
self.lastFaviconURLString = nil
// Keep find-in-page open and clear stale counters on failed loads.
self.restoreFindStateAfterNavigation(replaySearch: false)
}
}
}
private func isCurrentWebView(_ candidate: WKWebView, instanceID: UUID? = nil) -> Bool {
guard candidate === webView else { return false }
guard let instanceID else { return true }
@ -2389,30 +2419,6 @@ final class BrowserPanel: Panel, ObservableObject {
// Set up navigation delegate
let navDelegate = BrowserNavigationDelegate()
navDelegate.didFinish = { webView in
Task { @MainActor [weak self] in
self?.historyStore.recordVisit(url: webView.url, title: webView.title)
}
Task { @MainActor [weak self] in
guard let self, self.isCurrentWebView(webView) else { return }
self.refreshFavicon(from: webView)
self.applyBrowserThemeModeIfNeeded()
// Keep find-in-page open through load completion and refresh matches for the new DOM.
self.restoreFindStateAfterNavigation(replaySearch: true)
}
}
navDelegate.didFailNavigation = { [weak self] failedWebView, failedURL in
Task { @MainActor in
guard let self, self.isCurrentWebView(failedWebView) else { return }
// Clear stale title/favicon from the previous page so the tab
// shows the failed URL instead of the old page's branding.
self.pageTitle = failedURL.isEmpty ? "" : failedURL
self.faviconPNGData = nil
self.lastFaviconURLString = nil
// Keep find-in-page open and clear stale counters on failed loads.
self.restoreFindStateAfterNavigation(replaySearch: false)
}
}
navDelegate.openInNewTab = { [weak self] url in
self?.openLinkInNewTab(url: url)
}

3
Sources/Panels/BrowserPanelView.swift
View file

@ -1518,8 +1518,9 @@ struct BrowserPanelView: View {
private func applyBrowserProfileSelection(_ profileID: UUID) {
isBrowserProfileMenuPresented = false
let didApply = panel.profileID == profileID || panel.switchToProfile(profileID)
guard didApply else { return }
owningWorkspace?.setPreferredBrowserProfileID(profileID)
_ = panel.switchToProfile(profileID)
}
private func presentCreateBrowserProfilePrompt() {

10
Sources/Workspace.swift
View file

@ -2326,7 +2326,6 @@ final class Workspace: Identifiable, ObservableObject {
)
panels[browserPanel.id] = browserPanel
panelTitles[browserPanel.id] = browserPanel.displayTitle
setPreferredBrowserProfileID(browserPanel.profileID)
// Pre-generate the bonsplit tab ID so the mapping exists before the split lands.
let newTab = Bonsplit.Tab(
@ -2350,6 +2349,7 @@ final class Workspace: Identifiable, ObservableObject {
panelTitles.removeValue(forKey: browserPanel.id)
return nil
}
setPreferredBrowserProfileID(browserPanel.profileID)
// See newTerminalSplit: suppress old view's becomeFirstResponder during reparenting.
let previousHostedView = focusedTerminalPanel?.hostedView
@ -2386,16 +2386,19 @@ final class Workspace: Identifiable, ObservableObject {
bypassInsecureHTTPHostOnce: String? = nil
) -> BrowserPanel? {
let shouldFocusNewTab = focus ?? (bonsplitController.focusedPaneId == paneId)
let sourcePanelId = effectiveSelectedPanelId(inPane: paneId)
let browserPanel = BrowserPanel(
workspaceId: id,
profileID: resolvedNewBrowserProfileID(preferredProfileID: preferredProfileID),
profileID: resolvedNewBrowserProfileID(
preferredProfileID: preferredProfileID,
sourcePanelId: sourcePanelId
),
initialURL: url,
bypassInsecureHTTPHostOnce: bypassInsecureHTTPHostOnce
)
panels[browserPanel.id] = browserPanel
panelTitles[browserPanel.id] = browserPanel.displayTitle
setPreferredBrowserProfileID(browserPanel.profileID)
guard let newTabId = bonsplitController.createTab(
title: browserPanel.displayTitle,
@ -2412,6 +2415,7 @@ final class Workspace: Identifiable, ObservableObject {
}
surfaceIdToPanelId[newTabId] = browserPanel.id
setPreferredBrowserProfileID(browserPanel.profileID)
// Keyboard/browser-open paths want "new tab at end" regardless of global new-tab placement.
if insertAtEnd {

54
cmuxTests/CmuxWebViewKeyEquivalentTests.swift
View file

@ -6271,6 +6271,12 @@ final class WorkspaceBrowserProfileSelectionTests: XCTestCase {
}
}
private final class RejectingSplitPaneDelegate: BonsplitDelegate {
func splitTabBar(_ controller: BonsplitController, shouldSplitPane pane: PaneID, orientation: SplitOrientation) -> Bool {
false
}
}
private func makeProfile(named prefix: String) throws -> BrowserProfileDefinition {
try XCTUnwrap(
BrowserProfileStore.shared.createProfile(
@ -6354,6 +6360,38 @@ final class WorkspaceBrowserProfileSelectionTests: XCTestCase {
"Expected a failed browser creation to leave the workspace preferred profile unchanged"
)
}
func testNewBrowserSplitFailureDoesNotMutatePreferredProfile() throws {
let workspace = Workspace()
let preferredProfile = try makeProfile(named: "Preferred")
let unexpectedProfile = try makeProfile(named: "Unexpected")
let paneId = try XCTUnwrap(workspace.bonsplitController.focusedPaneId)
let browser = try XCTUnwrap(
workspace.newBrowserSurface(
inPane: paneId,
focus: true,
preferredProfileID: preferredProfile.id
)
)
XCTAssertEqual(workspace.preferredBrowserProfileID, preferredProfile.id)
let rejectingDelegate = RejectingSplitPaneDelegate()
workspace.bonsplitController.delegate = rejectingDelegate
let created = workspace.newBrowserSplit(
from: browser.id,
orientation: .horizontal,
preferredProfileID: unexpectedProfile.id,
focus: false
)
XCTAssertNil(created)
XCTAssertEqual(
workspace.preferredBrowserProfileID,
preferredProfile.id,
"Expected a failed browser split to leave the workspace preferred profile unchanged"
)
}
}
@MainActor
@ -6434,7 +6472,10 @@ final class BrowserPanelProfileIsolationTests: XCTestCase {
alternateStore.clearHistory()
}
let panel = BrowserPanel(workspaceId: UUID())
let panel = BrowserPanel(
workspaceId: UUID(),
profileID: BrowserProfileStore.shared.builtInDefaultProfileID
)
let staleWebView = panel.webView
let staleDelegate = try XCTUnwrap(staleWebView.navigationDelegate)
let staleURL = try XCTUnwrap(URL(string: "https://example.com/stale-finish"))
@ -6443,18 +6484,23 @@ final class BrowserPanelProfileIsolationTests: XCTestCase {
baseURL: staleURL
)
XCTAssertTrue(panel.switchToProfile(alternateProfile.id))
XCTAssertTrue(
panel.switchToProfile(alternateProfile.id),
"Expected profile switch to succeed, current=\(panel.profileID) requested=\(alternateProfile.id) exists=\(BrowserProfileStore.shared.profileDefinition(id: alternateProfile.id) != nil)"
)
defaultStore.clearHistory()
alternateStore.clearHistory()
staleDelegate.webView?(staleWebView, didFinish: nil)
drainMainQueue()
XCTAssertTrue(
defaultStore.entries.isEmpty,
"Expected stale completion callbacks to avoid writing into the old profile history store"
"Expected stale completion callbacks to avoid writing into the old profile history store, found \(defaultStore.entries.map { $0.url })"
)
XCTAssertTrue(
alternateStore.entries.isEmpty,
"Expected stale completion callbacks to avoid writing into the newly selected profile history store"
"Expected stale completion callbacks to avoid writing into the newly selected profile history store, found \(alternateStore.entries.map { $0.url })"
)
}
}

18
cmuxTests/GhosttyConfigTests.swift
View file

@ -2004,16 +2004,20 @@ final class BrowserInstallDetectorTests: XCTestCase {
return
}
XCTAssertEqual(safari.profiles.map(\.displayName), ["Default", "Work", "Travel"])
XCTAssertEqual(Set(safari.profiles.map(\.displayName)), Set(["Default", "Work", "Travel"]))
XCTAssertEqual(
safari.profiles.map { $0.rootURL.path(percentEncoded: false) }.sorted(),
safari.profiles
.map { $0.rootURL.standardizedFileURL.resolvingSymlinksInPath().path(percentEncoded: false) }
.sorted(),
[
home.appendingPathComponent("Library/Safari", isDirectory: true).path(percentEncoded: false),
home.appendingPathComponent("Library/Safari/Profiles/Work", isDirectory: true).path(percentEncoded: false),
home.appendingPathComponent("Library/Safari", isDirectory: true)
.standardizedFileURL.resolvingSymlinksInPath().path(percentEncoded: false),
home.appendingPathComponent("Library/Safari/Profiles/Work", isDirectory: true)
.standardizedFileURL.resolvingSymlinksInPath().path(percentEncoded: false),
home.appendingPathComponent(
"Library/Containers/com.apple.Safari/Data/Library/Safari/Profiles/Travel",
isDirectory: true
).path(percentEncoded: false),
).standardizedFileURL.resolvingSymlinksInPath().path(percentEncoded: false),
].sorted()
)
}
@ -2024,7 +2028,9 @@ final class BrowserInstallDetectorTests: XCTestCase {
private func createFile(at url: URL, contents: Data) throws {
try FileManager.default.createDirectory(at: url.deletingLastPathComponent(), withIntermediateDirectories: true)
_ = FileManager.default.createFile(atPath: url.path, contents: contents)
guard FileManager.default.createFile(atPath: url.path, contents: contents) else {
throw CocoaError(.fileWriteUnknown)
}
}
}

2
cmuxTests/SessionPersistenceTests.swift
View file

@ -150,7 +150,7 @@ final class SessionPersistenceTests: XCTestCase {
}
func testSessionBrowserPanelSnapshotHistoryRoundTrip() throws {
let profileID = UUID(uuidString: "8F03A658-5A84-428B-AD03-5A6D04692F64")
let profileID = try XCTUnwrap(UUID(uuidString: "8F03A658-5A84-428B-AD03-5A6D04692F64"))
let source = SessionBrowserPanelSnapshot(
urlString: "https://example.com/current",
profileID: profileID,

7
cmuxUITests/BrowserImportProfilesUITests.swift
View file

@ -138,12 +138,7 @@ final class BrowserImportProfilesUITests: XCTestCase {
}
RunLoop.current.run(until: Date().addingTimeInterval(0.05))
}
guard let data = try? Data(contentsOf: url),
let object = try? JSONSerialization.jsonObject(with: data) as? [String: Any] else {
return nil
}
return object
return nil
}
private func ensureForegroundAfterLaunch(_ app: XCUIApplication, timeout: TimeInterval) -> Bool {