Skip to content

Commit dff4c63

Browse files
steipetesteipete
authored
fix(macos): keep dashboard tab reuse and ordering accurate (#103464)
* fix(macos): preserve dashboard tab identity * test(macos): strengthen dashboard tab regressions * fix(macos): retire failed dashboard navigation aliases
1 parent 401f278 commit dff4c63

4 files changed

Lines changed: 248 additions & 25 deletions

File tree

‎apps/macos/Sources/OpenClaw/DashboardLinkBrowserTabBar.swift‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,24 @@ final class DashboardLinkBrowserTabBar: NSView {
7474
let location = self.stackView.convert(event.locationInWindow, from: nil)
7575
let arrangedItems = self.stackView.arrangedSubviews.compactMap { $0 as? DashboardLinkBrowserTabItemView }
7676
guard let currentIndex = arrangedItems.firstIndex(of: item) else { return }
77+
let midpoints = arrangedItems.map(\.frame.midX)
78+
guard let targetIndex = Self.dropIndex(
79+
currentIndex: currentIndex,
80+
itemMidpoints: midpoints,
81+
locationX: location.x)
82+
else { return }
83+
self.delegate?.tabBar(self, didMoveTab: id, toIndex: targetIndex)
84+
}
7785

78-
var targetIndex = arrangedItems.count - 1
79-
for (index, candidate) in arrangedItems.enumerated() where location.x < candidate.frame.midX {
80-
targetIndex = index
81-
break
86+
static func dropIndex(currentIndex: Int, itemMidpoints: [CGFloat], locationX: CGFloat) -> Int? {
87+
guard itemMidpoints.indices.contains(currentIndex) else { return nil }
88+
// Delegate indexes describe the array after removal. Excluding the dragged
89+
// item keeps rightward drops from advancing one tab too far.
90+
let remainingMidpoints = itemMidpoints.enumerated().compactMap { index, midpoint in
91+
index == currentIndex ? nil : midpoint
8292
}
83-
guard targetIndex != currentIndex else { return }
84-
self.delegate?.tabBar(self, didMoveTab: id, toIndex: targetIndex)
93+
let targetIndex = remainingMidpoints.firstIndex { locationX < $0 } ?? remainingMidpoints.count
94+
return targetIndex == currentIndex ? nil : targetIndex
8595
}
8696

8797
fileprivate func contextMenu(forTab id: UUID) -> NSMenu? {

‎apps/macos/Sources/OpenClaw/DashboardLinkBrowserView.swift‎

Lines changed: 113 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,79 @@ import WebKit
44

55
@MainActor
66
private final class DashboardLinkBrowserTab {
7+
/// WebKit preserves one WKNavigation identity across a redirect chain.
8+
/// A different identity means the opened-link alias is no longer current.
9+
private enum RequestAliasPhase {
10+
case awaitingNavigation
11+
case loading(AnyObject)
12+
case retained
13+
case retired
14+
}
15+
716
let id = UUID()
817
let webView: WKWebView
18+
let requestedURL: URL
919
var representedURL: URL?
1020
var title: String?
1121
var observations: [NSKeyValueObservation] = []
22+
private var requestAliasPhase: RequestAliasPhase = .awaitingNavigation
1223

13-
init(webView: WKWebView, representedURL: URL?) {
24+
init(webView: WKWebView, requestedURL: URL) {
1425
self.webView = webView
15-
self.representedURL = representedURL
26+
self.requestedURL = requestedURL
27+
self.representedURL = requestedURL
28+
}
29+
30+
var requestedURLAlias: URL? {
31+
if case .retired = self.requestAliasPhase {
32+
nil
33+
} else {
34+
self.requestedURL
35+
}
36+
}
37+
38+
func startNavigation(_ navigation: AnyObject) {
39+
switch self.requestAliasPhase {
40+
case .awaitingNavigation:
41+
self.requestAliasPhase = .loading(navigation)
42+
case let .loading(initial) where initial !== navigation:
43+
self.requestAliasPhase = .retired
44+
case .retained:
45+
self.requestAliasPhase = .retired
46+
case .loading, .retired:
47+
break
48+
}
49+
}
50+
51+
func updateRepresentedURL(_ url: URL?) {
52+
// Initial redirects keep the opened link reusable. Once that chain
53+
// finishes, a distinct navigation retires the now-stale alias.
54+
if case .retained = self.requestAliasPhase, let url, url != self.representedURL {
55+
self.requestAliasPhase = .retired
56+
}
57+
self.representedURL = url
58+
}
59+
60+
func finishNavigation(_ navigation: AnyObject?, at url: URL?, title: String?) {
61+
self.updateRepresentedURL(url)
62+
self.title = title
63+
guard let navigation else { return }
64+
switch self.requestAliasPhase {
65+
case .awaitingNavigation:
66+
self.requestAliasPhase = .retained
67+
case let .loading(initial) where initial === navigation:
68+
self.requestAliasPhase = .retained
69+
case .loading:
70+
self.requestAliasPhase = .retired
71+
case .retained, .retired:
72+
break
73+
}
74+
}
75+
76+
func failNavigation() {
77+
// A failed initial chain has no reusable page. Retire its alias so
78+
// opening the original link again starts a fresh load.
79+
self.requestAliasPhase = .retired
1680
}
1781
}
1882

@@ -83,17 +147,19 @@ final class DashboardLinkBrowserView: NSView {
83147
}
84148

85149
func open(_ url: URL) {
86-
// Repeat clicks on the exact same chat link reuse its tab so inline
87-
// browsing does not pile up duplicates.
88-
if let tab = self.tabs.first(where: { $0.representedURL == url }) {
150+
// Keep the original request as a stable dedupe key across redirects while
151+
// also reusing a tab that has since navigated to the requested target.
152+
let tab = self.tabs.first(where: { $0.representedURL == url }) ??
153+
self.tabs.first(where: { $0.requestedURLAlias == url })
154+
if let tab {
89155
self.activateTab(id: tab.id)
90156
return
91157
}
92158
self.openInNewTab(url)
93159
}
94160

95161
func openInNewTab(_ url: URL) {
96-
let tab = self.makeTab(representedURL: url)
162+
let tab = self.makeTab(requestedURL: url)
97163
self.tabs.append(tab)
98164
self.tabBar.appendTab(id: tab.id, title: self.displayTitle(for: tab), toolTip: url.absoluteString)
99165
self.activateTab(id: tab.id)
@@ -169,14 +235,39 @@ final class DashboardLinkBrowserView: NSView {
169235

170236
func navigationWillStart(_ url: URL, in webView: WKWebView) {
171237
guard let tab = self.tab(owning: webView) else { return }
172-
tab.representedURL = url
238+
tab.updateRepresentedURL(url)
239+
self.refreshTab(tab)
240+
}
241+
242+
func navigationDidStart(_ navigation: WKNavigation?, in webView: WKWebView) {
243+
guard let navigation, let tab = self.tab(owning: webView) else { return }
244+
tab.startNavigation(navigation)
245+
}
246+
247+
func navigationDidFinish(_ navigation: WKNavigation?, for webView: WKWebView) {
248+
self.finishNavigation(navigation, at: webView.url, title: webView.title, in: webView)
249+
}
250+
251+
func navigationDidFail(for webView: WKWebView) {
252+
guard let tab = self.tab(owning: webView) else { return }
253+
tab.failNavigation()
254+
self.updateChrome()
255+
}
256+
257+
func navigationURLDidChange(for webView: WKWebView) {
258+
guard let tab = self.tab(owning: webView) else { return }
259+
tab.updateRepresentedURL(webView.url)
173260
self.refreshTab(tab)
174261
}
175262

176-
func navigationDidFinish(for webView: WKWebView) {
263+
private func finishNavigation(
264+
_ navigation: AnyObject?,
265+
at url: URL?,
266+
title: String?,
267+
in webView: WKWebView)
268+
{
177269
guard let tab = self.tab(owning: webView) else { return }
178-
tab.representedURL = webView.url
179-
tab.title = webView.title
270+
tab.finishNavigation(navigation, at: url, title: title)
180271
self.refreshTab(tab)
181272
}
182273

@@ -197,11 +288,11 @@ final class DashboardLinkBrowserView: NSView {
197288
return webView
198289
}
199290

200-
private func makeTab(representedURL: URL?) -> DashboardLinkBrowserTab {
291+
private func makeTab(requestedURL: URL) -> DashboardLinkBrowserTab {
201292
let webView = Self.makeWebView(websiteDataStore: self.websiteDataStore)
202293
webView.navigationDelegate = self.webViewNavigationDelegate
203294
webView.uiDelegate = self.webViewUIDelegate
204-
let tab = DashboardLinkBrowserTab(webView: webView, representedURL: representedURL)
295+
let tab = DashboardLinkBrowserTab(webView: webView, requestedURL: requestedURL)
205296
self.installWebView(webView)
206297
self.observeNavigationState(for: tab)
207298
return tab
@@ -284,7 +375,7 @@ final class DashboardLinkBrowserView: NSView {
284375
webView.observe(\.url, options: [.new]) { [weak self, weak webView] _, _ in
285376
Task { @MainActor in
286377
guard let self, let webView else { return }
287-
self.navigationDidFinish(for: webView)
378+
self.navigationURLDidChange(for: webView)
288379
}
289380
},
290381
webView.observe(\.title, options: [.new]) { [weak self, weak tab, weak webView] _, _ in
@@ -562,6 +653,15 @@ extension DashboardLinkBrowserView {
562653
self.openInNewTab(url)
563654
}
564655

656+
func _testStartNavigation(_ navigation: AnyObject, in webView: WKWebView) {
657+
guard let tab = self.tab(owning: webView) else { return }
658+
tab.startNavigation(navigation)
659+
}
660+
661+
func _testFinishNavigation(_ navigation: AnyObject, at url: URL?, in webView: WKWebView) {
662+
self.finishNavigation(navigation, at: url, title: webView.title, in: webView)
663+
}
664+
565665
func _testContextMenu(forTabAt index: Int) -> NSMenu? {
566666
self.contextMenu(forTabAt: index)
567667
}

‎apps/macos/Sources/OpenClaw/DashboardWindowController.swift‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -672,21 +672,22 @@ final class DashboardWindowController: NSWindowController, WKNavigationDelegate,
672672
decisionHandler(.cancel)
673673
}
674674

675-
func webView(_ webView: WKWebView, didStartProvisionalNavigation _: WKNavigation!) {
675+
func webView(_ webView: WKWebView, didStartProvisionalNavigation navigation: WKNavigation!) {
676676
if self.linkBrowser.owns(webView) {
677+
self.linkBrowser.navigationDidStart(navigation, in: webView)
677678
self.linkBrowser.updateChrome()
678679
}
679680
}
680681

681-
func webView(_ webView: WKWebView, didFinish _: WKNavigation!) {
682+
func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) {
682683
if self.linkBrowser.owns(webView) {
683-
self.linkBrowser.navigationDidFinish(for: webView)
684+
self.linkBrowser.navigationDidFinish(navigation, for: webView)
684685
}
685686
}
686687

687688
func webView(_ webView: WKWebView, didFail _: WKNavigation!, withError error: Error) {
688689
if self.linkBrowser.owns(webView) {
689-
self.linkBrowser.updateChrome()
690+
self.linkBrowser.navigationDidFail(for: webView)
690691
return
691692
}
692693
guard webView === self.webView else { return }
@@ -699,7 +700,7 @@ final class DashboardWindowController: NSWindowController, WKNavigationDelegate,
699700
withError error: Error)
700701
{
701702
if self.linkBrowser.owns(webView) {
702-
self.linkBrowser.updateChrome()
703+
self.linkBrowser.navigationDidFail(for: webView)
703704
return
704705
}
705706
guard webView === self.webView else { return }

‎apps/macos/Tests/OpenClawIPCTests/DashboardWindowSmokeTests.swift‎

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,13 +208,125 @@ struct DashboardWindowSmokeTests {
208208
#expect(controller._testLinkBrowserActiveTabIndex == 0)
209209
}
210210

211+
@Test func `dashboard link browser calculates final drag insertion indexes`() {
212+
let midpoints: [CGFloat] = [50, 150, 250]
213+
let cases: [(currentIndex: Int, locationX: CGFloat, targetIndex: Int?, order: [Int])] = [
214+
(0, 100, nil, [0, 1, 2]),
215+
(0, 150, 1, [1, 0, 2]),
216+
(0, 200, 1, [1, 0, 2]),
217+
(0, 300, 2, [1, 2, 0]),
218+
(2, 100, 1, [0, 2, 1]),
219+
(2, 0, 0, [2, 0, 1]),
220+
(1, 150, nil, [0, 1, 2]),
221+
]
222+
for testCase in cases {
223+
let targetIndex = DashboardLinkBrowserTabBar.dropIndex(
224+
currentIndex: testCase.currentIndex,
225+
itemMidpoints: midpoints,
226+
locationX: testCase.locationX)
227+
#expect(targetIndex == testCase.targetIndex)
228+
229+
var order = Array(midpoints.indices)
230+
if let targetIndex {
231+
let moved = order.remove(at: testCase.currentIndex)
232+
order.insert(moved, at: targetIndex)
233+
}
234+
#expect(order == testCase.order)
235+
}
236+
}
237+
238+
@Test func `dashboard link browser retires initial URL after later navigation`() throws {
239+
let view = DashboardLinkBrowserView(websiteDataStore: .default())
240+
defer { view.closeBrowser() }
241+
let requestedURL = try #require(URL(string: "http://127.0.0.1:1/short"))
242+
let currentURL = try #require(URL(string: "http://127.0.0.1:1/final"))
243+
view.open(requestedURL)
244+
let webView = try #require(view._testActiveWebView)
245+
let initialNavigation = NSObject()
246+
view._testStartNavigation(initialNavigation, in: webView)
247+
view.navigationWillStart(currentURL, in: webView)
248+
249+
view.open(requestedURL)
250+
#expect(view._testTabCount == 1)
251+
#expect(view._testActiveWebView === webView)
252+
253+
view.open(currentURL)
254+
#expect(view._testTabCount == 1)
255+
#expect(view._testActiveWebView === webView)
256+
257+
view._testFinishNavigation(initialNavigation, at: currentURL, in: webView)
258+
view.open(requestedURL)
259+
#expect(view._testTabCount == 1)
260+
#expect(view._testActiveWebView === webView)
261+
262+
view._testStartNavigation(NSObject(), in: webView)
263+
view.navigationWillStart(currentURL, in: webView)
264+
view.open(requestedURL)
265+
#expect(view._testTabCount == 2)
266+
#expect(view._testActiveWebView !== webView)
267+
}
268+
269+
@Test func `dashboard link browser retires initial URL when navigation is replaced`() throws {
270+
let view = DashboardLinkBrowserView(websiteDataStore: .default())
271+
defer { view.closeBrowser() }
272+
let requestedURL = try #require(URL(string: "http://127.0.0.1:1/short"))
273+
let redirectURL = try #require(URL(string: "http://127.0.0.1:1/redirect"))
274+
let replacementURL = try #require(URL(string: "http://127.0.0.1:1/replacement"))
275+
view.open(requestedURL)
276+
let webView = try #require(view._testActiveWebView)
277+
view._testStartNavigation(NSObject(), in: webView)
278+
view.navigationWillStart(redirectURL, in: webView)
279+
view._testStartNavigation(NSObject(), in: webView)
280+
view.navigationWillStart(replacementURL, in: webView)
281+
282+
view.open(requestedURL)
283+
284+
#expect(view._testTabCount == 2)
285+
#expect(view._testActiveWebView !== webView)
286+
}
287+
288+
@Test func `dashboard link browser retires initial URL when redirected navigation fails`() throws {
289+
let view = DashboardLinkBrowserView(websiteDataStore: .default())
290+
defer { view.closeBrowser() }
291+
let requestedURL = try #require(URL(string: "http://127.0.0.1:1/short"))
292+
let redirectURL = try #require(URL(string: "http://127.0.0.1:1/redirect"))
293+
view.open(requestedURL)
294+
let webView = try #require(view._testActiveWebView)
295+
view._testStartNavigation(NSObject(), in: webView)
296+
view.navigationWillStart(redirectURL, in: webView)
297+
view.navigationDidFail(for: webView)
298+
299+
view.open(requestedURL)
300+
301+
#expect(view._testTabCount == 2)
302+
#expect(view._testActiveWebView !== webView)
303+
}
304+
305+
@Test func `dashboard link browser prefers current URL over initial alias`() throws {
306+
let view = DashboardLinkBrowserView(websiteDataStore: .default())
307+
defer { view.closeBrowser() }
308+
let requestedURL = try #require(URL(string: "http://127.0.0.1:1/short"))
309+
let currentURL = try #require(URL(string: "http://127.0.0.1:1/final"))
310+
view.open(requestedURL)
311+
let redirectedWebView = try #require(view._testActiveWebView)
312+
view.navigationWillStart(currentURL, in: redirectedWebView)
313+
view._testOpenInNewTab(requestedURL)
314+
let currentWebView = try #require(view._testActiveWebView)
315+
view._testSelectTab(at: 0)
316+
317+
view.open(requestedURL)
318+
319+
#expect(view._testTabCount == 2)
320+
#expect(view._testActiveWebView === currentWebView)
321+
}
322+
211323
@Test func `dashboard link browser menu disables URL actions for blank tab`() throws {
212324
let view = DashboardLinkBrowserView(websiteDataStore: .default())
213325
let url = try #require(URL(string: "http://127.0.0.1:1/blank"))
214326
view.open(url)
215327
let webView = try #require(view._testActiveWebView)
216328
#expect(webView.url == nil)
217-
view.navigationDidFinish(for: webView)
329+
view.navigationURLDidChange(for: webView)
218330
let menu = try #require(view._testContextMenu(forTabAt: 0))
219331
#expect(!menu.items[0].isEnabled)
220332
#expect(!menu.items[1].isEnabled)

0 commit comments

Comments
 (0)