From f384fd038b642cae3b4f537de894c39c0539c59a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 1 Dec 2024 11:31:55 -0800 Subject: [PATCH] macos: trigger fullscreenDidChange on any fullscreen event Fixes #2840 Related to #2842 This builds on #2842 by missing a key situation: when native fullscreen is toggled using the menu bar items it doesn't go through our `FullscreenStyle` machinery so we don't trigger fullscreen change events. This commit makes it so that our FullscreenStyle always listens for native fullscreen change (even in non-native modes) to fire a fullscreen did change event. This way we can always rely on the event to be fired when fullscreen changes no matter what. --- .../QuickTerminalController.swift | 1 + .../Terminal/BaseTerminalController.swift | 16 ++++- .../Terminal/TerminalController.swift | 1 + macos/Sources/Helpers/Fullscreen.swift | 72 +++++++++++++------ 4 files changed, 69 insertions(+), 21 deletions(-) diff --git a/macos/Sources/Features/QuickTerminal/QuickTerminalController.swift b/macos/Sources/Features/QuickTerminal/QuickTerminalController.swift index 18549eea1..b5e65d76e 100644 --- a/macos/Sources/Features/QuickTerminal/QuickTerminalController.swift +++ b/macos/Sources/Features/QuickTerminal/QuickTerminalController.swift @@ -57,6 +57,7 @@ class QuickTerminalController: BaseTerminalController { // MARK: NSWindowController override func windowDidLoad() { + super.windowDidLoad() guard let window = self.window else { return } // The controller is the window delegate so we can detect events such as diff --git a/macos/Sources/Features/Terminal/BaseTerminalController.swift b/macos/Sources/Features/Terminal/BaseTerminalController.swift index 721248013..68c243004 100644 --- a/macos/Sources/Features/Terminal/BaseTerminalController.swift +++ b/macos/Sources/Features/Terminal/BaseTerminalController.swift @@ -404,7 +404,21 @@ class BaseTerminalController: NSWindowController, } } - //MARK: - NSWindowDelegate + // MARK: NSWindowController + + override func windowDidLoad() { + guard let window else { return } + + // We always initialize our fullscreen style to native if we can because + // initialization sets up some state (i.e. observers). If its set already + // somehow we don't do this. + if fullscreenStyle == nil { + fullscreenStyle = NativeFullscreen(window) + fullscreenStyle?.delegate = self + } + } + + // MARK: NSWindowDelegate // This is called when performClose is called on a window (NOT when close() // is called directly). performClose is called primarily when UI elements such diff --git a/macos/Sources/Features/Terminal/TerminalController.swift b/macos/Sources/Features/Terminal/TerminalController.swift index 698551f3e..67e7259f3 100644 --- a/macos/Sources/Features/Terminal/TerminalController.swift +++ b/macos/Sources/Features/Terminal/TerminalController.swift @@ -275,6 +275,7 @@ class TerminalController: BaseTerminalController { } override func windowDidLoad() { + super.windowDidLoad() guard let window = window as? TerminalWindow else { return } // I copy this because we may change the source in the future but also because diff --git a/macos/Sources/Helpers/Fullscreen.swift b/macos/Sources/Helpers/Fullscreen.swift index f5df43ec2..bb3859e07 100644 --- a/macos/Sources/Helpers/Fullscreen.swift +++ b/macos/Sources/Helpers/Fullscreen.swift @@ -45,20 +45,53 @@ extension FullscreenDelegate { func fullscreenDidChange() {} } +/// The base class for fullscreen implementations, cannot be used as a FullscreenStyle on its own. +class FullscreenBase { + let window: NSWindow + weak var delegate: FullscreenDelegate? + + required init?(_ window: NSWindow) { + self.window = window + + // We want to trigger delegate methods on window native fullscreen + // changes (didEnterFullScreenNotification, etc.) no matter what our + // fullscreen style is. + let center = NotificationCenter.default + center.addObserver( + self, + selector: #selector(didEnterFullScreenNotification), + name: NSWindow.didEnterFullScreenNotification, + object: window) + center.addObserver( + self, + selector: #selector(didExitFullScreenNotification), + name: NSWindow.didExitFullScreenNotification, + object: window) + } + + deinit { + NotificationCenter.default.removeObserver(self) + } + + @objc private func didEnterFullScreenNotification(_ notification: Notification) { + delegate?.fullscreenDidChange() + } + + @objc private func didExitFullScreenNotification(_ notification: Notification) { + delegate?.fullscreenDidChange() + } +} + /// macOS native fullscreen. This is the typical behavior you get by pressing the green fullscreen /// button on regular titlebars. -class NativeFullscreen: FullscreenStyle { - private let window: NSWindow - - weak var delegate: FullscreenDelegate? +class NativeFullscreen: FullscreenBase, FullscreenStyle { var isFullscreen: Bool { window.styleMask.contains(.fullScreen) } var supportsTabs: Bool { true } required init?(_ window: NSWindow) { // TODO: There are many requirements for native fullscreen we should // check here such as the stylemask. - - self.window = window + super.init(window) } func enter() { @@ -72,8 +105,9 @@ class NativeFullscreen: FullscreenStyle { // Enter fullscreen window.toggleFullScreen(self) - // Notify the delegate - delegate?.fullscreenDidChange() + // Note: we don't call our delegate here because the base class + // will always trigger the delegate on native fullscreen notifications + // and we don't want to double notify. } func exit() { @@ -84,14 +118,13 @@ class NativeFullscreen: FullscreenStyle { window.toggleFullScreen(nil) - // Notify the delegate - delegate?.fullscreenDidChange() + // Note: we don't call our delegate here because the base class + // will always trigger the delegate on native fullscreen notifications + // and we don't want to double notify. } } -class NonNativeFullscreen: FullscreenStyle { - weak var delegate: FullscreenDelegate? - +class NonNativeFullscreen: FullscreenBase, FullscreenStyle { // Non-native fullscreen never supports tabs because tabs require // the "titled" style and we don't have it for non-native fullscreen. var supportsTabs: Bool { false } @@ -110,13 +143,8 @@ class NonNativeFullscreen: FullscreenStyle { var hideMenu: Bool = true } - private let window: NSWindow private var savedState: SavedState? - required init?(_ window: NSWindow) { - self.window = window - } - func enter() { // If we are in fullscreen we don't do it again. guard !isFullscreen else { return } @@ -187,8 +215,12 @@ class NonNativeFullscreen: FullscreenStyle { guard isFullscreen else { return } guard let savedState else { return } - // Remove all our notifications - NotificationCenter.default.removeObserver(self) + // Remove all our notifications. We remove them one by one because + // we don't want to remove the observers that our superclass sets. + let center = NotificationCenter.default + center.removeObserver(self, name: NSWindow.didBecomeMainNotification, object: window) + center.removeObserver(self, name: NSWindow.didResignMainNotification, object: window) + center.removeObserver(self, name: NSWindow.didChangeScreenNotification, object: window) // Unhide our elements if savedState.dock {