diff --git a/ora/Core/BrowserEngine/BrowserPageView.swift b/ora/Core/BrowserEngine/BrowserPageView.swift index 599faa25..cbc4e36d 100644 --- a/ora/Core/BrowserEngine/BrowserPageView.swift +++ b/ora/Core/BrowserEngine/BrowserPageView.swift @@ -1,6 +1,121 @@ import AppKit import SwiftUI +final class BrowserPageHostView: NSView { + private(set) weak var hostedContentView: NSView? + + override init(frame frameRect: NSRect) { + super.init(frame: frameRect) + wantsLayer = true + autoresizesSubviews = true + } + + @available(*, unavailable) + required init?(coder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } + + func host(contentView newContentView: NSView?) { + let previousContentView = hostedContentView + let isSameContentView = previousContentView === newContentView + + if isSameContentView, newContentView?.superview === self { + return + } + + let shouldRestoreFirstResponder = shouldTransferFirstResponder(from: previousContentView) + + if isSameContentView { + hostedContentView = nil + } else { + detachHostedContentView() + } + + guard let newContentView else { + refreshHostingLayout() + return + } + + configure(contentView: newContentView) + + if let previousHost = newContentView.superview as? BrowserPageHostView, + previousHost !== self + { + previousHost.releaseHostedContentView(newContentView) + } + + if newContentView.superview !== self { + if newContentView.superview != nil { + newContentView.removeFromSuperview() + } + addSubview(newContentView) + } + + newContentView.frame = bounds + hostedContentView = newContentView + + needsLayout = true + layoutSubtreeIfNeeded() + newContentView.needsLayout = true + newContentView.layoutSubtreeIfNeeded() + newContentView.needsDisplay = true + displayIfNeeded() + + if shouldRestoreFirstResponder { + window?.makeFirstResponder(newContentView) + } + } + + override func layout() { + super.layout() + hostedContentView?.frame = bounds + } + + private func configure(contentView: NSView) { + contentView.wantsLayer = true + contentView.autoresizingMask = [.width, .height] + contentView.layer?.isOpaque = true + contentView.layer?.drawsAsynchronously = true + } + + private func detachHostedContentView() { + guard let hostedContentView else { + return + } + + if hostedContentView.superview === self { + hostedContentView.removeFromSuperview() + } + + self.hostedContentView = nil + } + + private func releaseHostedContentView(_ contentView: NSView) { + guard hostedContentView === contentView else { + return + } + + detachHostedContentView() + refreshHostingLayout() + } + + private func refreshHostingLayout() { + needsLayout = true + layoutSubtreeIfNeeded() + displayIfNeeded() + } + + private func shouldTransferFirstResponder(from previousContentView: NSView?) -> Bool { + guard let previousContentView, + let firstResponder = window?.firstResponder as? NSView + else { + return false + } + + return firstResponder === previousContentView || firstResponder.isDescendant(of: previousContentView) + } +} + struct BrowserPageView: NSViewRepresentable { let page: BrowserPage @EnvironmentObject var tabManager: TabManager @@ -9,49 +124,44 @@ struct BrowserPageView: NSViewRepresentable { func makeCoordinator() -> Coordinator { Coordinator( - page: page, tabManager: tabManager, historyManager: historyManager, privacyMode: privacyMode ) } - func makeNSView(context: Context) -> NSView { - let wrapperView = NSView() - wrapperView.wantsLayer = true - wrapperView.autoresizesSubviews = true - + func makeNSView(context: Context) -> BrowserPageHostView { + let hostView = BrowserPageHostView(frame: .zero) let contentView = page.contentView - contentView.autoresizingMask = [.width, .height] - contentView.layer?.isOpaque = true - contentView.layer?.drawsAsynchronously = true - context.coordinator.setupMouseEventMonitoring(for: contentView) - wrapperView.addSubview(contentView) - contentView.frame = wrapperView.bounds - - return wrapperView + context.coordinator.update(page: page, contentView: contentView) + hostView.host(contentView: contentView) + return hostView } - func updateNSView(_ nsView: NSView, context: Context) {} + func updateNSView(_ nsView: BrowserPageHostView, context: Context) { + let contentView = page.contentView + context.coordinator.update(page: page, contentView: contentView) + nsView.host(contentView: contentView) + } final class Coordinator: NSObject { - private let page: BrowserPage weak var tabManager: TabManager? weak var historyManager: HistoryManager? weak var privacyMode: PrivacyMode? + private weak var page: BrowserPage? private var mouseEventMonitor: Any? private weak var contentView: NSView? init( - page: BrowserPage, tabManager: TabManager?, historyManager: HistoryManager?, privacyMode: PrivacyMode ) { - self.page = page self.tabManager = tabManager self.historyManager = historyManager self.privacyMode = privacyMode + super.init() + startMouseEventMonitoring() } deinit { @@ -60,11 +170,15 @@ struct BrowserPageView: NSViewRepresentable { } } - func setupMouseEventMonitoring(for contentView: NSView) { + func update(page: BrowserPage, contentView: NSView) { + self.page = page self.contentView = contentView + } + private func startMouseEventMonitoring() { mouseEventMonitor = NSEvent.addLocalMonitorForEvents(matching: [.otherMouseDown]) { [weak self] event in guard let self, + let page = self.page, let contentView = self.contentView, self.isEventInContentView(event, contentView: contentView) else { @@ -73,19 +187,19 @@ struct BrowserPageView: NSViewRepresentable { switch event.buttonNumber { case 2: - self.handleMiddleClick(at: event.locationInWindow, contentView: contentView) + self.handleMiddleClick(at: event.locationInWindow, contentView: contentView, page: page) return nil case 3: - if self.page.canGoBack { + if page.canGoBack { DispatchQueue.main.async { - self.page.goBack() + page.goBack() } } return nil case 4: - if self.page.canGoForward { + if page.canGoForward { DispatchQueue.main.async { - self.page.goForward() + page.goForward() } } return nil @@ -95,7 +209,7 @@ struct BrowserPageView: NSViewRepresentable { } } - private func handleMiddleClick(at location: NSPoint, contentView: NSView) { + private func handleMiddleClick(at location: NSPoint, contentView: NSView, page: BrowserPage) { let locationInContentView = contentView.convert(location, from: nil) guard locationInContentView.x.isFinite, locationInContentView.y.isFinite, @@ -115,8 +229,9 @@ struct BrowserPageView: NSViewRepresentable { })(); """ - page.evaluateJavaScript(script) { [weak self] result, error in + page.evaluateJavaScript(script) { [weak self, weak page] result, error in guard error == nil, + page != nil, let urlString = result as? String, let url = URL(string: urlString), let tabManager = self?.tabManager, diff --git a/ora/Features/Browser/Views/BrowserSplitView.swift b/ora/Features/Browser/Views/BrowserSplitView.swift index 3d7352ba..003c6f12 100644 --- a/ora/Features/Browser/Views/BrowserSplitView.swift +++ b/ora/Features/Browser/Views/BrowserSplitView.swift @@ -76,20 +76,14 @@ struct BrowserSplitView: View { } private func contentView() -> some View { - ZStack { - if tabManager.activeTab == nil { + Group { + if let activeTab = tabManager.activeTab { BrowserContentContainer { - HomeView() + BrowserWebContentView(tab: activeTab) } - } - let activeId = tabManager.activeTab?.id - ForEach(tabManager.tabsToRender) { tab in - if tab.isWebViewReady { - BrowserContentContainer { - BrowserWebContentView(tab: tab) - } - .opacity(tab.id == activeId ? 1 : 0) - .allowsHitTesting(tab.id == activeId) + } else { + BrowserContentContainer { + HomeView() } } } diff --git a/ora/Features/Tabs/State/TabManager.swift b/ora/Features/Tabs/State/TabManager.swift index 4b07707f..7bb9c4f2 100644 --- a/ora/Features/Tabs/State/TabManager.swift +++ b/ora/Features/Tabs/State/TabManager.swift @@ -52,13 +52,6 @@ class TabManager: ObservableObject { ) } - var tabsToRender: [Tab] { - guard let container = activeContainer else { return [] } - let specialTabs = container.tabs.filter { $0.type == .pinned || $0.type == .fav || $0.isPlayingMedia } - let combined = Set(recentTabs + specialTabs) - return Array(combined) - } - /// Note: Could be made injectable via init parameter if preferred let tabSearchingService: TabSearchingProviding diff --git a/ora/Features/Tabs/Switcher/FloatingTabSwitcher.swift b/ora/Features/Tabs/Switcher/FloatingTabSwitcher.swift index 3106d2dc..1f0b2820 100644 --- a/ora/Features/Tabs/Switcher/FloatingTabSwitcher.swift +++ b/ora/Features/Tabs/Switcher/FloatingTabSwitcher.swift @@ -99,12 +99,12 @@ struct FloatingTabSwitcher: View { } } } - .padding(.horizontal, 28) - .padding(.vertical, 20) + .padding(.horizontal, 20) + .padding(.vertical, 18) .background(BlurEffectView(material: .popover, blendingMode: .withinWindow)) .background(theme.background.opacity(0.3)) .cornerRadius(Constants.containerCornerRadius) - .shadow(color: .blue.opacity(0.07), radius: 16, x: 0, y: 12) + .shadow(color: theme.primary.opacity(0.07), radius: 16, x: 0, y: 12) .background(keyboardHandler) .overlay(containerBorder) } diff --git a/oraTests/BrowserPageHostViewTests.swift b/oraTests/BrowserPageHostViewTests.swift new file mode 100644 index 00000000..6b7bc433 --- /dev/null +++ b/oraTests/BrowserPageHostViewTests.swift @@ -0,0 +1,116 @@ +import AppKit +@testable import Ora +import Testing + +@MainActor +struct BrowserPageHostViewTests { + @Test func attachingFirstContentViewAddsSubview() { + let host = makeHost() + let contentView = TrackingContentView() + + host.host(contentView: contentView) + + #expect(host.subviews.count == 1) + #expect(host.subviews.first === contentView) + #expect(host.hostedContentView === contentView) + #expect(contentView.frame == host.bounds) + } + + @Test func switchingContentViewsDetachesOldViewAndAttachesNewView() { + let host = makeHost() + let firstContentView = TrackingContentView() + let secondContentView = TrackingContentView() + + host.host(contentView: firstContentView) + host.host(contentView: secondContentView) + + #expect(host.subviews.count == 1) + #expect(host.subviews.first === secondContentView) + #expect(host.hostedContentView === secondContentView) + #expect(firstContentView.superview == nil) + #expect(secondContentView.superview === host) + } + + @Test func updatingWithSameContentViewIsANoOp() { + let host = makeHost() + let contentView = TrackingContentView() + + host.host(contentView: contentView) + let transitionCountAfterFirstAttach = contentView.superviewTransitions + + host.host(contentView: contentView) + + #expect(host.subviews.count == 1) + #expect(host.subviews.first === contentView) + #expect(contentView.superviewTransitions == transitionCountAfterFirstAttach) + } + + @Test func clearingContentViewRemovesHostedSubview() { + let host = makeHost() + let contentView = TrackingContentView() + + host.host(contentView: contentView) + host.host(contentView: nil) + + #expect(host.subviews.isEmpty) + #expect(host.hostedContentView == nil) + #expect(contentView.superview == nil) + } + + @Test func attachingContentViewFromAnotherHostReparentsItCleanly() { + let firstHost = makeHost() + let secondHost = makeHost() + let contentView = TrackingContentView() + + firstHost.host(contentView: contentView) + secondHost.host(contentView: contentView) + + #expect(firstHost.subviews.isEmpty) + #expect(firstHost.hostedContentView == nil) + #expect(secondHost.subviews.count == 1) + #expect(secondHost.subviews.first === contentView) + #expect(secondHost.hostedContentView === contentView) + #expect(contentView.superview === secondHost) + #expect(contentView.removeFromSuperviewCalls == 1) + } + + @Test func switchingToAStaleSubviewAlreadyAttachedToTheSameHostDoesNotReparentIt() { + let host = makeHost() + let firstContentView = TrackingContentView() + let staleContentView = TrackingContentView() + + host.host(contentView: firstContentView) + host.addSubview(staleContentView) + + let staleSuperviewTransitions = staleContentView.superviewTransitions + + host.host(contentView: staleContentView) + + #expect(host.subviews.count == 1) + #expect(host.subviews.first === staleContentView) + #expect(host.hostedContentView === staleContentView) + #expect(firstContentView.superview == nil) + #expect(staleContentView.superview === host) + #expect(staleContentView.superviewTransitions == staleSuperviewTransitions) + #expect(staleContentView.removeFromSuperviewCalls == 0) + } + + private func makeHost() -> BrowserPageHostView { + BrowserPageHostView(frame: NSRect(x: 0, y: 0, width: 800, height: 600)) + } +} + +private final class TrackingContentView: NSView { + var superviewTransitions = 0 + var removeFromSuperviewCalls = 0 + + override func removeFromSuperview() { + removeFromSuperviewCalls += 1 + super.removeFromSuperview() + } + + override func viewWillMove(toSuperview newSuperview: NSView?) { + superviewTransitions += 1 + super.viewWillMove(toSuperview: newSuperview) + } +} diff --git a/oraTests/oraTests.swift b/oraTests/oraTests.swift index fe26c9aa..68d5dbb1 100644 --- a/oraTests/oraTests.swift +++ b/oraTests/oraTests.swift @@ -5,7 +5,8 @@ // Created by keni on 6/21/25. // -@testable import ora +import Foundation +@testable import Ora import Testing struct OraTests { diff --git a/project.yml b/project.yml index 17a5e42a..3fc70686 100644 --- a/project.yml +++ b/project.yml @@ -132,6 +132,19 @@ targets: CODE_SIGN_ENTITLEMENTS: ora/Info/ora.entitlements ENABLE_HARDENED_RUNTIME: YES + oraTests: + type: bundle.unit-test + platform: "macOS" + deploymentTarget: "15.0" + sources: + - path: oraTests + dependencies: + - target: ora + settings: + base: + TEST_HOST: "$(BUILT_PRODUCTS_DIR)/Ora.app/Contents/MacOS/Ora" + BUNDLE_LOADER: "$(TEST_HOST)" + schemes: ora: shared: true @@ -142,6 +155,8 @@ schemes: config: Debug test: config: Debug + targets: + - oraTests profile: config: Release analyze: