GTK registers two events for certain key bindings

[Martin Thomas]

When I open the find dialog (Strg F) and click on close it apperas again. When I click another time on close it closes really.

Application: KiCad
Version: (5.1.0-956-gfa7a961d0), debug build
Libraries:
    wxWidgets 3.0.2
    libcurl/7.52.1 GnuTLS/3.5.8 zlib/1.2.8 libidn2/0.16 libpsl/0.17.0 (+libidn2/0.16) libssh2/1.7.0 nghttp2/1.18.1 librtmp/2.3
Platform: Linux 5.1.3-mt x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.2 (wchar_t,wx containers,compatible with 2.8) GTK+ 2.24
    Boost: 1.62.0
    OpenCASCADE Community Edition: 6.8.0
    Curl: 7.52.1
    Compiler: GCC 6.3.0 with C++ ABI 1010

Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_PYTHON3=OFF
    KICAD_SCRIPTING_WXPYTHON=ON
    KICAD_SCRIPTING_WXPYTHON_PHOENIX=OFF
    KICAD_SCRIPTING_ACTION_MENU=OFF
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_USE_OCC=OFF
    KICAD_SPICE=OFF

[Michael Kavanagh (michaelkavanagh)]

I can't reproduce this on macOS.

Application: Pcbnew
Version: (5.1.0-961-gfac3e2d66), debug build
Libraries:
    wxWidgets 3.0.4
    libcurl/7.54.0 LibreSSL/2.6.5 zlib/1.2.11 nghttp2/1.24.1
Platform: Mac OS X (Darwin 18.6.0 x86_64), 64 bit, Little endian, wxMac
Build Info:
    wxWidgets: 3.0.4 (wchar_t,STL containers,compatible with 2.8)
    Boost: 1.69.0
    OpenCASCADE Community Edition: 6.9.1
    Curl: 7.54.0
    Compiler: Clang 10.0.1 with C++ ABI 1002

Build settings:
    KICAD_SCRIPTING=OFF
    KICAD_SCRIPTING_MODULES=OFF
    KICAD_SCRIPTING_PYTHON3=OFF
    KICAD_SCRIPTING_WXPYTHON=OFF
    KICAD_SCRIPTING_WXPYTHON_PHOENIX=OFF
    KICAD_SCRIPTING_ACTION_MENU=OFF
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_USE_OCC=OFF
    KICAD_SPICE=ON

[John Beard (john-j-beard)]

Happens for me on Arch.

If you change the Find hotkey, you still get a dialog for Ctrl+F. So it's presumably a WX hardcoded event somewhere, similar to the F1/Help/Zoom bug.

[John Beard (john-j-beard)]

OK, so it's not that - it's related to the new *_MENU stuff:

You get one event from the Edit CONDITIONAL_MENU, and you get one from the normal tool dispatch bound in setTransitions.

The same happens, with, e.g. the measure tool, you just don't notice it because it ends up in the right state.

@Jeff: I'm not quite sure how the new stuff works yet, so I'm putting it back on the shelf.

Side note: there appears to be a FIND_DIALOG in both SELECTION_TOOL and also in PCB_EDITOR_CONTROL. Are these both needed?

[Jeff Young (jeyjey)]

@John, I think you've already done all the hard work.

I suspect the two events is a red herring: the CONDITIONAL_MENU turns a wxEvent menu event into a TOOL_EVENT which is then dispatched through setTransitions.

I further suspect your side-note is the bug: remove the Go() line (and the function it calls) from PCB_EDITOR_CONTROL and I'll bet everything will be ducky.

I'm happy to fix this up, but you've already done the work so I thought you might want the satisfaction of doing it....

[John Beard (john-j-beard)]

