GNU bug report logs - #62234
Toolbar does not work properly (with Emacs 29 or 30)

Previous Next

Full log

Message #35 received at 62234 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Daniel Martín <mardani29 <at> yahoo.es>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 62234 <at> debbugs.gnu.org,
 Konrad Podczeck <konrad.podczeck <at> univie.ac.at>
Subject: Re: bug#62234: Toolbar does not work properly (with Emacs 29 or 30)
Date: Sat, 18 Mar 2023 11:11:26 +0800

Daniel Martín <mardani29 <at> yahoo.es> writes:

> 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?= <mardani29 <at> yahoo.es>
> 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]");
> -}
> -

I think this needs to be kept on GNUstep.

>  /* 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];
> +}
> +

This does no harm on GNUstep.  However, please indent with two spaces,
and place spaces after -, :, and ):

- (BOOL) validateToolbarItem: (NSToolBarItem *) toolbarItem
{
  return [toolbarItem isEnabled];
}

Otherwise, LGTM.  Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.