bug#39808: 28.0.50; toolbar broken

bug#39808: 28.0.50; toolbar broken

[Markus FFM]

emacs -Q
Toolbar: Discard (kill) current buffer 
Wrong type argument: symbolp, #<frame emacs@HOST 0x2444b50>
same with all other toolbar items


In GNU Emacs 28.0.50 (build 15, x86_64-pc-linux-gnu, GTK+ Version 3.24.13, cairo version 1.16.0)
 of 2020-01-22 built on INDRA
Repository revision: 5715eb94e90b33ace59dd4c4ccb6e2122bc6db72
Repository branch: master
System Description: Fedora 31 (Thirty One)

Recent messages:
>>>activated: get rid of auto-revert-verbosity<<<
>>>loaded: GNU Emacs 28.0.50 of 2020-01-22<<<
>>>load time: Thursday, February 27, 2020 13:27:29<<<
>>>activated: themes<<<
Desktop: 3 buffers restored, 1 to restore lazily.
For information about GNU Emacs and the GNU system, type C-M-h C-a.
Desktop lazily opening mem_log (1 remaining)...done
Lazy desktop load complete
Making completion list... [4 times]
user-error: End of history; no default available [2 times]

Configured using:
 'configure --prefix=/opt/emacs --sysconfdir=/etc
 --enable-locallisppath=/usr/local/share/emacs/site-lisp
 --libexecdir=/opt/emacs/lib/ --localstatedir=/usr/local/var
 --enable-largefile --with-x-toolkit=gtk3 --with-modules --with-xwidgets
 --without-pop --without-selinux --without-gnutls
 --with-file-notification=yes --with-json --with-cairo
 'CFLAGS=-march=native -Os''

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS XWIDGETS
LIBSYSTEMD JSON PDUMPER GMP

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Info

Minor modes in effect:
  show-paren-mode: t
  recentf-mode: t
  desktop-save-mode: 1
  delete-selection-mode: t
  cua-mode: t
  global-hl-line-mode: t
  gpm-mouse-mode: t
  savehist-mode: t
  global-auto-revert-mode: t
  auto-insert-mode: t
  smooth-scroll-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  save-place-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-quote-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  global-prettify-symbols-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  global-visual-line-mode: t
  visual-line-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired-sort-menu dired
dired-loaddefs rfc822 mml mml-sec epa epg epg-config gnus-util rmail
rmail-loaddefs text-property-search mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils cursor-sensor
term/linux user-profile powerline powerline-separators powerline-themes
server default-profile default-ui paren man recentf sh-script smie
executable vc-dispatcher vc-svn enriched mule-util jka-compr info
desktop frameset avoid default-faces default-keymap delsel cua-base
default-run default-restart default-modes default-platform default-nw
hl-line t-mouse default-unix default-flymake default-flymake-go
default-flymake-csharp default-flymake-ruby default-flymake-js
default-flymake-py default-flymake-java default-flymake-shell
flymake-proc flymake compile warnings default-run-assoc run-assoc
default-tempo default-tempo-rexx default-tempo-sh default-tempo-pas
default-tempo-js default-tempo-java default-tempo-perl
default-tempo-elisp default-tempo-c-cpp default-menu default-help
default-options default-tools default-search default-format default-view
aquamacs-cmm-menu default-edit default-file default-generic savehist
autorevert filenotify autoinsert default-functions default-autoload
ox-man ox-odt rng-loc rng-uri rng-parse rng-match rng-dt rng-util
rng-pttrn nxml-parse nxml-ns nxml-enc xmltok nxml-util ox-latex
ox-icalendar ox-html table ox-ascii ox-publish ox org-element org ob
ob-tangle ob-ref ob-lob ob-table org-macro org-footnote org-src
ob-comint org-pcomplete pcomplete comint ansi-color ring org-list
org-faces org-entities time-date org-version ob-emacs-lisp org-table
org-keys org-loaddefs find-func cal-menu calendar cal-loaddefs avl-tree
generator ol ob-exp ob-core org-compat ob-eval org-macs format-spec
markdown-mode rx color thingatpt noutline outline easy-mmode jison-mode
bison-mode cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align
cc-engine cc-vars cc-defs flex-mode derived aquamacs-tabbar cus-start
cus-load aquamacs-tools tabbar restore-last-frame-size org-bullets
syslog-mode hide-lines web-mode disp-table vimrc-mode go-autocomplete
go-mode-autoloads neotree advice smooth-scroll aok fill-column-indicator
tempbuf auto-complete-config auto-complete edmacro kmacro popup
undo-tree diff multi-shell windata tree-mode tree-widget wid-edit imenu
imenu-tree tempo saveplace google-translate google-translate-default-ui
google-translate-core-ui ido google-translate-core google-translate-tk
google-translate-backend url url-proxy url-privacy url-expand
url-methods url-history url-cookie url-domsuf url-util url-parse
auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs cl-loaddefs
cl-lib password-cache url-vars mailcap json subr-x map seq byte-opt gv
bytecomp byte-compile cconv eol-conversion easymenu default-path tooltip
eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel
term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame minibuffer cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
inotify dynamic-setting system-font-setting font-render-setting
xwidget-internal cairo move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 253383 185135)
 (symbols 48 26324 10)
 (strings 32 90935 29593)
 (string-bytes 1 2893472)
 (vectors 16 35482)
 (vector-slots 8 389169 203210)
 (floats 8 284 1433)
 (intervals 56 2505 232)
 (buffers 1000 16))

