bug#50534: 28.0.50; Toolbar shows despite tool-bar-mode being disabled on macOS

[Alan Third <alan@idiocy.org>]

[-- Attachment #1: Type: text/plain, Size: 1492 bytes --]

On Fri, Sep 17, 2021 at 06:26:54PM +0100, Alan Third wrote:
> On Tue, Sep 14, 2021 at 03:35:57PM +0100, Alan Third wrote:
> > On Tue, Sep 14, 2021 at 04:17:15PM +0200, Kirill A. Korinsky wrote:
> > > Hey,
> > > 
> > > I can't broke the nuclear option anymore.
> > > 
> > > Seems it fixing everything.
> > 
> > Thanks, I'll push it to master.
> 
> This last patch breaks toolbar display on my Mac.
> 
> I think probably the best option is to change the toolbar handling so
> it actually just removes the toolbar when we don't want to display it.
> I really don't understand why the NS port goes to great lengths to
> hide it. If we remove it then it can never be displayed when it
> shouldn't be, and if it's attached to the window then it should always
> be displayed, so we don't have to concern ourselves with making sure
> the settings match after the window is resized or whatever.

Can you please try the attached patch and check that the toolbar is
behaving as expected.

It seems to work correctly here everywhere (except in GNUstep we
sometimes get an extra tall toolbar, but I doubt anyone cares too much
about that), but I think it may have removed the animation when the
toolbar is attached to the frame.

If anyone is really upset about the animation (and I'm not even sure
it IS gone) then we probably just need to create the toolbar as
invisible and attach it then mark it visible, but only do that on
Cocoa as it breaks the toolbar on frame creation in GNUstep.
-- 
Alan Third

[-- Attachment #2: 0001-Fix-NS-toolbar-again-bug-50534.patch --]
[-- Type: text/x-diff, Size: 6793 bytes --]

From 2cf79c5636eb6fd5c7d082db2b0fdef1f168f404 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sun, 26 Sep 2021 11:12:48 +0100
Subject: [PATCH] Fix NS toolbar again (bug#50534)

* src/nsmenu.m (free_frame_tool_bar): Remove toolbar.
(update_frame_tool_bar_1): New function.
(update_frame_tool_bar): Move most of the functionality to
update_frame_tool_bar_1.
* src/nsterm.h: Definitions of functions and methods.
* src/nsterm.m (ns_update_begin):
([EmacsView windowDidEnterFullScreen]):
([EmacsView windowDidExitFullScreen]): We no longer need to reset the
toolbar visibility as that's done when we create the new fullscreen
window.
([EmacsWindow initWithEmacsFrame:fullscreen:screen:]): Move the check
for undecorated frames into createToolbar:.
([EmacsWindow createToolbar:]): Check whether a toolbar should be
created, and run the toolbar update immediately.
---
 src/nsmenu.m | 35 +++++++++++++++++++++++------------
 src/nsterm.h |  5 +++++
 src/nsterm.m | 28 ++++++----------------------
 3 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/src/nsmenu.m b/src/nsmenu.m
index f0c5bb24e6..9b78643d56 100644
--- a/src/nsmenu.m
+++ b/src/nsmenu.m
@@ -995,25 +995,24 @@ - (void)menu:(NSMenu *)menu willHighlightItem:(NSMenuItem *)item
   /* Note: This triggers an animation, which calls windowDidResize
      repeatedly.  */
   f->output_data.ns->in_animation = 1;
-  [[[view window] toolbar] setVisible: NO];
+  [[[view window] toolbar] setVisible:NO];
   f->output_data.ns->in_animation = 0;
 
+  [[view window] setToolbar:nil];
+
   unblock_input ();
 }
 
 void
-update_frame_tool_bar (struct frame *f)
+update_frame_tool_bar_1 (struct frame *f, EmacsToolbar *toolbar)
 /* --------------------------------------------------------------------------
     Update toolbar contents.
    -------------------------------------------------------------------------- */
 {
   int i, k = 0;
-  NSWindow *window = [FRAME_NS_VIEW (f) window];
-  EmacsToolbar *toolbar = (EmacsToolbar *)[window toolbar];
 
   NSTRACE ("update_frame_tool_bar");
 
-  if (window == nil || toolbar == nil) return;
   block_input ();
 
 #ifdef NS_IMPL_COCOA
@@ -1094,13 +1093,6 @@ - (void)menu:(NSMenu *)menu willHighlightItem:(NSMenuItem *)item
 #undef TOOLPROP
     }
 
-  if (![toolbar isVisible] != !FRAME_EXTERNAL_TOOL_BAR (f))
-    {
-      f->output_data.ns->in_animation = 1;
-      [toolbar setVisible: FRAME_EXTERNAL_TOOL_BAR (f)];
-      f->output_data.ns->in_animation = 0;
-    }
-
 #ifdef NS_IMPL_COCOA
   if ([toolbar changed])
     {
@@ -1124,9 +1116,28 @@ - (void)menu:(NSMenu *)menu willHighlightItem:(NSMenuItem *)item
       [newDict release];
     }
 #endif
+
+  [toolbar setVisible:YES];
   unblock_input ();
 }
 
+void
+update_frame_tool_bar (struct frame *f)
+{
+  EmacsWindow *window = (EmacsWindow *)[FRAME_NS_VIEW (f) window];
+  EmacsToolbar *toolbar = (EmacsToolbar *)[window toolbar];
+
+  if (!toolbar)
+    {
+      [window createToolbar:f];
+      return;
+    }
+
+  if (window == nil || toolbar == nil) return;
+
+  update_frame_tool_bar_1 (f, toolbar);
+}
+
 
 /* ==========================================================================
 
diff --git a/src/nsterm.h b/src/nsterm.h
index 6d4ea77121..1bedf78bcb 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -418,6 +418,7 @@ #define NSTRACE_UNSILENCE()
 
 - (instancetype)initWithEmacsFrame:(struct frame *)f;
 - (instancetype)initWithEmacsFrame:(struct frame *)f fullscreen:(BOOL)fullscreen screen:(NSScreen *)screen;
+- (void)createToolbar:(struct frame *)f;
 - (void)setParentChildRelationships;
 - (NSInteger)borderWidth;
 - (BOOL)restackWindow:(NSWindow *)win above:(BOOL)above;
@@ -1148,6 +1149,10 @@ ns_query_color (void *col, Emacs_Color *color_def, bool setPixel);
 
 /* in nsmenu */
 extern void update_frame_tool_bar (struct frame *f);
+#ifdef __OBJC__
+extern void update_frame_tool_bar_1 (struct frame *f, EmacsToolbar *toolbar);
+#endif
+
 extern void free_frame_tool_bar (struct frame *f);
 extern Lisp_Object find_and_return_menu_selection (struct frame *f,
                                                    bool keymaps,
diff --git a/src/nsterm.m b/src/nsterm.m
index 4ef20e4c2b..3363fac475 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1021,15 +1021,6 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
 
   ns_update_auto_hide_menu_bar ();
 
-  NSToolbar *toolbar = [[FRAME_NS_VIEW (f) window] toolbar];
-  if (toolbar)
-  {
-    /* Ensure the toolbars visibility is set correctly.  */
-    BOOL tbar_visible = FRAME_EXTERNAL_TOOL_BAR (f) ? YES : NO;
-    if (! tbar_visible != ! [toolbar isVisible])
-      [toolbar setVisible: tbar_visible];
-  }
-
   ns_updating_frame = f;
   [view lockFocus];
 }
@@ -7401,7 +7392,6 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
     }
   else
     {
-      BOOL tbar_visible = FRAME_EXTERNAL_TOOL_BAR (emacsframe) ? YES : NO;
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070 \
   && MAC_OS_X_VERSION_MIN_REQUIRED <= 1070
       unsigned val = (unsigned)[NSApp presentationOptions];
@@ -7419,7 +7409,6 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
           [NSApp setPresentationOptions: options];
         }
 #endif
