Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using macos_traditional_fullscreen yes fails to hide #4472

Closed
npearson72 opened this issue Jan 7, 2022 · 18 comments
Closed

Using macos_traditional_fullscreen yes fails to hide #4472

npearson72 opened this issue Jan 7, 2022 · 18 comments
Labels

Comments

@npearson72
Copy link
Contributor

npearson72 commented Jan 7, 2022

Describe the bug
Using MacOS version 10.15.7 (19H1615) (Oddly, it works fine on MacOS version 12.1 (21C52))

Make sure no other windows are open except for kitty.

  1. Add the following to kitty config: macos_traditional_fullscreen yes

  2. Enter full screen mode.

  3. Try hiding kitty by either using the menu or hitting H

Expected result is that kitty will be hidden.
Actual result... it appears to go into the background behind other windows (if other windows are open), but it does not get hidden.

Environment details

kitty 0.24.1 created by Kovid Goyal
Darwin nathan.local 19.6.0 Darwin Kernel Version 19.6.0: Sun Nov 14 19:58:51 PST 2021; root:xnu-6153.141.50~1/RELEASE_X86_64 x86_64
ProductName:	Mac OS X ProductVersion:	10.15.7 BuildVersion:	19H1615
Frozen: True
Paths:
  kitty: /Applications/kitty.app/Contents/MacOS/kitty
  base dir: /Applications/kitty.app/Contents/Resources/kitty
  extensions dir: /Applications/kitty.app/Contents/Resources/Python/lib/kitty-extensions
  system shell: /usr/local/bin/zsh
Loaded config files:
  /Users/nathan/.config/kitty/kitty.conf

Config options different from defaults:
adjust_column_width          -1
adjust_line_height           3
bold_font                    Hack Nerd Font Mono Bold
bold_italic_font             Hack Nerd Font Mono Bold Italic
enable_audio_bell            False
font_family                  Hack Nerd Font Mono Regular
font_size                    14.0
italic_font                  Hack Nerd Font Mono Italic
macos_thicken_font           1.0
macos_traditional_fullscreen True
shell_integration            frozenset({'no-cursor'})
visual_bell_duration         0.3
Added shortcuts:
	alt+/ send_text all \x1b\x2f
	alt+: send_text all \x1b\x3a
	alt+[ send_text all \x1b\x5b
	alt+] send_text all \x1b\x5d
	alt+_ send_text all \x1b\x5f
	alt+h send_text all \x1b\x68
	alt+j send_text all \x1b\x6a
	alt+k send_text all \x1b\x6b
	alt+l send_text all \x1b\x6c
	alt+o send_text all \x1b\x6f
	alt+r send_text all \x1b\x72
	alt+s send_text all \x1b\x73
	alt+t send_text all \x1b\x74
	alt+w send_text all \x1b\x77
	alt+| send_text all \x1b\x7c
	shift+alt+r send_text all \x1b\x52
	cmd+k send_text all \x1C
Changed shortcuts:
	cmd+enter toggle_fullscreen
Colors:
	background                   #212121   
	color0                       #212121   
	color1                       #c30771   
	color10                      #5fd7a7   
	color11                      #f3e430   
	color12                      #b6d6fd   
	color13                      #6855de   
	color14                      #4fb8cc   
	color2                       #10a778   
	color3                       #a89c14   
	color4                       #008ec4   
	color5                       #523c79   
	color6                       #20a5ba   
	color7                       #f1f1f1   
	color8                       #424242   
	color9                       #e32791   
	cursor                       #1ac7fc   
	foreground                   #f1f1f1   
@npearson72 npearson72 added the bug label Jan 7, 2022
@npearson72 npearson72 changed the title Using macos_traditional_fullscreen yes fails to hide when (⌘H) Using macos_traditional_fullscreen yes fails to hide <kbd>⌘</kbd><kbd>H</kbd> Jan 9, 2022
@npearson72 npearson72 changed the title Using macos_traditional_fullscreen yes fails to hide <kbd>⌘</kbd><kbd>H</kbd> Using macos_traditional_fullscreen yes fails to hide Jan 9, 2022
@kovidgoyal
Copy link
Owner

doesnt replicate for me with latest kitty on latest macOS. steps I tried

  1. kitty -o macos_traditional_fullscreen=y
  2. press ctrl+shift+f11 to enter full screen
  3. press command-h
  4. kitty window disappears and I can see the other windows on my desktop

