bug#31371: 26.1; Menu-bar stops working after search

bug#31371: 26.1; Menu-bar stops working after search

[Nick Helm]

Emacs menu-bar does not work after a help search on macOS.

  Emacs -Q
  Click "Help" in the macOS menu-bar
  Enter any text in the help search field, e.g. "text"
  Move the mouse pointer to the right until the help menu disappears
  Move the mouse pointer back over "Help"

Note that the Help menu does not reappear and the mouse does not move
correctly (slows down) while moving over the word "Help". Subsequent
clicks on the menu-bar do not work as expected either. 

This can also cause frames not receive focus correctly – e.g. two frames
can simultaneously appear to have focus or all frames can be out of
focus while Emacs is the foreground app. The latter can give the
impression that Emacs has locked up, as no keystrokes reach the frame.

It's possible to avoid the problem by hiding the whole system-provided
help menu in nsterm.m, but this is obviously not a great solution.



In GNU Emacs 26.1 (build 1, x86_64-apple-darwin17.5.0, NS appkit-1561.40 Version 10.13.4 (Build 17E202))
 of 2018-04-28 built on jupiter.local
Windowing system distributor 'Apple', version 10.3.1561
Recent messages:
Opening nnimap server on Office365...
Opening connection to localhost via shell...
Opening connection to localhost...done
Opening nnimap server on Office365...done
Opening nntp server on Gmane...done
1 new newsgroup has arrived
Checking new news...
Reading active file via nnnil...done
Reading active file via nndraft...done
Checking new news...done

Configured using:
 'configure --with-gnutls=no'

Configured features:
JPEG NOTIFY ACL LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS THREADS LCMS2

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

Major mode: Group

Minor modes in effect:
  gnus-undo-mode: t
  savehist-mode: t
  global-eldoc-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow gnus-cite sort mail-extr nnir emacsbug sendmail gnus-async
gnus-ml disp-table gnus-demon nndraft nnmh cl-extra help-mode utf-7
network-stream nsm auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs starttls nnfolder nnnil gnus-agent gnus-srvr gnus-score
score-mode nnvirtual gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime
smime dig mailcap nntp gnus-cache gnus-sum gnus-group gnus-undo
gnus-start gnus-cloud nnimap nnmail mail-source tls gnutls utf7 netrc
nnoo parse-time gnus-spec gnus-int gnus-range message rmc puny seq
byte-opt bytecomp byte-compile cconv format-spec rfc822 mml mml-sec
password-cache epa derived epg epg-config mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader gnus-win gnus
nnheader gnus-util rmail rmail-loaddefs rfc2047 rfc2045 ietf-drums
mail-utils mm-util mail-prsvr wid-edit time dired-x easymenu dired
dired-loaddefs pcase savehist easy-mmode iso-transl edmacro kmacro
cl-loaddefs cl-lib gv plain-theme time-date tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel term/ns-win ns-win
ucs-normalize mule-util term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame 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 minibuffer 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 kqueue cocoa ns lcms2 multi-tty make-network-process emacs)

Memory information:
((conses 16 292450 17785)
 (symbols 48 29375 1)
 (miscs 40 60 366)
 (strings 32 55165 2991)
 (string-bytes 1 1653924)
 (vectors 16 44144)
 (vector-slots 8 831856 12902)
 (floats 8 221 584)
 (intervals 56 272 60)
 (buffers 992 19))

bug#31371: 26.1; Menu-bar stops working after search

[Alan Third]

On Sun, May 06, 2018 at 02:15:57AM +1200, Nick Helm wrote:
> 
> Emacs menu-bar does not work after a help search on macOS.
> 
>   Emacs -Q
>   Click "Help" in the macOS menu-bar
>   Enter any text in the help search field, e.g. "text"
>   Move the mouse pointer to the right until the help menu disappears
>   Move the mouse pointer back over "Help"
> 
> Note that the Help menu does not reappear and the mouse does not move
> correctly (slows down) while moving over the word "Help". Subsequent
> clicks on the menu-bar do not work as expected either. 

Looks like it goes into some sort of infinite loop calling
ns_update_menubar. If you go to the left so it opens some non‐help
menu, then go back to help it works fine (I think).

The menu code seems pretty horrible to me, so this may take some
time...
-- 
Alan Third

bug#31371: 26.1; Menu-bar stops working after search

[Nick Helm]

On Sun, 06 May 2018 at 22:14:11 +1200, Alan Third wrote:

