Lars, the attached patch to xforms' forms.c cures these two bugs in xforms 
handling of key events.

o xforms should swallow null keyevents. They occur during composition of 
multi-byte chars. The problem was reported and a solution proposed here:

o xforms should pass FL_KEYRELEASE events to the widgets. Currently they are 
swallowed.

I find that holding down the 'a' key continuously results in this 
lyx -dbg 524292 (workarea + key)  output. 
Ie each and every KEYPRESS event is followed by a KEYRELEASE event.

Work area event KEYPRESS
Key is `a' [97]
Keysym is `a' [97]
State is
Workarea Diff: 141800041
KeySym is a
action first set to [88]
action now set to [88]
Key [action=88][a]
Cannot decode: a
SelfInsert arg[`a']
Workarea event: KEYRELEASE
Work area event KEYPRESS
Key is `a' [97]
Keysym is `a' [97]
State is
Workarea Diff: 250
KeySym is a
action first set to [88]
action now set to [88]
Key [action=88][a]
Cannot decode: a
SelfInsert arg[`a']
Workarea event: KEYRELEASE

You said yesterday that you would use this information to take the hack at 
the bottom of this letter out of the FL_KEYPRESS part of XWorkArea's 
handler. My question to you is how do you propose using this new information? 
I envisage replacing it with something like

handler() {
        // This variable is used to block spurious keyboard input from multiple 
        // FL_KEYPRESS events that are not separated by an FL_KEYRELEASE
        // event.
        static bool input_acceptable = true;
        switch (event) {
                case FL_KEYPRESS:
                        if (!input_acceptable) break;
                        ...
                        workAreaKeyPress(LyXKeySymPtr(xlk), x_key_state(ret_state));
                        input_acceptable = false;
                        break;
                case FL_KEYRELEASE:
                        input_acceptable = true;
                        break;
        }
}

Is that what you had in mind too? If so, wouldn't the most suitable place for 
this be in xforms itself, in the switch handling the raw XEvents 
(do_interaction_step)
        case KeyPress:
                ...
        case KeyRelease:
                ...

Let me know your thoughts on this. (Incidentally, is this a real problem or 
an imaginary one? Ie, is this code really needed?)
Angus

Code to go:
        static Time last_time_pressed;
        static unsigned int last_key_pressed;
        static unsigned int last_state_pressed;
        lyxerr[Debug::KEY] << "Workarea Diff: "
                           << xke->time - last_time_pressed
                           << endl;
        if (xke->time - last_time_pressed < 25 // Should be tunable?
            && ret_state == last_state_pressed
            && xke->keycode == last_key_pressed) {
                lyxerr[Debug::KEY]
                        << "Workarea: Purging X events." << endl;

                if (XEventsQueued(fl_get_display(), QueuedAlready) > 0)
                        waitForX(true);
                // This purge make f.ex. scrolling stop immediately when
                // releasing the PageDown button. The question is if
                // this purging of XEvents can cause any harm...
                // after some testing I can see no problems, but
                // I'd like other reports too. (Lgb)
                break;
        }
        last_time_pressed = xke->time;
        last_key_pressed = xke->keycode;
        last_state_pressed = ret_state;
--- xforms-1.0-release/lib/forms.c	Tue Nov 19 21:06:43 2002
+++ xforms-1.0-release-modified/lib/./forms.c	Tue Dec 10 09:58:44 2002
@@ -1218,7 +1218,8 @@
 }
 
 static void
-fl_keyboard(FL_FORM * form, int key, FL_Coord x, FL_Coord y, void *xev)
+fl_keyboard(int event,
+	    FL_FORM * form, int key, FL_Coord x, FL_Coord y, void *xev)
 {
     FL_OBJECT *obj, *obj1, *special;
 
@@ -1239,7 +1240,7 @@
     if (obj1 && obj1 != special)
 	special = fl_mouseobj;
 
-    if (form->focusobj)
+    if (event == FL_KEYPRESS && form->focusobj)
     {
 	FL_OBJECT *focusobj = form->focusobj;
 
@@ -1294,11 +1295,11 @@
 
     /* space is an exception for browser */
     if ((key > 255 || key == ' ') && (special->wantkey & FL_KEY_SPECIAL))
-	fl_handle_object(special, FL_KEYBOARD, x, y, key, xev);
+	fl_handle_object(special, event, x, y, key, xev);
     else if (key < 255 && (special->wantkey & FL_KEY_NORMAL))
-	fl_handle_object(special, FL_KEYBOARD, x, y, key, xev);
+	fl_handle_object(special, event, x, y, key, xev);
     else if (special->wantkey == FL_KEY_ALL)
-	fl_handle_object(special, FL_KEYBOARD, x, y, key, xev);
+	fl_handle_object(special, event, x, y, key, xev);
 
 #if FL_DEBUG >= ML_INFO
     M_info("KeyBoard", "(%d %d)pushing %d to %s\n", x, y, key, special->label);
@@ -1424,8 +1425,9 @@
 	fl_pushobj = NULL;
 	fl_handle_object(obj, FL_RELEASE, xx, yy, key, xev);
 	break;
-    case FL_KEYBOARD:		/* A key was pressed */
-	fl_keyboard(form, key, xx, yy, xev);
+    case FL_KEYPRESS:		/* A key was pressed */
+    case FL_KEYRELEASE:		/* A key was released */
+	fl_keyboard(event, form, key, xx, yy, xev);
 	break;
     case FL_STEP:		/* A simple step */
 	obj1 = fl_find_first(form, FL_FIND_AUTOMATIC, 0, 0);
@@ -1555,7 +1557,20 @@
 	    }
 
 	} else {
+
+	    /* Always pass on FL_KEYRELEASE, even though keysym is almost
+	       always null */
+	    if (formevent == FL_KEYRELEASE) {
+		fl_handle_form(keyform, formevent, keysym, xev);
+		return;
+	    }
 	    
+	    /* keysym == 0 during multi-byte char composition.
+	     Eg, I've typed Multi_key-a but not yet ' to give á */
+
+	    if (!keysym)
+		return;
+
 	    /* ignore modifier keys as they don't cause action and are
 	       taken care of by the lookupstring routine */
 	    

Reply via email to