@page-down
Copy link
Contributor

I recently tried on a macOS 10.15 device and I can reproduce the issue.
I cannot reproduce it on macOS 12.1.

When cmd+h is pressed, the full-screen window goes behind the other windows and before the desktop background.

@npearson72
Copy link
Contributor Author

npearson72 commented Jan 10, 2022

@kovidgoyal have you tried reproducing on Mac OS 10.15?

I retried the steps you've outlined and am able to reproduce.

I know 10.15 is a bit dated, but some of us are bound to what our IT deps force on us 😞

@kovidgoyal
Copy link
Owner

I'm afraid I dont have a 10.15 installation and given the implementation
of hide() comes from apple, it seems pretty clear they had a bug in it
they have since fixed. So you will need to either update macOS or live
with the bug.

@npearson72
Copy link
Contributor Author

@kovidgoyal hide works for all other apps.

@kovidgoyal
Copy link
Owner

None of the other apps use "traditional full screen" no.

@npearson72
Copy link
Contributor Author

Seems the problem can be reproduced with modern full screen. It's your project, so obviously your call. But it's possible a closer look might reveal a bug worth fixing.

@kovidgoyal
Copy link
Owner

Like I said I dont have Catalina, and this issue is not replicable on
modern macOS. You are most welcome to submit a PR working around
whatever the bug in Catalina is.

@npearson72
Copy link
Contributor Author

I don't have the tool chain or familiarity with the subject matter to offer a PR, else I would.

@page-down
Copy link
Contributor

Are there any possible issues with using the new (>= 11.0) traditional full screen mode in macOS 10.15?

Does the reference to Partial fix in this git commit mean that there are minor unresolved issues?

bdcac9a

It looks like Apple doesn't want NSWindowStyleMaskFullScreen to be used with Hide Window.

I tested it in macOS 10.15 using the new mode (full screen size window without title bar) which seems to work and also hides the window.

Here are the compilation errors I recorded when I ran python3 setup.py. If you compile it yourself, you need to add compilation parameters to bypass it.

// macOS 10.15
kitty/cocoa_window.m:374:64: error: unused parameter 'center' [-Werror,-Wunused-parameter]
    - (void)userNotificationCenter:(UNUserNotificationCenter *)center
                                                               ^
kitty/cocoa_window.m:375:55: error: unused parameter 'notification' [-Werror,-Wunused-parameter]
            willPresentNotification:(UNNotification *)notification
                                                      ^
2 errors generated.

I don't currently have a macOS 10.15 device at hand, so I can't test and submit a PR. I can only provide the basic information that I know.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Jan 11, 2022 via email

@page-down
Copy link
Contributor

page-down commented Jan 11, 2022

If someone can try to test it, I think the following changes can be made.

glfw/cocoa_window.m