-      [[[self window]toolbar] setVisible:tbar_visible];
     }
 }
 
@@ -7460,14 +7449,6 @@ - (void)windowDidExitFullScreen /* provided for direct calls */
 #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
   [self updateCollectionBehavior];
 #endif
-  if (FRAME_EXTERNAL_TOOL_BAR (emacsframe))
-    {
-      [[[self window] toolbar] setVisible:YES];
-      update_frame_tool_bar (emacsframe);
-      [[self window] display];
-    }
-  else
-    [[[self window] toolbar] setVisible:NO];
 
   if (next_maximized != -1)
     [[self window] performZoom:self];
@@ -8298,8 +8279,7 @@ - (instancetype) initWithEmacsFrame:(struct frame *)f
         [self setOpaque:NO];
 
       /* toolbar support */
-      if (! FRAME_UNDECORATED (f))
-        [self createToolbar:f];
+      [self createToolbar:f];
 
       /* macOS Sierra automatically enables tabbed windows.  We can't
          allow this to be enabled until it's available on a Free system.
@@ -8316,13 +8296,17 @@ - (instancetype) initWithEmacsFrame:(struct frame *)f
 
 - (void)createToolbar: (struct frame *)f
 {
+  if (FRAME_UNDECORATED (f) || !FRAME_EXTERNAL_TOOL_BAR (f))
+    return;
+
   EmacsView *view = (EmacsView *)FRAME_NS_VIEW (f);
 
   EmacsToolbar *toolbar = [[EmacsToolbar alloc]
                             initForView:view
                             withIdentifier:[NSString stringWithLispString:f->name]];
-  [toolbar setVisible:NO];
+
   [self setToolbar:toolbar];
+  update_frame_tool_bar_1 (f, toolbar);
 
 #ifdef NS_IMPL_COCOA
   {
-- 
2.33.0