Re: [PATCH] On the nasty "ghost key" problem on NS

[Kai Ma <justksqsf@gmail.com>]

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


> On Nov 4, 2022, at 23:09, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> 
> I'm glad we found a way to make the code work, apparently, but
> Here we need a comment explaining why we do this gymnastic of
> `safe_call` + `inhibit_quit` + `waiting_for_input`.
> It's very unusual to have to do that.

Thanks for the pointer, this indeed turned out unnecessary (and dangerous).

A tester informed me of a problem in the v3 patch:

  safe_call (0, Qns_in_echo_area)

is incorrect. The 0 should be 1, or nargs will be -1 for funcall_general.  
This will cause an error to be signaled, which explains why 
`waiting_for_input` has to be masked.

[ I guess we could add an assertion that numargs >= 0 in funcall_general or
  somewhere else? ]

This patch should be correct even without the ugly `waiting_for_input` hack.
I’ve been running patched Emacs for some time without issues.

With the current understanding of the bug, I guess the comment line could be
  /* Protect against throw-on-input. */

WDYT?



[-- Attachment #2.1: Type: text/html, Size: 2702 bytes --]

[-- Attachment #2.2: fix-ghost-key-v4.patch --]
[-- Type: application/octet-stream, Size: 1193 bytes --]

diff --git a/src/nsterm.m b/src/nsterm.m
index 17f40dc7e3..0ed047849a 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -7063,16 +7063,19 @@ - (NSRect)firstRectForCharacterRange: (NSRange)theRange
   NSRect rect;
   NSPoint pt;
   struct window *win;
+  specpdl_ref count = SPECPDL_INDEX ();
 
   NSTRACE ("[EmacsView firstRectForCharacterRange:]");
 
   if (NS_KEYLOG)
     NSLog (@"firstRectForCharRange request");
 
-  if (WINDOWP (echo_area_window) && ! NILP (call0 (intern ("ns-in-echo-area"))))
+  specbind (Qinhibit_quit, Qt);
+  if (WINDOWP (echo_area_window) && ! NILP (safe_call (1, Qns_in_echo_area)))
     win = XWINDOW (echo_area_window);
   else
     win = XWINDOW (FRAME_SELECTED_WINDOW (emacsframe));
+  unbind_to (count, Qnil);
 
   rect.size.width = theRange.length * FRAME_COLUMN_WIDTH (emacsframe);
   rect.size.height = FRAME_LINE_HEIGHT (emacsframe);
@@ -11012,6 +11015,7 @@ Nil means use fullscreen the old (< 10.7) way.  The old way works better with
   DEFSYM (Qcondensed, "condensed");
   DEFSYM (Qreverse_italic, "reverse-italic");
   DEFSYM (Qexpanded, "expanded");
+  DEFSYM (Qns_in_echo_area, "ns-in-echo-area");
 
 #ifdef NS_IMPL_COCOA
   Fprovide (Qcocoa, Qnil);

[-- Attachment #2.3: Type: text/html, Size: 223 bytes --]