bug#39808: 28.0.50; toolbar broken

[Robert Pluim]

>>>>> On Thu, 27 Feb 2020 13:30:02 +0100, Markus FFM <markusffm@fn.de> said:

    Markus> emacs -Q
    Markus> Toolbar: Discard (kill) current buffer 
    Markus> Wrong type argument: symbolp, #<frame emacs@HOST 0x2444b50>
    Markus> same with all other toolbar items

Iʼve bisected this down to:

3b4bd4be1dfa8717cb6911bd57c4c7d9d13614b4

* 3b4bd4be1d (HEAD, refs/bisect/bad) * src/keyboard.c (make_lispy_event): Generate proper tool-bar events.
|     - Stefan Monnier <monnier@iro.umontreal.ca>

Robert

bug#39808: 28.0.50; toolbar broken

[Stefan Monnier]

> Iʼve bisected this down to:
>
> 3b4bd4be1dfa8717cb6911bd57c4c7d9d13614b4
>
> * 3b4bd4be1d (HEAD, refs/bisect/bad) * src/keyboard.c (make_lispy_event):
> Generate proper tool-bar events.
> |     - Stefan Monnier <monnier@iro.umontreal.ca>

Looks like my grep-fu was very weak, indeed.
I installed the patch below, which should hopefully fix the problem for
Gtk and NS builds,


        Stefan


diff --git a/src/gtkutil.c b/src/gtkutil.c
index 5e7cf3d211..338c6036c2 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -4398,13 +4398,6 @@ xg_tool_bar_callback (GtkWidget *w, gpointer client_data)
   key = AREF (f->tool_bar_items, idx + TOOL_BAR_ITEM_KEY);
   XSETFRAME (frame, f);
 
-  /* We generate two events here.  The first one is to set the prefix
-     to `(tool_bar)', see keyboard.c.  */
-  event.kind = TOOL_BAR_EVENT;
-  event.frame_or_window = frame;
-  event.arg = frame;
-  kbd_buffer_store_event (&event);
-
   event.kind = TOOL_BAR_EVENT;
   event.frame_or_window = frame;
   event.arg = key;
diff --git a/src/nsterm.m b/src/nsterm.m
index c0535825ee..aefbb2721e 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -8185,12 +8185,6 @@ - (instancetype)toolbarClicked: (id)item
   if (!emacs_event)
     return self;
 
