Fix browser pane drag/drop follow-ups
This commit is contained in:
parent
1dbd1e5011
commit
8aafb68935
5 changed files with 67 additions and 31 deletions
|
|
@ -2856,6 +2856,7 @@ final class WindowBrowserPortal: NSObject {
|
|||
containerView.setPaneTopChromeHeight(0)
|
||||
containerView.setSearchOverlay(nil)
|
||||
containerView.setPaneDropContext(nil)
|
||||
containerView.setPortalDragDropZone(nil)
|
||||
containerView.setDropZoneOverlay(zone: nil)
|
||||
if !containerView.isHidden, webView.superview === containerView {
|
||||
webView.browserPortalNotifyHidden(reason: reason)
|
||||
|
|
@ -2887,6 +2888,7 @@ final class WindowBrowserPortal: NSObject {
|
|||
)
|
||||
#endif
|
||||
containerView.setPaneDropContext(nil)
|
||||
containerView.setPortalDragDropZone(nil)
|
||||
containerView.setDropZoneOverlay(zone: nil)
|
||||
return true
|
||||
}
|
||||
|
|
@ -3031,6 +3033,7 @@ final class WindowBrowserPortal: NSObject {
|
|||
)
|
||||
#endif
|
||||
containerView.setPaneDropContext(nil)
|
||||
containerView.setPortalDragDropZone(nil)
|
||||
containerView.setDropZoneOverlay(zone: nil)
|
||||
return
|
||||
}
|
||||
|
|
@ -3096,7 +3099,8 @@ final class WindowBrowserPortal: NSObject {
|
|||
!containerView.isHidden
|
||||
let recoveredFromTransientGeometry =
|
||||
previousTransientRecoveryReason != nil &&
|
||||
transientRecoveryReason == nil
|
||||
transientRecoveryReason == nil &&
|
||||
!shouldHide
|
||||
#if DEBUG
|
||||
let frameWasClamped = hasFiniteFrame && !Self.rectApproximatelyEqual(frameInHost, targetFrame)
|
||||
if frameWasClamped {
|
||||
|
|
@ -3139,6 +3143,7 @@ final class WindowBrowserPortal: NSObject {
|
|||
if hasExistingVisibleFrame {
|
||||
containerView.setDropZoneOverlay(zone: nil)
|
||||
containerView.setPaneDropContext(nil)
|
||||
containerView.setPortalDragDropZone(nil)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
|
@ -3268,7 +3273,7 @@ final class WindowBrowserPortal: NSObject {
|
|||
containerOwnsWebView &&
|
||||
hostView.reapplyHostedInspectorDividerIfNeeded(in: containerView, reason: "portal.sync")
|
||||
if !shouldHide, containerOwnsWebView, !refreshReasons.isEmpty {
|
||||
if hostedInspectorAdjustedDuringSync {
|
||||
if hostedInspectorAdjustedDuringSync && !recoveredFromTransientGeometry {
|
||||
#if DEBUG
|
||||
dlog(
|
||||
"browser.portal.refresh.skip web=\(browserPortalDebugToken(webView)) " +
|
||||
|
|
|
|||
|
|
@ -2005,6 +2005,12 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
setupObservers(for: webView)
|
||||
}
|
||||
|
||||
private func isCurrentWebView(_ candidate: WKWebView, instanceID: UUID? = nil) -> Bool {
|
||||
guard candidate === webView else { return false }
|
||||
guard let instanceID else { return true }
|
||||
return instanceID == webViewInstanceID
|
||||
}
|
||||
|
||||
init(workspaceId: UUID, initialURL: URL? = nil, bypassInsecureHTTPHostOnce: String? = nil) {
|
||||
self.id = UUID()
|
||||
self.workspaceId = workspaceId
|
||||
|
|
@ -2020,15 +2026,16 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
navDelegate.didFinish = { webView in
|
||||
BrowserHistoryStore.shared.recordVisit(url: webView.url, title: webView.title)
|
||||
Task { @MainActor [weak self] in
|
||||
self?.refreshFavicon(from: webView)
|
||||
self?.applyBrowserThemeModeIfNeeded()
|
||||
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)
|
||||
self.restoreFindStateAfterNavigation(replaySearch: true)
|
||||
}
|
||||
}
|
||||
navDelegate.didFailNavigation = { [weak self] _, failedURL in
|
||||
navDelegate.didFailNavigation = { [weak self] failedWebView, failedURL in
|
||||
Task { @MainActor in
|
||||
guard let self else { return }
|
||||
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
|
||||
|
|
@ -2162,10 +2169,13 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
}
|
||||
|
||||
private func setupObservers(for webView: WKWebView) {
|
||||
let observedWebViewInstanceID = webViewInstanceID
|
||||
|
||||
// URL changes
|
||||
let urlObserver = webView.observe(\.url, options: [.new]) { [weak self] webView, _ in
|
||||
Task { @MainActor in
|
||||
self?.currentURL = webView.url
|
||||
guard let self, self.isCurrentWebView(webView, instanceID: observedWebViewInstanceID) else { return }
|
||||
self.currentURL = webView.url
|
||||
}
|
||||
}
|
||||
webViewObservers.append(urlObserver)
|
||||
|
|
@ -2173,12 +2183,13 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
// Title changes
|
||||
let titleObserver = webView.observe(\.title, options: [.new]) { [weak self] webView, _ in
|
||||
Task { @MainActor in
|
||||
guard let self, self.isCurrentWebView(webView, instanceID: observedWebViewInstanceID) else { return }
|
||||
// Keep showing the last non-empty title while the new navigation is loading.
|
||||
// WebKit often clears title to nil/"" during reload/navigation, which causes
|
||||
// a distracting tab-title flash (e.g. to host/URL). Only accept non-empty titles.
|
||||
let trimmed = (webView.title ?? "").trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
guard !trimmed.isEmpty else { return }
|
||||
self?.pageTitle = trimmed
|
||||
self.pageTitle = trimmed
|
||||
}
|
||||
}
|
||||
webViewObservers.append(titleObserver)
|
||||
|
|
@ -2186,7 +2197,8 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
// Loading state
|
||||
let loadingObserver = webView.observe(\.isLoading, options: [.new]) { [weak self] webView, _ in
|
||||
Task { @MainActor in
|
||||
self?.handleWebViewLoadingChanged(webView.isLoading)
|
||||
guard let self, self.isCurrentWebView(webView, instanceID: observedWebViewInstanceID) else { return }
|
||||
self.handleWebViewLoadingChanged(webView.isLoading)
|
||||
}
|
||||
}
|
||||
webViewObservers.append(loadingObserver)
|
||||
|
|
@ -2194,7 +2206,7 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
// Can go back
|
||||
let backObserver = webView.observe(\.canGoBack, options: [.new]) { [weak self] webView, _ in
|
||||
Task { @MainActor in
|
||||
guard let self else { return }
|
||||
guard let self, self.isCurrentWebView(webView, instanceID: observedWebViewInstanceID) else { return }
|
||||
self.nativeCanGoBack = webView.canGoBack
|
||||
self.refreshNavigationAvailability()
|
||||
}
|
||||
|
|
@ -2204,7 +2216,7 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
// Can go forward
|
||||
let forwardObserver = webView.observe(\.canGoForward, options: [.new]) { [weak self] webView, _ in
|
||||
Task { @MainActor in
|
||||
guard let self else { return }
|
||||
guard let self, self.isCurrentWebView(webView, instanceID: observedWebViewInstanceID) else { return }
|
||||
self.nativeCanGoForward = webView.canGoForward
|
||||
self.refreshNavigationAvailability()
|
||||
}
|
||||
|
|
@ -2214,7 +2226,8 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
// Progress
|
||||
let progressObserver = webView.observe(\.estimatedProgress, options: [.new]) { [weak self] webView, _ in
|
||||
Task { @MainActor in
|
||||
self?.estimatedProgress = webView.estimatedProgress
|
||||
guard let self, self.isCurrentWebView(webView, instanceID: observedWebViewInstanceID) else { return }
|
||||
self.estimatedProgress = webView.estimatedProgress
|
||||
}
|
||||
}
|
||||
webViewObservers.append(progressObserver)
|
||||
|
|
@ -2250,6 +2263,9 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
|
||||
webViewObservers.removeAll()
|
||||
webViewCancellables.removeAll()
|
||||
faviconTask?.cancel()
|
||||
faviconTask = nil
|
||||
faviconRefreshGeneration &+= 1
|
||||
BrowserWindowPortalRegistry.detach(webView: terminatedWebView)
|
||||
terminatedWebView.stopLoading()
|
||||
terminatedWebView.navigationDelegate = nil
|
||||
|
|
@ -2260,11 +2276,12 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
|
||||
let replacement = Self.makeWebView()
|
||||
replacement.pageZoom = desiredZoom
|
||||
webView = replacement
|
||||
webViewInstanceID = UUID()
|
||||
webView = replacement
|
||||
shouldRenderWebView = wasRenderable
|
||||
|
||||
bindWebView(replacement)
|
||||
applyBrowserThemeModeIfNeeded()
|
||||
|
||||
if !history.backHistoryURLStrings.isEmpty || !history.forwardHistoryURLStrings.isEmpty {
|
||||
restoreSessionNavigationHistory(
|
||||
|
|
@ -2360,9 +2377,11 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
guard let scheme = pageURL.scheme?.lowercased(), scheme == "http" || scheme == "https" else { return }
|
||||
faviconRefreshGeneration &+= 1
|
||||
let refreshGeneration = faviconRefreshGeneration
|
||||
let refreshWebViewInstanceID = webViewInstanceID
|
||||
|
||||
faviconTask = Task { @MainActor [weak self, weak webView] in
|
||||
guard let self, let webView else { return }
|
||||
guard self.isCurrentWebView(webView, instanceID: refreshWebViewInstanceID) else { return }
|
||||
guard self.isCurrentFaviconRefresh(generation: refreshGeneration) else { return }
|
||||
|
||||
// Try to discover the best icon URL from the document.
|
||||
|
|
@ -2397,6 +2416,7 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
discoveredURL = u
|
||||
}
|
||||
}
|
||||
guard self.isCurrentWebView(webView, instanceID: refreshWebViewInstanceID) else { return }
|
||||
guard self.isCurrentFaviconRefresh(generation: refreshGeneration) else { return }
|
||||
|
||||
let fallbackURL = URL(string: "/favicon.ico", relativeTo: pageURL)
|
||||
|
|
@ -2422,6 +2442,7 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
} catch {
|
||||
return
|
||||
}
|
||||
guard self.isCurrentWebView(webView, instanceID: refreshWebViewInstanceID) else { return }
|
||||
guard self.isCurrentFaviconRefresh(generation: refreshGeneration) else { return }
|
||||
|
||||
guard let http = response as? HTTPURLResponse,
|
||||
|
|
@ -2701,12 +2722,12 @@ final class BrowserPanel: Panel, ObservableObject {
|
|||
if let detachedDeveloperToolsWindowCloseObserver {
|
||||
NotificationCenter.default.removeObserver(detachedDeveloperToolsWindowCloseObserver)
|
||||
}
|
||||
webViewObservers.removeAll()
|
||||
webViewCancellables.removeAll()
|
||||
let webView = webView
|
||||
Task { @MainActor in
|
||||
BrowserWindowPortalRegistry.detach(webView: webView)
|
||||
}
|
||||
webViewObservers.removeAll()
|
||||
webViewCancellables.removeAll()
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -2761,6 +2782,9 @@ extension BrowserPanel {
|
|||
|
||||
loadingEndWorkItem?.cancel()
|
||||
loadingEndWorkItem = nil
|
||||
faviconTask?.cancel()
|
||||
faviconTask = nil
|
||||
faviconRefreshGeneration &+= 1
|
||||
loadingGeneration &+= 1
|
||||
activeDownloadCount = 0
|
||||
isDownloading = false
|
||||
|
|
@ -2800,10 +2824,11 @@ extension BrowserPanel {
|
|||
}
|
||||
|
||||
let replacement = Self.makeWebView()
|
||||
webView = replacement
|
||||
webViewInstanceID = UUID()
|
||||
webView = replacement
|
||||
shouldRenderWebView = false
|
||||
bindWebView(replacement)
|
||||
applyBrowserThemeModeIfNeeded()
|
||||
refreshNavigationAvailability()
|
||||
|
||||
#if DEBUG
|
||||
|
|
|
|||
|
|
@ -5523,12 +5523,11 @@ struct WebViewRepresentable: NSViewRepresentable {
|
|||
}
|
||||
|
||||
// SwiftUI can transiently dismantle/rebuild the browser host view during split
|
||||
// rearrangement. Do not detach the portal-hosted WKWebView here; explicit detach
|
||||
// still happens on real web view replacement and panel teardown.
|
||||
// rearrangement. Do not detach the portal-hosted WKWebView or clear its pane-drop
|
||||
// context here; explicit teardown still happens on real web view replacement and
|
||||
// panel teardown, and preserving this state lets internal tab drags re-enter the
|
||||
// browser pane while SwiftUI churns underneath.
|
||||
BrowserWindowPortalRegistry.updateDropZoneOverlay(for: webView, zone: nil)
|
||||
BrowserWindowPortalRegistry.updatePaneTopChromeHeight(for: webView, height: 0)
|
||||
BrowserWindowPortalRegistry.updatePaneDropContext(for: webView, context: nil)
|
||||
BrowserWindowPortalRegistry.updateSearchOverlay(for: webView, configuration: nil)
|
||||
coordinator.lastPortalHostId = nil
|
||||
coordinator.lastSynchronizedHostGeometryRevision = 0
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1713,29 +1713,23 @@ final class Workspace: Identifiable, ObservableObject {
|
|||
|
||||
for browserPanel in browserPanels {
|
||||
browserPanel.resetForWorkspaceContextChange(reason: reason)
|
||||
let nextTitle = browserPanel.displayTitle
|
||||
_ = updatePanelTitle(panelId: browserPanel.id, title: nextTitle)
|
||||
|
||||
guard let tabId = surfaceIdFromPanelId(browserPanel.id),
|
||||
let existing = bonsplitController.tab(tabId) else {
|
||||
continue
|
||||
}
|
||||
|
||||
let nextTitle = browserPanel.displayTitle
|
||||
if panelTitles[browserPanel.id] != nextTitle {
|
||||
panelTitles[browserPanel.id] = nextTitle
|
||||
}
|
||||
|
||||
let resolvedTitle = resolvedPanelTitle(panelId: browserPanel.id, fallback: nextTitle)
|
||||
let titleUpdate: String? = existing.title == resolvedTitle ? nil : resolvedTitle
|
||||
let faviconUpdate: Data?? = existing.iconImageData == nil ? nil : .some(nil)
|
||||
let loadingUpdate: Bool? = existing.isLoading ? false : nil
|
||||
|
||||
guard titleUpdate != nil || faviconUpdate != nil || loadingUpdate != nil else {
|
||||
guard faviconUpdate != nil || loadingUpdate != nil else {
|
||||
continue
|
||||
}
|
||||
|
||||
bonsplitController.updateTab(
|
||||
tabId,
|
||||
title: titleUpdate,
|
||||
iconImageData: faviconUpdate,
|
||||
hasCustomTitle: panelCustomTitles[browserPanel.id] != nil,
|
||||
isLoading: loadingUpdate
|
||||
|
|
|
|||
|
|
@ -2466,6 +2466,14 @@ final class BrowserSessionHistoryRestoreTests: XCTestCase {
|
|||
url: try XCTUnwrap(URL(string: "https://example.com/pull/1208")),
|
||||
status: .open
|
||||
)
|
||||
workspace.logEntries.append(
|
||||
SidebarLogEntry(
|
||||
message: "Issue #1208",
|
||||
level: .info,
|
||||
source: "test",
|
||||
timestamp: Date()
|
||||
)
|
||||
)
|
||||
workspace.surfaceListeningPorts[contextPanelId] = [3000]
|
||||
workspace.recomputeListeningPorts()
|
||||
|
||||
|
|
@ -2475,12 +2483,15 @@ final class BrowserSessionHistoryRestoreTests: XCTestCase {
|
|||
XCTAssertTrue(browser.canGoForward)
|
||||
XCTAssertNotNil(browser.searchState)
|
||||
XCTAssertFalse(workspace.statusEntries.isEmpty)
|
||||
XCTAssertFalse(workspace.logEntries.isEmpty)
|
||||
XCTAssertFalse(workspace.metadataBlocks.isEmpty)
|
||||
XCTAssertNotNil(workspace.progress)
|
||||
XCTAssertNotNil(workspace.gitBranch)
|
||||
XCTAssertNotNil(workspace.pullRequest)
|
||||
XCTAssertEqual(workspace.listeningPorts, [3000])
|
||||
|
||||
let priorWebView = browser.webView
|
||||
let priorInstanceID = browser.webViewInstanceID
|
||||
workspace.resetSidebarContext(reason: "test")
|
||||
|
||||
XCTAssertTrue(workspace.statusEntries.isEmpty)
|
||||
|
|
@ -2498,6 +2509,8 @@ final class BrowserSessionHistoryRestoreTests: XCTestCase {
|
|||
XCTAssertFalse(browser.canGoBack)
|
||||
XCTAssertFalse(browser.canGoForward)
|
||||
XCTAssertNil(browser.searchState)
|
||||
XCTAssertFalse(browser.webView === priorWebView)
|
||||
XCTAssertNotEqual(browser.webViewInstanceID, priorInstanceID)
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue