On Thursday 12 December 2002 2:43 pm, Jean-Marc Lasgouttes wrote:
> >>>>> "Angus" == Angus Leeming <[EMAIL PROTECTED]> writes:
>
> Angus> Yes, I was thinking about this myself last night. I think
> Angus> do_keyboard should be like this:
>
> Yes, this seems fine. Did you try it? If you have a patch handy, I
> would appreciate it. I'll try it here, and send it to Rod.
>
> Angus> Have I got the right view of where we should head with this?
>
> I _think_. But you have a much clearer idea than I have of the events
> flow.
>
> JMarc
Good. I'm glad that the world I describe at least sounds attractive.
I attach 2 patches. Both can be applied by moving them into ${XFORMS_SRC}/lib
and typing
patch -p2 < ${PATCH}
* smallpatch.diff is the minimal patch to get compose support working on a
solaris box (I hope ;-)
* bigpatch.diff does the whole schmooze.
Internally, xforms is a nasty mess when it comes to handling key press
events. Sometimes it passes keysym to fl_handle_form (the next step in the
event loop). Sometimes it passes the keybuf_return string from
fl_XLookupString, on a char-by-char basis.
To me that is just plain broken, so I've tried to get it to work using keysym
only. The result is much cleaner and more transparent code (I believe ;-) As
a side effect, this patch also silently swallows key events for partially
composed multi-byte strings and dispatches KeyRelease events to the form
handler.
As an aside, I also believe that this could be made to work with cghan's
stuff because every composed char has a unique keysym. (Am I right here?)
That means fl_XLookupString could be modified to work with far eastern
languages and everything else will continue to work. Note that this thought
is pure speculation ;-)
The disadvantage of cleaning up messy code is, of course, that the reworking
isn't trivial. Nonetheless, I think I've gone and done the whole thing.
I've tested it pretty thoroughly and have found no difference to the current
behaviour of xforms internal FL_TEXTBOX and FL_INPUT widgets (the only ones
accepting keyboard input).
One slight possible change is in the use of the cursor keys to navigate a
form. I used the return state from the object handler to tell me whether the
key had been acted upon. I felt that this resulted in much cleaner code than
the present mess and the behaviour appears to be unchanged. I am not
absolutely, 100% convinced about this however.
None of the changes I've made affect the behaviour of LyX's workarea widget.
That's because it doesn't trust the keysym passed to it on a key press event
and generates its own from the raw XEvent.
Having said that, one further advantage of this clean-up to the xforms source
is that XWorkArea's event handler CAN be simplified considerably because the
keysym passed to it is now trustworthy. I have appended the modified code
snippet below.
Please feel free to try out bigpatch.diff. I've come to the opinion that the
only real testing place for this stuff is lyxcvs. I think that we should set
up an xforms tree in the lyx cvs repository and use it in the 1.4 cycle. It
appears that Steve Lamont is happy to accept patches but has little time to
devote to xforms. I'm sure he would be happier to accept big patches like
this if we could tell him they'd been tested hard.
I'm going to cc the lyx-devel list with this. If I can find anyone willing to
test this stuff out, I'll be very happy indeed.
As ever your opinions are solicited.
Best regards,
Angus
(a snippet of my modified XWorkArea.C):
case FL_KEYPRESS:
{
if (!ev)
break;
XKeyEvent * xke = reinterpret_cast<XKeyEvent *>(ev);
lyxerr[Debug::KEY] << "Workarea: key event time diff: "
<< xke->time - time_key_released
<< endl;
if (xke->time - time_key_released < 25 // Should be tunable?
&& xke->keycode == last_key_pressed
&& xke->state == last_state_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;
}
unsigned int const ret_state = xke->state;
debug_key_event(event, key, ret_state);
XLyXKeySym * xlk = new XLyXKeySym;
xlk->initFromKeySym(key);
workAreaKeyPress(LyXKeySymPtr(xlk), x_key_state(ret_state));
}
break;
case FL_KEYRELEASE:
if (!ev)
break;
debug_key_event(event, key, key, ev->xkey.state);
time_key_released = ev->xkey.time;
last_key_pressed = ev->xkey.keycode;
last_state_pressed = ev->xkey.state;
break;
diff -p -N -r -U 4 -X excl.tmp xforms-1.0-release/lib/flinternal.h xforms-1.0-release-modified/lib/flinternal.h
--- xforms-1.0-release/lib/flinternal.h Mon May 20 20:38:39 2002
+++ xforms-1.0-release-modified/lib/flinternal.h Thu Dec 12 16:53:52 2002
@@ -489,8 +489,9 @@ extern int fl_handle_event_callbacks(XEv
#define IsDown(k) (k==XK_Down || k==XK_KP_Down)
#define IsEnd(k) (k==XK_End || k==XK_KP_End)
#define IsPageDown(k) (k==XK_Next || k==XK_Page_Down || k==XK_KP_Page_Down)
#define IsPageUp(k) (k==XK_Prior || k==XK_Page_Up || k==XK_KP_Page_Up)
+#define IsReturn(k) (k==XK_Return || k==XK_KP_Enter)
#else
#define IsHome(k) (k==XK_Home || k== XK_Begin)
#define IsLeft(k) (k==XK_Left)
#define IsRight(k) (k==XK_Right)
@@ -498,8 +499,9 @@ extern int fl_handle_event_callbacks(XEv
#define IsUp(k) (k==XK_Up)
#define IsEnd(k) (k==XK_End)
#define IsPageDown(k) (k==XK_Next)
#define IsPageUp(k) (k==XK_Prior)
+#define IsReturn(k) (k==XK_Return)
#endif
#define FL_HALFPAGE_UP 0x10000000
#define FL_HALFPAGE_DOWN 0x20000000
@@ -512,8 +514,17 @@ extern int fl_handle_event_callbacks(XEv
#define IsNLinesUp(k) ((k)==FL_NLINES_UP)
#define IsNLinesDown(k) ((k)==FL_NLINES_DOWN)
#define Is1LineUp(k) ((k)==FL_1LINE_UP)
#define Is1LineDown(k) ((k)==FL_1LINE_DOWN)
+
+#define IsFLCursorKey(k) (IsHome(k) || IsLeft(k) || \
+ IsRight(k) || IsUp(k) || \
+ IsDown(k) || IsEnd(k) || \
+ IsPageDown(k) || IsPageUp(k) || \
+ IsReturn(k) || IsHalfPageUp(k) || \
+ IsHalfPageDown(k) || IsNLinesUp(k) || \
+ IsNLinesDown(k) || Is1LineUp(k) || \
+ Is1LineDown(k))
extern int fl_convert_shortcut(const char *, long *);
extern int fl_get_underline_pos(const char *, const char *);
diff -p -N -r -U 4 -X excl.tmp xforms-1.0-release/lib/flresource.c xforms-1.0-release-modified/lib/flresource.c
--- xforms-1.0-release/lib/flresource.c Sat Jun 1 23:09:37 2002
+++ xforms-1.0-release-modified/lib/flresource.c Wed Dec 11 12:18:28 2002
@@ -959,9 +959,17 @@ fl_initialize(int *na, char *arg[], cons
int style = XIMPreeditNothing|XIMStatusNothing;
fl_context->xic = XCreateIC(fl_context->xim,
XNInputStyle, style,
0);
- }
+
+ /* Clean-up on failure */
+ if (!fl_context->xic) {
+ M_err("fl_initialize", "Could not create an input context");
+ XCloseIM (fl_context->xim);
+ fl_context->xim;
+ }
+ } else
+ M_err("fl_initialize", "Could not create an input method");
}
#endif
fl_default_xswa();
fl_init_stipples();
diff -p -N -r -U 4 -X excl.tmp xforms-1.0-release/lib/forms.c xforms-1.0-release-modified/lib/forms.c
--- xforms-1.0-release/lib/forms.c Thu Dec 12 13:03:41 2002
+++ xforms-1.0-release-modified/lib/forms.c Thu Dec 12 17:00:37 2002
@@ -1217,9 +1217,10 @@ fl_do_shortcut(FL_FORM * form, int key,
return ret;
}
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;
/* always check shortcut first */
@@ -1227,9 +1228,9 @@ fl_keyboard(FL_FORM * form, int key, FL_
return;
/* focus policy is done as follows: Input object has the highiest
priority. Next is the object that wants special keys which is followed
- by mouseobj that has the lowest. Focusobj == FL_INPUT OBJ */
+ by mouseobj that has the lowest. */
special = fl_find_first(form, FL_FIND_KEYSPECIAL, 0, 0);
obj1 = special ? fl_find_object(special->next, FL_FIND_KEYSPECIAL, 0, 0) : 0;
@@ -1238,35 +1239,32 @@ fl_keyboard(FL_FORM * form, int key, FL_
if (obj1 && obj1 != special)
special = fl_mouseobj;
- if (form->focusobj)
+ /* Focusobj == FL_INPUT OBJ */
+ if (event == FL_KEYPRESS && form->focusobj)
{
FL_OBJECT *focusobj = form->focusobj;
+ int special_key = 0;
- /* handle special keys first */
- if (key > 255)
- {
- if (IsLeft(key) || IsRight(key) || IsHome(key) || IsEnd(key))
- fl_handle_object(focusobj, FL_KEYBOARD, x, y, key, xev);
- else if ((IsUp(key) || IsDown(key) ||
- IsPageUp(key) || IsPageDown(key)) &&
- (focusobj->wantkey & FL_KEY_TAB))
- fl_handle_object(focusobj, FL_KEYBOARD, x, y, key, xev);
- else if (special && (special->wantkey & FL_KEY_SPECIAL))
- {
- /* moving the cursor in input field that does not have focus
- looks weird */
- if (special->objclass != FL_INPUT)
- fl_handle_object(special, FL_KEYBOARD, x, y, key, xev);
+ /* Handle positioning keys first.
+ Let the FL_OBJECT tell us whether it has done anything. */
+ if (IsFLCursorKey(key)) {
+ if (fl_handle_object_direct(focusobj, event, x, y, key, xev)) {
+ fl_object_qenter(focusobj);
+ return;
+ }
+
+ if (special && (special->wantkey & FL_KEY_SPECIAL) &&
+ special->objclass != FL_INPUT &&
+ fl_handle_object_direct(special, event, x, y, key, xev)) {
+ fl_object_qenter(special);
+ return;
}
- else if (key == XK_BackSpace || key == XK_Delete)
- fl_handle_object(focusobj, FL_KEYBOARD, x, y, key, xev);
- return;
}
- /* dispatch tab & return switches focus if not FL_KEY_TAB */
- if ((key == 9 || key == 13) && !(focusobj->wantkey & FL_KEY_TAB))
+ /* Dispatch tab & return switches focus if not FL_KEY_TAB */
+ if ((IsTab(key) || IsReturn(key)) && !(focusobj->wantkey & FL_KEY_TAB))
{
if ((((XKeyEvent *) xev)->state & fl_context->navigate_mask))
{
if (focusobj == fl_find_first(form, FL_FIND_INPUT, 0, 0))
@@ -1283,9 +1281,9 @@ fl_keyboard(FL_FORM * form, int key, FL_
fl_handle_object(focusobj, FL_UNFOCUS, x, y, 0, xev);
fl_handle_object(obj, FL_FOCUS, x, y, 0, xev);
}
else if (focusobj->wantkey != FL_KEY_SPECIAL)
- fl_handle_object(focusobj, FL_KEYBOARD, x, y, key, xev);
+ fl_handle_object(focusobj, event, x, y, key, xev);
return;
}
/* keyboard input is not wanted */
@@ -1293,13 +1291,13 @@ fl_keyboard(FL_FORM * form, int key, FL_
return;
/* 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);
#endif
@@ -1423,10 +1421,11 @@ fl_handle_form(FL_FORM * form, int event
obj = fl_pushobj;
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);
if (obj1 != NULL)
@@ -1510,20 +1509,23 @@ hack_test(void)
/* formevent is either FL_KEYPRESS or FL_KEYRELEASE */
static void
do_keyboard(XEvent * xev, int formevent)
{
-
Window win;
int kbuflen;
+ unsigned char keybuf[227];
+ KeySym keysym = 0;
+ static KeySym last_pressed_keysym;
+ unsigned char * ch;
- /* before doing anything, save the current modifier key for the handlers */
+ /* Before doing anything, save the current modifier key for the handlers. */
win = xev->xkey.window;
fl_keymask = xev->xkey.state;
if (win && (!keyform || !fl_is_good_form(keyform)))
keyform = fl_win_to_form(win);
- /* switch keyboard input only if different top-level form */
+ /* Switch keyboard input only if different top-level form. */
if (keyform && keyform->window != win)
{
M_warn("KeyEvent", "pointer/keybd focus differ");
if ((keyform->child && keyform->child->window == win) ||
@@ -1532,69 +1534,52 @@ do_keyboard(XEvent * xev, int formevent)
else
keyform = fl_win_to_form(win);
}
- if ( keyform ) {
-
- KeySym keysym = 0;
- unsigned char keybuf[227];
+ if (!keyform)
+ return;
- kbuflen = fl_XLookupString((XKeyEvent *) xev, (char *) keybuf,
- sizeof(keybuf), &keysym);
-
- if (kbuflen < 0) {
-
- if ( kbuflen != INT_MIN ) {
-
- /* buffer overflow, should not happen */
- M_err("DoKeyBoard", "keyboad buffer overflow ?");
-
- } else {
+ /* Note that the behaviour of XmbLookupString is undefined on a KeyRelease
+ event, so we do not attempt to call fl_XLookupString.
+ Rather, we store the keysym of the previous FL_KEYPRESS event
+ and dispatch that. */
+ if (formevent == FL_KEYRELEASE) {
+ fl_handle_form(keyform, FL_KEYRELEASE, last_pressed_keysym, xev);
+ return;
+ }
- M_err("DoKeyBoard", "fl_XLookupString failed ?");
-
- }
+ kbuflen = fl_XLookupString((XKeyEvent *) xev, (char *) keybuf,
+ sizeof(keybuf), &keysym);
+ if (kbuflen < 0) {
+ if ( kbuflen != INT_MIN ) {
+ /* buffer overflow, should not happen */
+ M_err("DoKeyBoard", "keyboad buffer overflow ?");
} else {
-
- /* ignore modifier keys as they don't cause action and are
- taken care of by the lookupstring routine */
-
- if (IsModifierKey(keysym))
- ;
- else if (IsTab(keysym)) {
-
- /* fake a tab key. */
- /* some system shift+tab does not generate tab */
-
- fl_handle_form(keyform, formevent, 9, xev);
-
- }
-#if (FL_DEBUG >= ML_DEBUG )
- else if (keysym == XK_F10) /* && controlkey_down(fl_keymask)) */
- hack_test();
-#endif
-
- /* pass all keys to the handler */
- else if (IsCursorKey(keysym) || kbuflen == 0)
- fl_handle_form(keyform, formevent, keysym, xev);
- else {
-
- unsigned char *ch;
-
- /* all regular keys, including mapped strings */
+ M_err("DoKeyBoard", "fl_XLookupString failed ?");
+ }
+ return;
+ }
- for (ch = keybuf; ch < (keybuf + kbuflen) && keyform; ch++)
- fl_handle_form(keyform, formevent, *ch, xev);
+ /* keysym == NoSymbol during multi-byte char composition.
+ Eg, I've typed Multi_key-a but not yet ' to give á
+ Silently swallow these partial-compositions. */
+ if (keysym == NoSymbol)
+ return;
- }
-
- }
+ /* Ignore modifier keys as they don't cause action and are
+ taken care of by the LookupString routine. */
+ if (IsModifierKey(keysym))
+ return;
- }
+ /* Dispatch the key event */
+ fl_handle_form(keyform, formevent, keysym, xev);
+ /* Record the keysym for the next KEYRELEASE event. */
+ last_pressed_keysym = keysym;
}
+
FL_FORM_ATCLOSE
fl_set_form_atclose(FL_FORM * form, FL_FORM_ATCLOSE fmclose, void *data)
{
FL_FORM_ATCLOSE old = form->close_callback;
@@ -2785,9 +2770,14 @@ static int
fl_XLookupString(XKeyEvent * xkey, char *buf, int buflen, KeySym * ks)
{
int len = INT_MIN;
- if (!fl_context->xic)
+ /* Note that the behaviour of XmbLookupString is undefined on a KeyRelease
+ event. Safest therefore to bail out in such a situation. */
+ if (xkey->type != KeyPress)
+ return 0;
+
+ if (!fl_context->xic)
{
len = XLookupString(xkey, buf, buflen, ks, 0);
}
else
@@ -2799,10 +2789,9 @@ fl_XLookupString(XKeyEvent * xkey, char
*ks = NoSymbol;
return 0;
}
- len =
- XmbLookupString(fl_context->xic, xkey, buf, buflen, ks, &status);
+ len = XmbLookupString(fl_context->xic, xkey, buf, buflen, ks, &status);
switch (status)
{
case XBufferOverflow:
diff -p -N -r -U 4 -X excl.tmp xforms-1.0-release/lib/input.c xforms-1.0-release-modified/lib/input.c
--- xforms-1.0-release/lib/input.c Mon Jun 3 18:57:29 2002
+++ xforms-1.0-release-modified/lib/input.c Thu Dec 12 16:47:20 2002
@@ -704,9 +704,9 @@ handle_edit(FL_OBJECT * ob, int key, int
SPEC *sp = ob->spec;
int ret = 1, i, j;
int prev = 1;
- if (key == kmap.del_prev_char || key == kmap.backspace)
+ if (key == XK_BackSpace)
{
prev = -1;
if (sp->endrange >= 0)
delete_piece(ob, sp->beginrange, sp->endrange - 1);
@@ -716,9 +716,9 @@ handle_edit(FL_OBJECT * ob, int key, int
}
else
ret = 0;
}
- else if (key == kmap.del_next_char)
+ else if (key == XK_Delete)
{
if (sp->endrange >= 0)
delete_piece(ob, sp->beginrange, sp->endrange - 1);
else if (sp->position < slen)
@@ -827,25 +827,31 @@ handle_key(FL_OBJECT * ob, int key, unsi
int oldl = sp->lines;
int oldx = sp->xoffset;
int oldmax = sp->max_pixels;
+ if ((ob->type != FL_MULTILINE_INPUT) &&
+ (IsUp(key) || IsDown(key) || IsPageUp(key) || IsPageDown(key))) {
+ /* Don't want to handle this key. */
+ return 0;
+ }
+
/* Extend field size if required */
slen = strlen(sp->str);
if (sp->size == slen + 1)
{
sp->size += 64;
sp->str = (char *) fl_realloc(sp->str, sp->size);
}
- if (ob->type == FL_MULTILINE_INPUT && key == 13)
+ if (ob->type == FL_MULTILINE_INPUT && IsReturn(key))
key = NL;
/* Compute starting position of current line */
startpos = sp->position;
while (startpos > 0 && sp->str[startpos - 1] != NL)
startpos--;
- if (controlkey_down(kmask) && key > 255)
+ if (controlkey_down(kmask))
key |= FL_CONTROL_MASK;
if (metakey_down(kmask))
key |= METAMASK;
@@ -877,8 +883,14 @@ handle_key(FL_OBJECT * ob, int key, unsi
key = XK_PageDn;
else if (key == kmap.moveto_prev_page)
key = XK_PageUp;
+ /* ditto for the delete keys */
+ if (key == kmap.del_prev_char || key == kmap.backspace)
+ key == XK_BackSpace;
+ else if (key == kmap.del_next_char)
+ key == XK_Delete;
+
if (IsRegular(key)) /* Normal keys or NL */
{
int ok = FL_VALID;
char *tmpbuf = 0;
@@ -1712,9 +1724,9 @@ fl_get_input_cursorpos(FL_OBJECT * ob, i
return sp->position;
}
-#define Control(c) ((c) - 'a' + 1)
+#define Control(c) ((c) | FL_CONTROL_MASK)
#define Meta(c) ((c) | METAMASK)
#define Dump(a) fprintf(stderr,"\t%s: %ld(0x%lx)\n",#a,kmap.a,kmap.a)
diff -p -N -r -U 4 -X excl.tmp xforms-1.0-release/lib/forms.c xforms-1.0-release-modified/lib/forms.c
--- xforms-1.0-release/lib/forms.c Thu Dec 12 13:03:41 2002
+++ xforms-1.0-release-modified/lib/forms.c Thu Dec 12 18:41:37 2002
@@ -1537,8 +1537,16 @@ do_keyboard(XEvent * xev, int formevent)
KeySym keysym = 0;
unsigned char keybuf[227];
+ /* Note that the behaviour of XmbLookupString is undefined on
+ a KeyRelease event, so we do not attempt to call fl_XLookupString.
+ Rather, we just dispatch the event with keysym == 0. */
+ if (formevent == FL_KEYRELEASE) {
+ fl_handle_form(keyform, FL_KEYRELEASE, 0, xev);
+ return;
+ }
+
kbuflen = fl_XLookupString((XKeyEvent *) xev, (char *) keybuf,
sizeof(keybuf), &keysym);
if (kbuflen < 0) {
@@ -2784,8 +2792,13 @@ fl_restore_target(void)
static int
fl_XLookupString(XKeyEvent * xkey, char *buf, int buflen, KeySym * ks)
{
int len = INT_MIN;
+
+ /* Note that the behaviour of XmbLookupString is undefined on a KeyRelease
+ event. Safest therefore to bail out in such a situation. */
+ if (xkey->type != KeyPress)
+ return 0;
if (!fl_context->xic)
{
len = XLookupString(xkey, buf, buflen, ks, 0);
