Subject: bug#62234: Toolbar does not work properly (with Emacs 29 or 30)

From: Daniel Martín <[email-masked]>
Date: Sat, 18 Mar 2023 02:09:04 +0100

Po Lu <[email-masked]> writes:

>> I can certainly reproduce an issue with the toolbar in the NS port.  I
>> think it's the same issue that the OP is describing.  Here's some steps
>> to reproduce it:
>>
>> emacs -Q
>>
>> The Save and the Cut(Kill) icons are enabled.
>>
>> Resize the Emacs frame a little bit.
>>
>> The Save and the Cut(Kill) icons are now disabled, as expected, because
>> the *scratch* buffer is not backed by a file, and there's no active
>> region.
>
> If it helps, I don't see this at all on GNUstep.  Both buttons are
> initially disabled.

I've browsed some old Apple documentation and I think the way the
toolbar validation is implemented in the NS port may not be totally
correct.
https://web.archive.org/web/20090405012930if_/http://developer.apple.com/documentation/Cocoa/Conceptual/Toolbars/Tasks/ValidatingTBItems.html#//apple_ref/doc/uid/20000753-1036447
states that "If an image item has a valid target/action pair, then the
toolbar will call NSToolbarItemValidation’s validateToolbarItem: on
target if the target implements it; otherwise the item is enabled by
default.".

So I've implemented the above idea in the attached patch and it fixes
the issue for me.  I've also removed the validateVisibleItems override,
which should not be needed (and it's probably incorrect, as Apple's
documentation says that if you override this method, you must call
super).

Could you check if the patch also works fine on GNUStep?  If not,
perhaps we could make the change conditional to Apple systems.  Thanks.

>From eabab73b0dca6cd7aff67427b4443fd08f3b9bcc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <[email-masked]>
Date: Sat, 18 Mar 2023 01:47:32 +0100
Subject: [PATCH] Fix toolbar item validation in the NS port

* src/nsmenu.m ([EmacsToolbar validateVisibleItems]): Remove this
override, which is not really needed.  Also, it doesn't do the right
thing, because Apple's documentation states that this method, if
overriden, must call super.
* src/nsterm.m ([EmacsView validateToolbarItem:]): Implement this
NSToolbarItemValidation method instead, which is called by AppKit when
it needs to validate an image item.
---
 src/nsmenu.m | 8 --------
 src/nsterm.m | 5 +++++
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/nsmenu.m b/src/nsmenu.m
index 2c1f575bdf2..f7a401dc407 100644
--- a/src/nsmenu.m
+++ b/src/nsmenu.m
@@ -1346,14 +1346,6 @@ - (void) addDisplayItemWithImage: (EmacsImage *)img
   enablement = (enablement << 1) | (enabled == YES);
 }
 
-/* This overrides super's implementation, which automatically sets
-   all items to enabled state (for some reason).  */
-- (void)validateVisibleItems
-{
-  NSTRACE ("[EmacsToolbar validateVisibleItems]");
-}
-
-
 /* delegate methods */
 
 - (NSToolbarItem *)toolbar: (NSToolbar *)toolbar
diff --git a/src/nsterm.m b/src/nsterm.m
index 0e75cbf3f0f..8bb22c0f756 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -8573,6 +8573,11 @@ - (instancetype)toolbarClicked: (id)item
   return self;
 }
 
+-(BOOL)validateToolbarItem:(NSToolbarItem *)toolbarItem
+{
+    return [toolbarItem isEnabled];
+}
+
 
 - (instancetype)toggleToolbar: (id)sender
 {
-- 
2.34.1