Re: New bug with mouse-3

[Stefan Monnier]

> This is in recent cvs, noticed first a couple of days ago.  Clicking
> down mouse-3 works usually fine, and
> 
> C-h k   click on mouse-3
> 
> pops up a help buffer as usual:
> ====
> <mouse-3> (translated from <down-mouse-3> <mouse-3>) at that spot runs the 
> command mouse-save-then-kill
>    which is an interactive compiled Lisp function in `mouse'.
> It is bound to <mouse-3>.
> (mouse-save-then-kill CLICK)
> 
> Save text to point in kill ring; the second time, kill the text.
> ...
> ===
> 
> But after loading pcvs-defs.el, mouse-3 does not work anymore, just
> beeps.  Moreover

Here is what happens:

- originally, we have:

        ELISP> (symbol-plist 'down-mouse-3)
        (event-symbol-element-mask
         (mouse-3 2)
         event-symbol-elements
         (mouse-3 down))

- loading pcvs-defs causes [(down-mouse-3)] to be passed to
  event-convert-list which then assumes `down-mouse-3' to be a base
  event with no modifiers and thus changes the proplist to:

        ELISP> (symbol-plist 'down-mouse-3)
        (event-symbol-element-mask
         (down-mouse-3 0)
         event-symbol-elements
         (down-mouse-3)
         modifier-cache
         ((0 . down-mouse-3)))

- then when another down-mouse-3 event comes up, the "parsing" just
  reuses the event-symbol-element-mask which says it has no
  modifier and thus isn't recognized as a down event and is thus not
  automatically dropped (if unbound).

Now why didn't this happen before ?  Well, here is how things looked
with Emacs-21.3 at startup:

        ELISP> (symbol-plist 'down-mouse-3)
        (event-symbol-element-mask
         (down-mouse-3 0)
         event-symbol-elements
         (down-mouse-3)
         modifier-cache
         ((67108864 . C-down-mouse-3)
          (0 . down-mouse-3)))

Now you'll say that it looks even worse since the wrong parsing info
(the one that's giving us problem) is already present even before
we load pcvs-defs.el.  And yet worked.  How did it work ?

For that we have to look at the properties on `mouse-3' because when
a click comes in, we build the event symbol by taking the
base (i.e. `mouse-3') and adding modifiers to it.  What
it was in Emacs-21.3:

        ELISP> (symbol-plist 'mouse-3)
        (event-symbol-element-mask
         (mouse-3 8)
         event-symbol-elements
         (mouse-3 click)
         modifier-cache
         ((-134217728 . M-mouse-3)
          (0 . mouse-3)))

Since it has no info for "mouse-3 + down" in its modifier-cache, the
apply_modifiers function will build the down-mouse-3 symbol and will
reset down-mouse-3's event-symbol-elements and event-symbol-element-mask,
overwriting the bogus values with the valid ones.  Notice that if
loading pcvs-defs.el in this scenario has no effect since the
modifier-cache of down-mouse-3 is already setup, so the cache lookup
succeed and we don't change any values.

On the trunk, OTOH, I fixed the parsing of down-mouse-3, so the start
state is correct, but that means that down-mouse-3 has no modifier-cache
so loading pcvs-defs.el will cause the cache lookup to fail and
we'll thus change down-mouse-3's event-symbol-elements property
to something incorrect.

This is related to the following problem with canonicalization of
event symbols:

        ELISP> (let ((map (make-sparse-keymap))) (define-key map [C-M-hello] 
'ha) map)
        (keymap (C-M-hello . ha))

        ELISP> (let ((map (make-sparse-keymap))) (define-key map [M-C-hello] 
'ha) map)
        (keymap (C-M-hello . ha))

        ELISP> (let ((map (make-sparse-keymap))) (define-key map [(M-C-hello)] 
'ha) map)
        (keymap (M-C-hello . ha))

        ELISP> (let ((map (make-sparse-keymap))) (define-key map [(meta 
C-hello)] 'ha) map)
        (keymap (M-C-hello . ha))

Note how the [M-C-hello] was canonicalized to [C-M-hello] but
[(M-C-hello)] wasn't.

The simplest patch for those problems is:

--- keyboard.c.~1.743.~ Thu May 15 13:19:09 2003
+++ keyboard.c  Thu May 15 16:46:24 2003
@@ -6090,13 +6090,6 @@
       /* Add the new symbol to the base's cache.  */
       entry = Fcons (index, new_symbol);
       Fput (base, Qmodifier_cache, Fcons (entry, cache));
-
-      /* We have the parsing info now for free, so add it to the caches.  */
-      XSETFASTINT (index, modifiers);
-      Fput (new_symbol, Qevent_symbol_element_mask,
-           Fcons (base, Fcons (index, Qnil)));
-      Fput (new_symbol, Qevent_symbol_elements,
-           Fcons (base, lispy_modifier_list (modifiers)));
     }
 
   /* Make sure this symbol is of the same kind as BASE.

It will cause parse_modifiers_uncached to be called a few more times
to fill out those cache entries not filled by apply_modifiers
(on my tests, starting Emacs, clicking with all three mouse buttons and
typing `hello', the difference is 46 vs 39 calls
to parse_modifiers_uncached.  On another test where I use the menu
a lot more, I get 137 vs 130).

I installed the above patch, so the problem should now be fixed,


        Stefan