> On Sun, May 06, 2018 at 02:15:57AM +1200, Nick Helm wrote:
>> 
>> Emacs menu-bar does not work after a help search on macOS.
>> 
>> Note that the Help menu does not reappear and the mouse does not move
>> correctly (slows down) while moving over the word "Help". Subsequent
>> clicks on the menu-bar do not work as expected either. 
>
> Looks like it goes into some sort of infinite loop calling
> ns_update_menubar. If you go to the left so it opens some non‐help
> menu, then go back to help it works fine (I think).
>
> The menu code seems pretty horrible to me, so this may take some
> time...

I had a look as well and I see what you mean, it's a bit of a mess.

It's a wild guess, but here's a theory: the problem happens because
mainMenu is trying to simultaneously be part NSMenu (Help's spotlight
search field and the context help topics) and part EmacsMenu (Help's
standard Lisp menu items).

When a user clicks on the menu bar, Emacs postpones the event via
menu_will_open_state, creates all the EmacsMenu menu items from Lisp and
regenerates the mouse click event with a call to
ns_check_pending_open_menu in order to actually display the menu.

The trouble is, NSMenu doesn't know anything about this. When the user
clicks it immediately displays what it thinks is the Help menu (sans the
Lisp stuff, which doesn't exist yet). When the EmacsMenu part is ready,
it regenerates the click event to display the menu, but NSMenu
interprets this as an instruction to hide the menu. This repeats for
each dragging mouse event over the menu-bar, hence we have a loop.

If this is the case, it should cause problems even by simply opening and
closing the Help menu. And I think that's what we're seeing. From
Emacs -Q, try opening and closing the Help menu (ignore search), then
click between the visible frame and the desktop a few times. After a
couple of tries, the frame cannot regain proper focus and the menu-bar
doesn't operate at all.

I had a fiddle around with a couple of ideas. The first removes the
NSMenu parts of the Help menu by creating an empty menu object and using
it to override the system's default Help. Unfortunately, this removes
the search field and the context topics, but the EmacsMenu menu items
then work as expected. I don't think NS Emacs has any Apple Help Book
files (the only entries that appear seem to be auto-generated) so this
might not be so bad. The search field is really useful for command
discovery though.

I also tried interrupting the call to ns_check_pending_open_menu in
x_activate_menubar. If you comment the call out, the menu sort of works
as expected, other than having to initially generate two events (click
twice) to get the Lisp menus to appear on each frame (one click works
for the appMenu and Help, but they only contain the NSMenu menu items,
as expected). I tried to make the call conditional on menu_state or
trackingMenu, but I haven't got it working.

Maybe I don't understand why a custom menu class is necessary, but I
think the right solution is to convert Help from EmacsMenu to NSMenu.
And, if we're going to do that, why not convert all of mainMenu and do
everything with standard NSMenu and NSMenuItem methods? 

All of the delayed events and tracking stuff seems over-complicated and
unnecessary. What am I missing? 

bug#31371: 26.1; Menu-bar stops working after search

[Nick Helm]

On Mon, 07 May 2018 at 14:48:16 +1200, Nick Helm wrote:

> here's a theory: the problem happens because mainMenu is trying to
> simultaneously be part NSMenu (Help's spotlight search field and the
> context help topics) and part EmacsMenu (Help's standard Lisp menu
> items).

On second glance, I don't think this is quite right, at least it's not
the whole story.

Other apps seem to have a related problem. Open Terminal for example,
click on Help, mouse down and click in the Terminal window, then try to
type a command. It doesn't work, I just get system beeps. TextEdit is
the same.

It's like the Help memu isn't returning focus correctly. Maybe it's a
bug in macOS making this show up?

bug#31371: 26.1; Menu-bar stops working after search

[Alan Third]

On Mon, May 07, 2018 at 02:48:16PM +1200, Nick Helm wrote:
> If this is the case, it should cause problems even by simply opening and
> closing the Help menu. And I think that's what we're seeing. From
> Emacs -Q, try opening and closing the Help menu (ignore search), then
> click between the visible frame and the desktop a few times. After a
> couple of tries, the frame cannot regain proper focus and the menu-bar
> doesn't operate at all.

I can’t replicate this. Nor your experiment with textedit in the other
email. I definitely see the issue you originally described, though.

> All of the delayed events and tracking stuff seems over-complicated and
> unnecessary. What am I missing? 

Apparently it was added because regenerating the menus calls lisp,
which is then able to call ns_select, however the code to regenerate
the menus is called from within the NS app loop within ns_select, so
we end up with the app loop running within the app loop via ns_select.

This isn’t allowed.

I notice all this code is cocoa only, though. Makes me wonder why
GNUstep is different. (The menus on GNUstep Emacs are awful, though,
they flicker constantly.)

I still don’t understand how this works, because I get the impression
from the code that the menus are generated when the user clicks on
them, but they clearly change when you switch windows or frame or
whatever.

So on the one hand it appears they’re updated within the NS app loop
as part of a response to the user clicking on the menu, but on the
other they’re generated outside the app loop by some lisp function or
something.

Maybe it’s both, in which case why?

Of course, none of this might be relevant to the help menu bug. I’m
quite lost at the moment.
-- 
Alan Third

bug#31371: 26.1; Menu-bar stops working after search

[Nick Helm]

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

I think I've worked out what's going on here. I've attached a patch -
could you give it a try and see if works for you? 

When the user types in the search field, NSMenu looks for matching
candidates by creating events to open each menu, trigger an update and
read the results. If the search field already contains text, this
happens as soon as the Help menu opens, either when the user clicks Help
or mouse drags onto the Help menu. 

The code in ns_check_menu_open and ns_check_pending_open_menu that
postpones mouse clicks (to fetch menus from Lisp) also tries to postpone
these drag and search events. When it releases a delayed click on Help
(even if the event wasn't a click to begin with), the menu reopens and
the process loops.

The attached patch gets around this by never postponing mouse drag or
non-user mouse down events.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 31371.patch --]
[-- Type: text/x-patch, Size: 1385 bytes --]

--- a/src/nsterm.m	2018-05-13 16:45:39.000000000 +1200
+++ b/src/nsterm.m	2018-05-13 16:44:31.000000000 +1200
@@ -4299,14 +4299,22 @@
           NSEvent *theEvent = [NSApp currentEvent];
           struct frame *emacsframe = SELECTED_FRAME ();
 
-          [menu cancelTracking];
-          menu_will_open_state = MENU_PENDING;
-          emacs_event->kind = MENU_BAR_ACTIVATE_EVENT;
-          EV_TRAILER (theEvent);
-
-          CGEventRef ourEvent = CGEventCreate (NULL);
-          menu_mouse_point = CGEventGetLocation (ourEvent);
-          CFRelease (ourEvent);
+          /*On macOS, the following can cause an event loop when the 
+            Spotlight for Help search field is populated.  Avoid this by
+            not postponing mouse drag and non-user-generated mouse down 
+            events.  (bug#31371).*/
+          if (([theEvent type] == NSEventTypeLeftMouseDown)
+              && [theEvent eventNumber])
+            {
+              [menu cancelTracking];
+              menu_will_open_state = MENU_PENDING;
+              emacs_event->kind = MENU_BAR_ACTIVATE_EVENT;
+              EV_TRAILER (theEvent);
+
+              CGEventRef ourEvent = CGEventCreate (NULL);
+              menu_mouse_point = CGEventGetLocation (ourEvent);
+              CFRelease (ourEvent);
+            }
         }
       else if (menu_will_open_state == MENU_OPENING)
         {

[-- Attachment #3: Type: text/plain, Size: 472 bytes --]



On Wed, 09 May 2018 at 09:40:24 +1200, Alan Third wrote:

> I notice all this code is cocoa only, though. Makes me wonder why
> GNUstep is different. (The menus on GNUstep Emacs are awful, though,
> they flicker constantly.)

I think Cocoa uses a delegate method to update single submenus which
(I've read) isn't supported on GNUStep. This means ns_update_menubar has
to recreate every menu on every call (deep_p is always 1). Maybe that
has something to do with it?

bug#31371: 26.1; Menu-bar stops working after search

[George Plymale II]

Hi,

Just passing by since I saw this bug mentioned by Alan Third on
emacs-devel. I want to point out that this bug does not occur at all on
the Emacs Mac Port by Mitsuharu Yamamoto. I've described it in more
detail here: https://lists.gnu.org/archive/html/emacs-devel/2018-05/msg00404.html

Thanks,
- George Plymale II

bug#31371: 26.1; Menu-bar stops working after search

[Nick Helm]

On Sat, 19 May 2018 at 09:33:39 +1200, George Plymale II wrote:

> I want to point out that this bug does not occur at all on
> the Emacs Mac Port by Mitsuharu Yamamoto.

Thanks. Yes, you're right, the mac-port doesn't suffer from this
particular issue. Although it handles menu-bar events slightly
differently, it takes the same general approach to the problem as we do
on NS.

BTW, has anyone had a chance to try my patch for this bug?

bug#31371: 26.1; Menu-bar stops working after search

[Stefan Kangas]

tags 31371 + patch
quit

Nick Helm <nick@tenpoint.co.nz> writes:

> I think I've worked out what's going on here. I've attached a patch -
> could you give it a try and see if works for you?
>
> When the user types in the search field, NSMenu looks for matching
> candidates by creating events to open each menu, trigger an update and
> read the results. If the search field already contains text, this
> happens as soon as the Help menu opens, either when the user clicks Help
> or mouse drags onto the Help menu.
>
> The code in ns_check_menu_open and ns_check_pending_open_menu that
> postpones mouse clicks (to fetch menus from Lisp) also tries to postpone
> these drag and search events. When it releases a delayed click on Help
> (even if the event wasn't a click to begin with), the menu reopens and
> the process loops.
>
> The attached patch gets around this by never postponing mouse drag or
> non-user mouse down events.

I've tried your patch on macOS 10.13, and it fixes the issues with the
help menu for me.  The code looks okay to me, but I don't know much
about Objective-C or macOS development, so it would be good if someone
else could review it too.

Nick, could you please provide a ChangeLog entry for these changes and
send them using "git format-patch -1"?  Details on how to do that well
are in the CONTRIBUTE file.  Thanks in advance.

Best regards,
Stefan Kangas

bug#31371: 26.1; Menu-bar stops working after search

[Robert Pluim]

>>>>> On Fri, 11 Oct 2019 13:25:06 +0200, Stefan Kangas <stefan@marxist.se> said:
    Stefan> I've tried your patch on macOS 10.13, and it fixes the issues with the
    Stefan> help menu for me.  The code looks okay to me, but I don't know much
    Stefan> about Objective-C or macOS development, so it would be good if someone
    Stefan> else could review it too.

I donʼt know anything about those either, but it definitely fixes the
reported issue for me on macOS 10.14 (Iʼm too chicken to move to 10.15
just yet).

    Stefan> Nick, could you please provide a ChangeLog entry for these changes and
    Stefan> send them using "git format-patch -1"?  Details on how to do that well
    Stefan> are in the CONTRIBUTE file.  Thanks in advance.

Thereʼs also trailing whitespace in the comment, and missing spaces
after '.'

Thanks

Robert

bug#31371: 26.1; Menu-bar stops working after search

[Nick]

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

Stefan Kangas <stefan@marxist.se> writes:

> I've tried your patch on macOS 10.13, and it fixes the issues with the
> help menu for me.  The code looks okay to me, but I don't know much
> about Objective-C or macOS development, so it would be good if someone
> else could review it too.
>
> Nick, could you please provide a ChangeLog entry for these changes and
> send them using "git format-patch -1"?  Details on how to do that well
> are in the CONTRIBUTE file.  Thanks in advance.

Thanks for testing. Updated patch attached.

Nick


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-unresponsive-Help-menu-in-macOS.patch --]
[-- Type: text/x-patch, Size: 1878 bytes --]

From 9ec201c229fdb8f9fbed1050961730daaf9db718 Mon Sep 17 00:00:00 2001
From: Nick Helm <nick@tenpoint.co.nz>
Date: Mon, 14 Oct 2019 14:11:25 +1300
Subject: [PATCH] Fix unresponsive Help menu in macOS

* src/nsterm.m (ns_check_menu_open): Don't postpone mouse drag and
non-user-generated mouse down events (Bug#31371).
---
 src/nsterm.m | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index bbd2c84..cc21ec3 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -4292,14 +4292,22 @@ in certain situations (rapid incoming events).
           NSEvent *theEvent = [NSApp currentEvent];
           struct frame *emacsframe = SELECTED_FRAME ();
 
-          [menu cancelTracking];
-          menu_will_open_state = MENU_PENDING;
-          emacs_event->kind = MENU_BAR_ACTIVATE_EVENT;
-          EV_TRAILER (theEvent);
-
-          CGEventRef ourEvent = CGEventCreate (NULL);
-          menu_mouse_point = CGEventGetLocation (ourEvent);
-          CFRelease (ourEvent);
+          /* On macOS, the following can cause an event loop when the
+             Spotlight for Help search field is populated.  Avoid this by
+             not postponing mouse drag and non-user-generated mouse down
+             events (Bug#31371).  */
+          if (([theEvent type] == NSEventTypeLeftMouseDown)
+              && [theEvent eventNumber])
+            {
+              [menu cancelTracking];
+              menu_will_open_state = MENU_PENDING;
+              emacs_event->kind = MENU_BAR_ACTIVATE_EVENT;
+              EV_TRAILER (theEvent);
+
+              CGEventRef ourEvent = CGEventCreate (NULL);
+              menu_mouse_point = CGEventGetLocation (ourEvent);
+              CFRelease (ourEvent);
+            }
         }
       else if (menu_will_open_state == MENU_OPENING)
         {
-- 
2.20.1 (Apple Git-117)

bug#31371: 26.1; Menu-bar stops working after search

[Stefan Kangas]

> Thanks for testing. Updated patch attached.

Thanks.  Are there any objections to installing this fix?

Best regards,
Stefan Kangas

bug#31371: 26.1; Menu-bar stops working after search

[Robert Pluim]

>>>>> On Mon, 14 Oct 2019 14:44:21 +0200, Stefan Kangas <stefan@marxist.se> said:

    >> Thanks for testing. Updated patch attached.
    Stefan> Thanks.  Are there any objections to installing this fix?

LGTM.

Robert

bug#31371: 26.1; Menu-bar stops working after search

[Alan Third]

On Mon, Oct 14, 2019 at 02:44:21PM +0200, Stefan Kangas wrote:
> > Thanks for testing. Updated patch attached.
> 
> Thanks.  Are there any objections to installing this fix?

My only concern is that I believe the postponement stops lisp codie
being run within the NS event loop, but if nobody’s seeing any crashes
with the patch applied then I have no problem with it being applied.

(caveat: I’ve not tried the patch myself)
-- 
Alan Third

bug#31371: 26.1; Menu-bar stops working after search

[Stefan Kangas]

Alan Third <alan@idiocy.org> writes:

> On Mon, Oct 14, 2019 at 02:44:21PM +0200, Stefan Kangas wrote:
>> > Thanks for testing. Updated patch attached.
>> 
>> Thanks.  Are there any objections to installing this fix?
>
> My only concern is that I believe the postponement stops lisp codie
> being run within the NS event loop, but if nobody’s seeing any crashes
> with the patch applied then I have no problem with it being applied.
>
> (caveat: I’ve not tried the patch myself)

Thanks.  I haven't seen any crashes, but I haven't subjected it to any
heavy testing besides making sure that it solves the issue.

Perhaps we could apply it to give it some real world testing.  We
could always revert the patch before Emacs 27.1 is released if it
turns out to cause any problems.  Any objections to that plan?
Otherwise, I'll go ahead and do that in a couple of days.

Best regards,
Stefan Kangas

bug#31371: 26.1; Menu-bar stops working after search

[Stefan Kangas]

Stefan Kangas <stefan@marxist.se> writes:

> Perhaps we could apply it to give it some real world testing.  We
> could always revert the patch before Emacs 27.1 is released if it
> turns out to cause any problems.  Any objections to that plan?
> Otherwise, I'll go ahead and do that in a couple of days.

No objections within 4 days, so I've now pushed the fix to master as
commit 6daa80d04e.  I'll leave the bug open for another couple of
weeks.  Please report here if you see any issues related to this
patch.

Best regards,
Stefan Kangas

bug#31371: 26.1; Menu-bar stops working after search

[Alan Third]

On Sat, Nov 09, 2019 at 11:18:36AM +0100, Stefan Kangas wrote:
> Stefan Kangas <stefan@marxist.se> writes:
> 
> > Perhaps we could apply it to give it some real world testing.  We
> > could always revert the patch before Emacs 27.1 is released if it
> > turns out to cause any problems.  Any objections to that plan?
> > Otherwise, I'll go ahead and do that in a couple of days.
> 
> No objections within 4 days, so I've now pushed the fix to master as
> commit 6daa80d04e.  I'll leave the bug open for another couple of
> weeks.  Please report here if you see any issues related to this
> patch.

Thanks.
-- 
Alan Third

bug#31371: 26.1; Menu-bar stops working after search

[Stefan Kangas]

close 31371 27.1
thanks

Stefan Kangas <stefan@marxist.se> writes:

> No objections within 4 days, so I've now pushed the fix to master as
> commit 6daa80d04e.  I'll leave the bug open for another couple of
> weeks.  Please report here if you see any issues related to this
> patch.

Since no one has reported any issues with this patch within 7 weeks,
I'm closing this bug report now.

Best regards,
Stefan Kangas