-  /* Send first event (for some reason two needed).  */
-  theEvent = [[self window] currentEvent];
-  emacs_event->kind = TOOL_BAR_EVENT;
-  XSETFRAME (emacs_event->arg, emacsframe);
-  EV_TRAILER (theEvent);
-
   emacs_event->kind = TOOL_BAR_EVENT;
   /* XSETINT (emacs_event->code, 0); */
   emacs_event->arg = AREF (emacsframe->tool_bar_items,

bug#39808: 28.0.50; toolbar broken

[Stefan Monnier]

> why not committing your changes to our git repository?

I don't understand what you mean.  What's "our git repository"?
As I wrote, I already installed the patch (in the `master` branch, in
the emacs.git official repository).


        Stefan

bug#39808: 28.0.50; toolbar broken

[Robert Pluim]

>>>>> On Thu, 27 Feb 2020 11:19:53 -0500, Stefan Monnier <monnier@iro.umontreal.ca> said:

    >> Iʼve bisected this down to:
    >> 
    >> 3b4bd4be1dfa8717cb6911bd57c4c7d9d13614b4
    >> 
    >> * 3b4bd4be1d (HEAD, refs/bisect/bad) * src/keyboard.c (make_lispy_event):
    >> Generate proper tool-bar events.
    >> |     - Stefan Monnier <monnier@iro.umontreal.ca>

    Stefan> Looks like my grep-fu was very weak, indeed.

The '.m' used by the NS files always trips me up.

    Stefan> I installed the patch below, which should hopefully fix the problem for
    Stefan> Gtk and NS builds,

That fixes it for GTK. For NS, the buffer does not get killed, and I
get the following in the terminal:

2020-02-27 17:40:03.139 emacs[41579:935742] -[NSControlAuxiliary
modifierFlags]: unrecognized selector sent to instance 0x7fd582c6c8c0

To be fair, it might have been doing this before: I never run with the
tool bar.

Robert

bug#39808: 28.0.50; toolbar broken

[Robert Pluim]

>>>>> On Thu, 27 Feb 2020 17:43:46 +0100, Robert Pluim <rpluim@gmail.com> said:

    Robert> That fixes it for GTK. For NS, the buffer does not get killed, and I
    Robert> get the following in the terminal:

    Robert> 2020-02-27 17:40:03.139 emacs[41579:935742] -[NSControlAuxiliary
    Robert> modifierFlags]: unrecognized selector sent to instance 0x7fd582c6c8c0

    Robert> To be fair, it might have been doing this before: I never run with the
    Robert> tool bar.

I think you were over-enthusiastic with your pruning, I put back the
initialisation of theEvent, that fixes this (Iʼm a bit pressed for
time tonight, so canʼt commit it right now).

diff --git a/src/nsterm.m b/src/nsterm.m
index aefbb2721e..84acb61dcd 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -8185,6 +8185,7 @@ - (instancetype)toolbarClicked: (id)item
   if (!emacs_event)
     return self;
 
+  theEvent = [[self window] currentEvent];
   emacs_event->kind = TOOL_BAR_EVENT;
   /* XSETINT (emacs_event->code, 0); */
   emacs_event->arg = AREF (emacsframe->tool_bar_items,

bug#39808: 28.0.50; toolbar broken

[Stefan Monnier]

>     Stefan> Looks like my grep-fu was very weak, indeed.
> The '.m' used by the NS files always trips me up.

Yes, that's the regular weakness.  But in the present case I also missed
the occurrence in the gtkutil.c file.  I have no idea how that happened.

>     Stefan> I installed the patch below, which should hopefully fix
>     Stefan> the problem for Gtk and NS builds,
>
> That fixes it for GTK. For NS, the buffer does not get killed, and I
> get the following in the terminal:
[...]
> I think you were over-enthusiastic with your pruning, I put back the
> initialisation of theEvent, that fixes this (Iʼm a bit pressed for
> time tonight, so canʼt commit it right now).

Thanks, I pushed it for you,


        Stefan

bug#39808: 28.0.50; toolbar broken

[Stefan Monnier]

> thanks a lot Stefan. It works perfectly!

Thank you for the confirmation.
Closing,


        Stefan