@Jeff: Hmm, just removing Go( &PCB_EDITOR_CONTROL::Find, doesn't seem to cut it, I still get two events, once like this:

#0 0x00007fffee34b0a4 in TOOL_MANAGER::ProcessEvent(TOOL_EVENT const&)
    (this=0x555558d80890, aEvent=...) at /home/john/src/kicad/common/tool/tool_manager.cpp:810
#1 0x00007fffee344239 in TOOL_DISPATCHER::DispatchWxEvent(wxEvent&)
    (this=0x555557946900, aEvent=...)
    at /home/john/src/kicad/common/tool/tool_dispatcher.cpp:447
#2 0x00007fffee3b302a in EDA_DRAW_PANEL_GAL::OnEvent(wxEvent&)
    (this=0x5555570499f0, aEvent=...) at /home/john/src/kicad/common/draw_panel_gal.cpp:447
#3 0x00007ffff74db89e in wxEvtHandler::ProcessEventIfMatchesId(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&) () at /usr/lib/libwx_baseu-3.0.so.0
#4 0x00007ffff74dbc1b in wxEvtHandler::SearchDynamicEventTable(wxEvent&) ()
    at /usr/lib/libwx_baseu-3.0.so.0

And once like this when the first dialog is closed:

#0 0x00007fffee34b0a4 in TOOL_MANAGER::ProcessEvent(TOOL_EVENT const&)
    (this=0x555558d80890, aEvent=...) at /home/john/src/kicad/common/tool/tool_manager.cpp:810
#1 0x00007fffee31af35 in ACTION_MENU::OnMenuEvent(wxMenuEvent&)
    (this=0x55555afe4e40, aEvent=...) at /home/john/src/kicad/common/tool/action_menu.cpp:425
#2 0x00007ffff74db89e in wxEvtHandler::ProcessEventIfMatchesId(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&) () at /usr/lib/libwx_baseu-3.0.so.0
#3 0x00007ffff74dbc1b in wxEvtHandler::SearchDynamicEventTable(wxEvent&) ()

and once like this:

[Jeff Young (jeyjey)]

@John, interesting.

The wxWidgets /should/ be deactivated. I know we disable the window's event handler, but maybe command-keys come in differently? We might have to drag Tom in as I'm no expert on the event handling either. But let me investigate a bit....

[Jeff Young (jeyjey)]

Ugh. I forgot this doesn't reproduce on OSX.

On OSX we're only getting a wxEVT_COMMAND_MENU_SELECTED event. This leads to your second stack trace.

GTK (at least) is also getting either a wxEVT_CHAR or wxEVT_CHAR_HOOK, which leads to your first stack trace. I suspect it's a HOOK event, as that would at least be a defensible position on the behalf of the wxWidgets developers. Could you validate that though before I try to figure out a work-around?

[Jeff Young (jeyjey)]

Our code /should/ only process a wxEVT_CHAR_HOOK if it's a special key (arrows or page up/down). This would suggest GTK must be firing a wxEVT_CHAR and a wxEVT_COMMAND_MENU_SELECTED.

[John Beard (john-j-beard)]

@Jeff: I get the following KICAD_KEY_EVENTs trace when I press Ctrl-F:

16:50:10: Trace: (KICAD_KEY_EVENTS) TOOL_DISPATCHER::DispatchWxEvent Hook CONTROL 308 C--- 0 (U+0000) 65507 0x00000025 ( 319, 374)

16:50:10: Trace: (KICAD_KEY_EVENTS) EDA_DRAW_FRAME::OnCharHook Hook CONTROL 308 C--- 0 (U+0000) 65507 0x00000025 ( 319, 374)

16:50:12: Trace: (KICAD_KEY_EVENTS) TOOL_DISPATCHER::DispatchWxEvent Hook 'F' 70 C--- 70 (U+0046) 102 0x00000029 ( 319, 374)

16:50:12: Trace: (KICAD_KEY_EVENTS) EDA_DRAW_FRAME::OnCharHook Hook 'F' 70 C--- 70 (U+0046) 102 0x00000029 ( 319, 374)

16:50:12: Trace: (KICAD_KEY_EVENTS) TOOL_DISPATCHER::DispatchWxEvent Char Ctrl-F 6 C--- 6 (U+0006) 102 0x00000029 ( 319, 374)

[Jeff Young (jeyjey)]

@John, OK, it looks like it really is a CHAR_HOOK that's getting processed. Put a breakpoint in tool_dispatcher.cpp on line 383. keyIsSpecial() should return false, and we should exit processing. Is that happening correctly?

One other thing: in your comment #9 I assume the 2 16:50:10 events opened the Find dialog, and then the 3 16:50:12 events were generated when you closed it?

[Jeff Young (jeyjey)]

@John, I recently made another change (regarding calling PassEvent()) that may fix this. Could you check it again?

[Martin Thomas (mtlaunchpad)]

I still have to click on Close twice.

Application: Pcbnew
Version: (5.1.0-1030-g4b37c397f), debug build
Libraries:
    wxWidgets 3.0.2
    libcurl/7.52.1 GnuTLS/3.5.8 zlib/1.2.8 libidn2/0.16 libpsl/0.17.0 (+libidn2/0.16) libssh2/1.7.0 nghttp2/1.18.1 librtmp/2.3
Platform: Linux 5.1.3-mt x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.2 (wchar_t,wx containers,compatible with 2.8) GTK+ 2.24
    Boost: 1.62.0
    OpenCASCADE Community Edition: 6.8.0
    Curl: 7.52.1
    Compiler: GCC 6.3.0 with C++ ABI 1010

Build settings:
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_PYTHON3=OFF
    KICAD_SCRIPTING_WXPYTHON=ON
    KICAD_SCRIPTING_WXPYTHON_PHOENIX=OFF
    KICAD_SCRIPTING_ACTION_MENU=OFF
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_USE_OCC=OFF
    KICAD_SPICE=OFF

[Wayne Stambaugh (stambaughw)]

I'm still seeing this issue as well.

Application: Pcbnew
Version: (5.1.0-1028-g5d7739a66), debug build
Libraries:
    wxWidgets 3.0.4
    libcurl/7.64.0 OpenSSL/1.1.1c zlib/1.2.11 libidn2/2.0.5 libpsl/0.20.2 (+libidn2/2.0.5) libssh2/1.8.0 nghttp2/1.36.0 librtmp/2.3
Platform: Linux 4.19.0-5-amd64 x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.4 (wchar_t,wx containers,compatible with 2.8) GTK+ 3.24
    Boost: 1.62.0
    OpenCASCADE Community Edition: 6.9.1
    Curl: 7.64.0
    Compiler: Clang 7.0.1 with C++ ABI 1002

Build settings:
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_PYTHON3=ON
    KICAD_SCRIPTING_WXPYTHON=ON
    KICAD_SCRIPTING_WXPYTHON_PHOENIX=ON
    KICAD_SCRIPTING_ACTION_MENU=ON
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_USE_OCC=OFF
    KICAD_SPICE=ON

[Jeff Young (jeyjey)]

@Wayne, if you have a chance could you check the stuff in comment #10 and post your results? (The bug doesn't reproduce on OSX.)

[Wayne Stambaugh (stambaughw)]

@Jeff, this is odd. I am getting a false indication on the is special key check. What is strange is that there is no character or character hook events between the two instances of the find dialog.

[Jeff Young (jeyjey)]

@Wayne, I suspect both events are logged before the dialog is brought up; the second just waits around in the frame's event loop until the dialog is closed.

Although I'm not sure that explains the 2 second lag in John's trace statements....

[Wayne Stambaugh (stambaughw)]

@Jeff, be careful with the timing of logged events. The log order is guaranteed to be correct but our context switching may cause timing issues unless we forcefully flush the log cache. Log output is written during idle time in the event queue. I don't know how the context switching impacts the idle time event handling. This is well documented in the "Logging in Multi-Threaded Applications" section of the wxWidgets "Logging Overview"[1].

[1]: https://docs.wxwidgets.org/3.0/overview_log.html

[Seth Hillbrand (sethh)]

Hi All-

It appears that all of these are being caused on GTK by the menu shortcuts firing the event as well as the normal event handler.

There are a few ways I can imagine handling this. Preference and/or other suggestions would be appreciated:

1) Put our hotkey handling in an event filter[1]
2) Intercept wxEVT_MENU_OPEN and wxEVT_MENU_CLOSE and populating the menus there but not leaving them.
3) Adding a special character to the hotkey string when displaying in the menu to fool wx into not handling the key. Maybe a unicode blank or similar.
(4) Skip the event handling for hotkeys that map to menu items on GTK.