// _glfwPlatformIsFullscreen()
// _glfwPlatformToggleFullscreen()
if (traditional) {  if (@available(macOS 11.0, *))

11.0 -> 10.15 <.minor-ver?-i-dont-known>

That's about as much as I can do.

@npearson72
Copy link
Contributor Author

@page-down I can test, but not sure what change needs to be made.

I see the following functions:

bool _glfwPlatformIsFullscreen(_GLFWwindow* w, unsigned int flags) {
    NSWindow *window = w->ns.object;
    bool traditional = !(flags & 1);
    if (traditional) { if(@available(macOS 11.0, *)) return w->ns.in_traditional_fullscreen; }
    NSWindowStyleMask sm = [window styleMask];
    return sm & NSWindowStyleMaskFullScreen;
}

bool _glfwPlatformToggleFullscreen(_GLFWwindow* w, unsigned int flags) {
    NSWindow *window = w->ns.object;
    bool made_fullscreen = true;
    bool traditional = !(flags & 1);
    NSWindowStyleMask sm = [window styleMask];
    if (traditional) {
        if (@available(macOS 11.0, *)) {
            // As of Big Turd NSWindowStyleMaskFullScreen is no longer useable
            if (!w->ns.in_traditional_fullscreen) {
                w->ns.pre_full_screen_style_mask = sm;
                [window setStyleMask: NSWindowStyleMaskBorderless];
                [[NSApplication sharedApplication] setPresentationOptions: NSApplicationPresentationAutoHideMenuBar | NSApplicationPresentationAutoHideDock];
                [window setFrame:[window.screen frame] display:YES];
                w->ns.in_traditional_fullscreen = true;
            } else {
                made_fullscreen = false;
                [window setStyleMask: w->ns.pre_full_screen_style_mask];
                [[NSApplication sharedApplication] setPresentationOptions: NSApplicationPresentationDefault];
                w->ns.in_traditional_fullscreen = false;
            }
        } else {
            bool in_fullscreen = sm & NSWindowStyleMaskFullScreen;
            if (!(in_fullscreen)) {
                sm |= NSWindowStyleMaskBorderless | NSWindowStyleMaskFullScreen;
                [[NSApplication sharedApplication] setPresentationOptions: NSApplicationPresentationAutoHideMenuBar | NSApplicationPresentationAutoHideDock];
            } else {
                made_fullscreen = false;
                sm &= ~(NSWindowStyleMaskBorderless | NSWindowStyleMaskFullScreen);
                [[NSApplication sharedApplication] setPresentationOptions: NSApplicationPresentationDefault];
            }
            [window setStyleMask: sm];
        }
        // Changing the style mask causes the first responder to be cleared
        [window makeFirstResponder:w->ns.view];
        // If the dock and menubar are hidden going from maximized to fullscreen doesnt change the window size
        // and macOS forgets to trigger windowDidResize, so call it ourselves
        NSNotification *notification = [NSNotification notificationWithName:NSWindowDidResizeNotification object:window];
        [w->ns.delegate performSelector:@selector(windowDidResize:) withObject:notification afterDelay:0];
    } else {
        bool in_fullscreen = sm & NSWindowStyleMaskFullScreen;
        if (in_fullscreen) made_fullscreen = false;
        [window toggleFullScreen: nil];
    }
    return made_fullscreen;
}

What would I need to change in them?

@page-down
Copy link
Contributor

What would I need to change in them?

diff --git a/glfw/cocoa_window.m b/glfw/cocoa_window.m
index 2fff85c8..b2d76af2 100644
--- a/glfw/cocoa_window.m
+++ b/glfw/cocoa_window.m
@@ -2302,7 +2302,7 @@ void _glfwPlatformSetCursor(_GLFWwindow* window, _GLFWcursor* cursor UNUSED)
 bool _glfwPlatformIsFullscreen(_GLFWwindow* w, unsigned int flags) {
     NSWindow *window = w->ns.object;
     bool traditional = !(flags & 1);
-    if (traditional) { if(@available(macOS 11.0, *)) return w->ns.in_traditional_fullscreen; }
+    if (traditional) { if(@available(macOS 10.15, *)) return w->ns.in_traditional_fullscreen; }
     NSWindowStyleMask sm = [window styleMask];
     return sm & NSWindowStyleMaskFullScreen;
 }
@@ -2313,8 +2313,9 @@ bool _glfwPlatformToggleFullscreen(_GLFWwindow* w, unsigned int flags) {
     bool traditional = !(flags & 1);
     NSWindowStyleMask sm = [window styleMask];
     if (traditional) {
-        if (@available(macOS 11.0, *)) {
+        if (@available(macOS 10.15, *)) {
             // As of Big Turd NSWindowStyleMaskFullScreen is no longer useable
+            // Also no longer compatible after a minor release of macOS 10.15
             if (!w->ns.in_traditional_fullscreen) {
                 w->ns.pre_full_screen_style_mask = sm;
                 [window setStyleMask: NSWindowStyleMaskBorderless];

Try the above patch to see if it works.

@npearson72
Copy link
Contributor Author

@page-down Can confirm, this works!

Only issue now is... hide does not work at all when macos_traditional_fullscreen no

Even the menu button is disabled.

Screen Shot 2022-01-11 at 5 46 02 AM

@npearson72
Copy link
Contributor Author

Also, not sure it's helpful, but I have the Catalina install app. I can provide a dropbox link to share it with you if you guys want?

@page-down
Copy link
Contributor

Only issue now is... hide does not work at all when macos_traditional_fullscreen no

It works as expected.

The modern macOS full-screen mode takes up the entire Space. The window cannot be hidden. The same is true when you go full screen with any other app.

@npearson72
Copy link
Contributor Author

npearson72 commented Jan 11, 2022

It works as expected.

Ah, you're right. Hadn't noticed that before.

Any chance this change can be added to a future release?

I've pushed up a PR: #4496

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants