Subject: Re: bug#74423: Low level key events

From: Po Lu <[email-masked]>
Date: Tue, 19 Nov 2024 07:49:00 +0800

Cecilio Pardo <[email-masked]> writes:

> +   switch (keysym)
> +    {
> +    case GDK_KEY_Shift_L:
> +    case GDK_KEY_Shift_R:
> +      modifier = Qshift;
> +      break;
> +    case GDK_KEY_Control_L:
> +    case GDK_KEY_Control_R:
> +      modifier = Vx_ctrl_keysym;
> +      if (NILP (modifier))
> +	modifier = Qctrl;
> +      break;
> +    case GDK_KEY_Alt_L:
> +    case GDK_KEY_Alt_R:
> +      modifier = Vx_meta_keysym;
> +      if (NILP (modifier))
> +	modifier = Qalt;
> +      break;
> +    case GDK_KEY_Meta_L:
> +    case GDK_KEY_Meta_R:
> +      modifier = Vx_meta_keysym;
> +      if (NILP (modifier))
> +	modifier = Qmeta;
> +      break;
> +    case GDK_KEY_Hyper_L:
> +    case GDK_KEY_Hyper_R:
> +      modifier = Vx_hyper_keysym;
> +      if (NILP (modifier))
> +	modifier = Qhyper;
> +      break;
> +    case GDK_KEY_Super_L:
> +    case GDK_KEY_Super_R:
> +      modifier = Vx_super_keysym;
> +      if (NILP (modifier))
> +	modifier = Qsuper;
> +      break;
> +    default:
> +      modifier = Qnil;
> +    }

This is not the proper means of establishing the modifier bound to a key
symbol in GTK, but I cannot immediately tell you what the alternative is.

> +	  my_post_msg( &wmsg, hwnd, WM_EMACS_LOW_LEVEL_KEY, wParam, lParam );
> +	}

> +	  my_post_msg( &wmsg, hwnd, WM_EMACS_LOW_LEVEL_KEY, wParam, lParam );
> +	}

Stylistic issues.

> +static void
> +x_maybe_send_low_level_key_event (struct x_display_info *dpyinfo,
> +				  const XEvent *xev)
> +{
> +  XKeyEvent xkey;
> +  bool is_press;
> +  KeySym keysym;
> +  Lisp_Object key, modifier;
> +  struct input_event ie;
> +
> +  if (!Venable_low_level_key_events)
> +    return;
> +
> +  switch (xev->type)
> +    {
> +    case KeyPress:
> +      is_press = true;
> +      xkey = xev->xkey;
> +      break;
> +    case KeyRelease:
> +      is_press = false;
> +      xkey = xev->xkey;
> +      break;
> +#ifdef HAVE_XINPUT2
> +    case GenericEvent:
> +      XIDeviceEvent *xiev = xev->xcookie.data;
> +      switch (xev->xgeneric.evtype)
> +	{
> +	case XI_KeyPress:
> +	  is_press = true;
> +	  break;
> +	case XI_KeyRelease:
> +	  is_press = false;
> +	  break;
> +	default:
> +	  return;
> +	}
> +
> +      xkey.serial = xiev->serial;
> +      xkey.send_event = xiev->send_event;
> +      xkey.display = xiev->display;
> +      xkey.window = xiev->event;
> +      xkey.root = xiev->root;
> +      xkey.subwindow = xiev->child;
> +      xkey.time = xiev->time;
> +      xkey.x = xiev->event_x;
> +      xkey.y = xiev->event_y;
> +      xkey.x_root = xiev->root_x;
> +      xkey.y_root = xiev->root_y;
> +      xkey.state = xiev->mods.effective;
> +      xkey.keycode = xiev->detail;
> +      xkey.same_screen = 1;
> +      break;
> +#endif
> +    default:
> +      return;
> +    }
> +
> +  struct frame *f = x_any_window_to_frame (dpyinfo, xkey.window);
> +  if (!f)
> +    return;
> +
> +  XLookupString (&xkey, NULL, 0, &keysym, NULL);
> +
> +  switch (keysym)
> +    {
> +    case XK_Shift_L: key = Qlshift; break;
> +    case XK_Shift_R: key = Qrshift; break;
> +    case XK_Control_L: key = Qlctrl; break;
> +    case XK_Control_R: key = Qrctrl; break;
> +    case XK_Alt_L: key = Qlalt; break;
> +    case XK_Alt_R: key = Qralt; break;
> +    default:
> +      key = Qnil;
> +    }

This doesn't generate `ralt' events on my system, as that key produces
the keysym ISO_Level3_Shift/

> +   switch (keysym)
> +    {
> +    case XK_Shift_L:
> +    case XK_Shift_R:
> +      modifier = Qshift;
> +      break;
> +    case XK_Control_L:
> +    case XK_Control_R:
> +      modifier = Vx_ctrl_keysym;
> +      if (NILP (modifier))
> +	modifier = Qctrl;
> +      break;
> +    case XK_Alt_L:
> +    case XK_Alt_R:
> +      modifier = Vx_meta_keysym;
> +      if (NILP (modifier))
> +	modifier = Qalt;
> +      break;
> +    case XK_Meta_L:
> +    case XK_Meta_R:
> +      modifier = Vx_meta_keysym;
> +      if (NILP (modifier))
> +	modifier = Qmeta;
> +      break;
> +    case XK_Hyper_L:
> +    case XK_Hyper_R:
> +      modifier = Vx_hyper_keysym;
> +      if (NILP (modifier))
> +	modifier = Qhyper;
> +      break;
> +    case XK_Super_L:
> +    case XK_Super_R:
> +      modifier = Vx_super_keysym;
> +      if (NILP (modifier))
> +	modifier = Qsuper;
> +      break;
> +    default:
> +      modifier = Qnil;
> +    }

You are computing the modifier event to be generated incorrectly.  You
should search the modifier map (dpyinfo->modmap) for columns containing
received keysyms, and compare modifier bits derived from the rows where
they appear against meta_mod_mask, shift_lock_mask, alt_mod_mask,
super_mod_mask, and hyper_mod_mask in the display structure, or CtrlMask
and ShiftMask.

>      case KeyPress:
> +      x_maybe_send_low_level_key_event (dpyinfo, event);
>        x_display_set_last_user_time (dpyinfo, event->xkey.time,
>  				    event->xkey.send_event,
>  				    true);
> @@ -20715,6 +20853,7 @@ handle_one_xevent (struct x_display_info *dpyinfo,
>  #endif
>  
>      case KeyRelease:
> +      x_maybe_send_low_level_key_event (dpyinfo, event);
>  #ifdef HAVE_X_I18N
>        /* Don't dispatch this event since XtDispatchEvent calls
>           XFilterEvent, and two calls in a row may freeze the
> @@ -23970,6 +24109,8 @@ handle_one_xevent (struct x_display_info *dpyinfo,
>  	      struct xi_device_t *device, *source;
>  	      XKeyPressedEvent xkey;
>  
> +	      x_maybe_send_low_level_key_event (dpyinfo, event);
> +
>  	      coding = Qlatin_1;
>  
>  	      /* The code under this label is quite desultory.  There
> @@ -24586,6 +24727,8 @@ handle_one_xevent (struct x_display_info *dpyinfo,
>  #endif
>  
>  	  case XI_KeyRelease:
> +	    x_maybe_send_low_level_key_event (dpyinfo, event);

Would you rearrange the locations of calls to x_any_window_to_frame in
handle_one_xevent so that you need not redundantly call the same
function in x_maybe_send_low_level_key_event?

Thanks for your interest in Emacs.