Personally, I don't like (2) as it feels hack-y. And (3) seems like it would cause more problems than it solves

[1] https://docs.wxwidgets.org/3.0/classwx_event_filter.html

[Jeff Young (jeyjey)]

Hi Seth,

I don’t think double-firing can be the whole story. The OSX menu system automatically fires hotkeys too (which is why we have a pretty egregious hack in there to determine whether it was a real menu event or a hotkey). Speaking of which, we already intercept wxEVT_MENU_OPEN and wxEVT_MENU_CLOSE, both for the afore-mentioned hack and for another wxWidgets work-around where the events aren’t delivered to the ACTION_MENUs.

Maybe on OSX it suppresses the following keycode, and on GTK it doesn’t. I suppose that would explain it, but then why wouldn’t that give everyone’s apps headaches? (And why didn’t we have issues with this before?)

For (1) we’d still need to know if the hotkey had already been handled. (I’m not sure how this would be different from (4).) In either case it might be problematic, and I’m not sure there’s an automatic way to know that.

Cheers,
Jeff.

[KiCad Janitor (kicad-janitor)]

Fixed in revision b87630999928c313d60934b65852e36e8f861696
https://git.launchpad.net/kicad/patch/?id=b87630999928c313d60934b65852e36e8f861696

[Seth Hillbrand (sethh)]

Doh! A little more searching and the answer presented itself. Should be clear but it would be good if a MSW user could test this as well to make sure I didn't break anything.

[Jeff Young (jeyjey)]

@Seth, well done! That turned out to be simple enough.

[Martin Thomas (mtlaunchpad)]

@Seth, I tested it on Windows. I think you've made it! Thank you.