#270 Crash when pressing ESC in an open menu

closed-fixed
nobody
None
5
2009-07-11
2006-10-16
Scott Tringali
No

1) Run lesstif-0.95.0/test/Xm/menushell/test4
2) Click "cascade2" (the bug does not manifest in "cascade1")
3) Press ESC

Result: crashes

Also in 0.94.4.

Discussion

  • Thomas Orgis
    Thomas Orgis
    2009-03-13

    In lesstif 0.95.0, this crash happens because of an unexpected NULL pointer... I verified that by adding a bit of debugging code before that call in _XmMenuExcape:

    RC_MenuButtonPopdown(MGR_ActiveChild(rc), ev, &poppedUp);

    XmRowColumn fileMenu: _XmMenuEscape()
    XmRowColumn fileMenu: _XmMenuEscape()
    XmRowColumn fileMenu: Menu fileMenu (XmMENU_PULLDOWN)
    XmRowColumn fileMenu: Menu fileMenu (XmMENU_PULLDOWN)
    XmRowColumn fileMenu: must be a popup
    XmRowColumn fileMenu: activeChild: (nil)
    Segmentation fault

    MGR_ActiveChild(rc) is NULL

    There is no active child of fileMenu to pop down, because in that case I have been in the edit menu instead.
    Pressing ESC kills nedit for every menu _except_ the file menu... you get the idea already, I'm sure.
    Let's look what happens when opening the file menu (hotkey or mouse, does not matter):

    XmRowColumn fileMenu: MenuFocusIn
    XmRowColumn editMenu: MenuFocusIn
    XmPushButton paste: FocusIn()
    XmRowColumn fileMenu: _XmVirtKeysHandler
    XmRowColumn fileMenu: _XmVirtKeysHandler

    ... and later, on closing with ESC:

    XmRowColumn fileMenu: _XmMenuEscape()
    XmRowColumn fileMenu: _XmMenuEscape()
    XmRowColumn fileMenu: Menu fileMenu (XmMENU_PULLDOWN)
    XmRowColumn fileMenu: Menu fileMenu (XmMENU_PULLDOWN)
    XmRowColumn fileMenu: must be a popup
    XmRowColumn fileMenu: activeChild: (nil)

    Well... that should editMenu, or shouldn't it? Could that be nedit's fault?
    What would cause the fileMenu to get focus when popping up the edit menu?

    Also, I switched to lesstif CVS now (where copy and paste works again, yay!), where the focus on opening a menu is messed up. Have to investigate that... or perhaps it's known?

    Is there a chance to get insigths from the lesstif team about that? I have seen that there have been CVS commits in the past year... so it's not that dead as it looks from the website or the tracker here?

    I really would like to get lesstif working on my GNU/Linux box, with nedit at that.

     
  • Thomas Orgis
    Thomas Orgis
    2009-03-14

    I investigated a bit further and was able to pin this bug to a change happening between releases 0.93.96 and 0.93.97 .
    I don't know what change yet, but the diff is not _that_ large than the one I got from comparing 0.94 to 0.93.95 ...

    A lesstif developer should easily be able to spot the affecting changes, I guess. It is not yet obvious to me.
    I am attaching two log files, obtained using non-production builds and such a command

    bash$ for name in lesstif-0.93.9{6,7}-disable-production
    do
    echo Testing $name
    DEBUG_SOURCES=all LD_LIBRARY_PATH=/data/test/$name/lib nedit
    echo "return value: $?"
    done

    In the nedit window, I just did Alt+H and then ESC (and, if nedit did not crash, Alt+Q to quit the program).

     
  • Paul Gevers
    Paul Gevers
    2009-03-20

    I found which part of that transition causes the problem. It is the change in Traversal.c

    When I build lesstif with the following patch applied (only reversing the change, I think somebody should see what the patch was supposed to fix) I didn't get the crash.

    I hope somebody can improve the original patch to not crash on <ESC>

    Paul

    PATCH:
    Fix crash on <ESC> when in menus
    Reverses patch from lesstif 0.93.96 to 0.93.97
    Fixes Debian bug 356017, Upstream bug 1578451, Ubuntu bug 124573
    Index: lesstif2-0.95.0/lib/Xm-2.1/Traversal.c
    ===================================================================
    --- lesstif2-0.95.0.orig/lib/Xm-2.1/Traversal.c 2009-03-20 21:19:05.000000000 +0100
    +++ lesstif2-0.95.0/lib/Xm-2.1/Traversal.c 2009-03-20 21:20:15.000000000 +0100
    @@ -1807,12 +1807,6 @@

    return fd;
    }
    - else if (XtIsSubclass(wid, xmMenuShellWidgetClass) && MS_FocusData(wid) != 0)
    - {
    - XmMenuShellWidget msh = (XmMenuShellWidget)wid;
    - MS_FocusData(wid)->focus_policy = msh->menu_shell.focus_policy;
    - return (MS_FocusData(wid));
    - }

    DEBUGOUT(_LtDebug(__FILE__, orig,
    "_XmGetFocusData: not LessTif vendorshell subclass\n"));

     
  • Thomas Orgis
    Thomas Orgis
    2009-03-21

    Interesting... That indeed prevents that crash.
    What is still annoying is that for one my lesstif CVS with your patch (well, also without the patch) has issues getting keyboard focus right and more to the point for this bug:

    xpdf crashes nicely when opening the right-click menu and pressing ESC ... observe:

    XmPushButton open: _XmMenuEscape()
    XmPushButton open: _XmMenuEscape()
    XmPushButton open: Menu popupMenu (XmMENU_POPUP)
    XmPushButton open: Menu popupMenu (XmMENU_POPUP)
    XmPushButton open: must be a popup
    Segmentation fault

    Quite similar, eh?
    So we don't have the "real" cause yet, I guess - or at least only part of it.
    This business could well be related to the focus issues... and also the strange behaviour with being able to navigate in a menu with cursor keys but enter key doing nothing.

     
  • Paul Gevers
    Paul Gevers
    2009-03-21

    Just wondering, does this last issue your mention also starts appearing with the transition from 0.93.96 to 0.93.97? There were two other patches related to focusing in the same transition, in RowColumn.c and MenuShell.c I will try to test tomorrow, but find attached the reverse patches (NOTHING TESTED YET), but mind the comments. Apparently the people doing the patches weren't completely sure.

    Paul

    Index: lesstif2-0.95.0/lib/Xm-2.1/MenuShell.c

    --- lesstif2-0.95.0.orig/lib/Xm-2.1/MenuShell.c 2009-03-20 20:22:41.000000000 +0100
    +++ lesstif2-0.95.0/lib/Xm-2.1/MenuShell.c 2009-03-20 21:17:29.000000000 +0100
    @@ -742,43 +742,6 @@
    "MenuShellPopdownDone - found RC %s, posted: %s\n",
    XtName(rc), RC_PopupPosted(rc) ? XtName(RC_PopupPosted(rc)) : "NULL"));

    -#if 1
    - /*
    - * Don't use an event that's within the multi click time.
    - * Hopefully this is an event which we can discard :-)
    - * If we get bug reports about things not disappearing when
    - * the user is fast, this is the reason.
    - * Danny 22/04/1998
    - *
    - * I also have a bad feeling about the test above. Is it meant to do
    - * what I changed it to below ?
    - */
    - /* Danny had this ifdef'd out, but it seems to be exactly what
    - * Motif does, so I put it back in. In Motif, a quick click/release
    - * causes the menu to "stick" open.
    - * -dwilliss 28-Sep-04
    - */
    - if (_XmMenuGetInPMMode(w) && event && event->type == ButtonRelease &&
    - event->xbutton.time <= state->RC_ButtonEventStatus.time
    - + XtGetMultiClickTime(XtDisplay(w)))
    - {
    - if (RC_Type(rc) != XmMENU_OPTION)
    - {
    - /* Not that this _shouldn't_ be done for option menus, but I'm
    - * not sure if this is the correct way to do it for option menus,
    - * so in that case, leave them alone for now
    - * -- dwilliss 28-Sep-04
    - */
    - _XmMenuFocus(rc, XmMENU_FOCUS_SAVE, CurrentTime);
    - RCClass_MenuTraverse(rc, XmTRAVERSE_HOME);
    - XAllowEvents(XtDisplay(rc), SyncPointer, event->xbutton.time);
    - }
    - DEBUGOUT(_LtDebug(__FILE__, w, "MenuShellPopdownDone(): exiting b/c user too fast\n"));
    - return;
    - }
    -#endif
    -
    -
    _XmGetActiveTopLevelMenu(rc, &toplevelrc);

    if (!toplevelrc)

    Index: lesstif2-0.95.0/lib/Xm-2.1/RowColumn.c

    --- lesstif2-0.95.0.orig/lib/Xm-2.1/RowColumn.c 2009-03-20 21:21:50.000000000 +0100
    +++ lesstif2-0.95.0/lib/Xm-2.1/RowColumn.c 2009-03-20 21:26:09.000000000 +0100
    @@ -1047,7 +1047,6 @@
    old_handler = XtAppSetWarningHandler(
    XtWidgetToApplicationContext(realpar),
    xtWarnCB);
    - XtUngrabButton(realpar, RC_PostButton(w), RC_PostModifiers(w));
    XtAppSetWarningHandler(XtWidgetToApplicationContext(realpar),
    old_handler);
    }
    @@ -2478,12 +2477,6 @@

    DEBUGOUT(_LtDebug2(__FILE__, new_w, RC_MemWidget(new_w),
    "_XmFixOptionMenu: Set RC_MemWidget\n"));
    - /* 1-sep-04 -- dwilliss - don't ever let MemWidget get set to, for example, a seperator
    - because we always assume it's a label of some kind */
    - if (RC_MemWidget(new_w) != NULL && !XmIsLabel(RC_MemWidget(new_w)) && !XmIsLabelGadget(RC_MemWidget(new_w)))
    - {
    - RC_MemWidget(new_w) = NULL;
    - }
    }
    }

     
  • Thomas Orgis
    Thomas Orgis
    2009-03-22

    No, those additional reverts don't help.
    The keyboard handling in the menu is also broken in 0.93.94 .
    The xpdf crash issue is not there in that lesstif version simply because one cannot get the menu to stay open.

    paulgevers: Do you intend to spend time on hacking lesstif?
    I tend to answer that question for myself with "no". There seems to be a too lively population of nasty bugs in aging software... with few (any?) people left understanding what is going in.

     
  • Thomas Orgis
    Thomas Orgis
    2009-03-23

    Sorry, I messed up the patching locally... yes, the xpdf crash is fixed with the MenuShell revert, because the menu does not stay open anymore.
    The focus issue does prevail... On opening the search dialog with Ctrl+F, nedit sometimes looses keyboard input (in that said dialog and also the main window, while Ctrl+F still works).
    So we're a step further, but still not there... that keyboard issue is _really_ annoying.

     
  • Another update: The keyboard focus issue is also there with lesstif 0.93.94 .
    Checked also with KDE 3.5 instead of my normal window manager, fluxbox.

     
  • Thomas Orgis
    Thomas Orgis
    2009-03-24

    Well, I'm more and more misusing this bug as a master bug for "need some working lesstif on linux"...
    Reporting on the keyboard focus issue:
    It does not happen with lesstif-0.93.18 . It does happen with 0.93.36 .
    One could try CVS revisions in between... anyhow: There is an acceleration in shortcut response in that version jump.
    The earlier lesstif needs the Ctrl+F to be pressed for a longer time span to take effect... but for that you get always keyboard input in nedit.

     
  • Thomas Orgis
    Thomas Orgis
    2009-03-25

    This seems to be the relevant change for the Ctrl+F reaction speed + keyboard focus.
    With the patch, lesstif-0.93.36 behaves like 0.93.18 ... applied to current cvs but not tested yet.

    diff -ruN lesstif-cvs-20090312/lib/Xm-2.1/Manager.c lesstif-cvs-20090312-shortcut-focus/lib/Xm-2.1/Manager.c
    --- lesstif-cvs-20090312/lib/Xm-2.1/Manager.c 2007-09-12 22:29:35.000000000 +0200
    +++ lesstif-cvs-20090312-shortcut-focus/lib/Xm-2.1/Manager.c 2009-03-25 01:24:19.000000000 +0100
    @@ -1930,7 +1930,7 @@
    item->key, item->keysym, item->modifiers,
    item->needGrab ? "Grabbing" : "Not Grabbing",
    item->isMnemonic ? "mnemonic" : "not mnemonic"));
    -#if 1
    +#if 0 /* effect of this block in nedit: fast shortcut reaction but unreliable keyboard input focus */
    switch (item->eventType) {
    case KeyPress:
    mask = KeyPressMask;
    @@ -1950,7 +1950,7 @@
    #endif
    mask = 0;
    }
    -#else
    +#else /* This code looks horribly wrong and makes slow shortcut reaction but at least is reliable (in nedit...). */
    mask = KeyPressMask;
    mask = item->eventType; /* This must be wrong but the alternatives crash nedit. FIX ME */
    #endif

     
  • @sobukus: yes, I intend to put some effort in getting things to work. But be prepared for a slow pace because I use it to learn some programming (finding bugs and regressions, no new features). And have other projects as well.

     
  • Danny Backx
    Danny Backx
    2009-03-25

    Those who want CVS update access to the LessTif sources should send me private E-mail.

    Danny

     
  • Thomas Orgis
    Thomas Orgis
    2009-03-29

    Hm, so we can expect some fixing in CVS (largely reverts of changes with side-effects)?
    Perhaps even a stabilizing release (even "just" for all the fixes accumulated in CVS anyway)?

     
  • Paul Gevers
    Paul Gevers
    2009-03-31

    @sobukus With respect to a stabilized release: I have asked for commit rights, I will see what I can do.

     
  • Paul Gevers
    Paul Gevers
    2009-04-29

    I committed the reversal of the changes in Traversal.c

    At least lesstif should not crash anymore on <ESC>. I will keep on working on lesstif, and hopefully I will be able to figure out what the intension of the original patch was.

     
  • Paul Gevers
    Paul Gevers
    2009-07-11

    The fix was included in the latest release (0.95.2).

     
  • Paul Gevers
    Paul Gevers
    2009-07-11

    • status: open --> closed-fixed