On master branch, Cmd+Q doesn't quit app, but emits warning instead

[TonalidadeHidrica]

I recently switched from v0.7.0 to master. I recognized that Cmd+Q does not quit the app anymore. This happens not only in my project but also in the example (at least, flex example). Instead, it emits a warning every time I submit the keystroke, apparently related to OS layer:

cargo run --example flex

2021-04-10 13:37:22.456 flex[74291:7719784] +[NSView doCommandBySelector:]: unrecognized selector sent to class 0x7fff885efa08

This happens only in master. In v0.7.0, I can quit the app with Cmd+Q without any warning.

Environment

• druid master (2c5d73c8426f90d2a1ede9fdd3b5cb7bee57f108)
• macOS Big Sur 11.2.3 (Intel)

I can provide further environmental information if needed.

[TonalidadeHidrica]

I ran git bisect and found the (possible) cause of the commit: 8e09a6d5847f83b194dced8c5b9cfa3381b80371.

# bad: [2c5d73c8426f90d2a1ede9fdd3b5cb7bee57f108] Add missing derived lens documentation (#1696)
# good: [ed4f9ef0e763d8396ef2fb7facd8ea4ba541c41e] Prepare for 0.7.0
git bisect start '2c5d73c' 'v0.7.0'
# good: [3b902a993623d5ed4e96693d4efbc0ece1fd2333] Fixup changelog links
git bisect good 3b902a993623d5ed4e96693d4efbc0ece1fd2333
# good: [9c361872050cac1aa055bce2e908ad23adc9341d] Relax constraints on who can change focus
git bisect good 9c361872050cac1aa055bce2e908ad23adc9341d
# bad: [0e88d99692ff0163cb0f0838e915b29a25732a77] Fix another clippy lint
git bisect bad 0e88d99692ff0163cb0f0838e915b29a25732a77
# good: [a281a814d1bc61a5169491991367ecb53fd0f383] Unify druid & druid-shell selection types
git bisect good a281a814d1bc61a5169491991367ecb53fd0f383
# bad: [75e83ae5a73fb1da2407eb157580145d833e1caf] updating changelog for on_added WidgetExt method (#1668)
git bisect bad 75e83ae5a73fb1da2407eb157580145d833e1caf
# good: [dde1587267ef0f0f8f4275276aef9288397f8678] Unify druid & druid-shell Movement types
git bisect good dde1587267ef0f0f8f4275276aef9288397f8678
# bad: [8e09a6d5847f83b194dced8c5b9cfa3381b80371] Menus that use data (#1625)
git bisect bad 8e09a6d5847f83b194dced8c5b9cfa3381b80371
# good: [39f4855b0e63ceb8a4c054a4f6ef218145f92ec3] Fix word selection logic
git bisect good 39f4855b0e63ceb8a4c054a4f6ef218145f92ec3
# first bad commit: [8e09a6d5847f83b194dced8c5b9cfa3381b80371] Menus that use data (#1625)

[TonalidadeHidrica]

Actually, the issue is not limited to Cmd+Q, but also Cmd+W which is expected to close the window; moreover, any keystroke with the modifier either Cmd+ or Ctrl+ causes the warning.

[raphlinus]

Repros here too.

[cmyr]

This works as expected for me on 10.15.7, so I think this is a change in macOS?

[lord]

cmd-q in the druid-shell shello example works for me on big sur, until I delete the menubar code — then it stops working. Quit is not actually one of the menu options in that example, which makes this even more weird?

The doCommandBySelector selector being passed in the error case is noop, which I guess is a little unexpected kind of makes sense given how, as noted above, it's emitted for any unbound keyboard shortcut

[cmyr]

Quit isn't an option, but cmd+Q is bound to the 'exit' menu item.

[JarrettBillingsley]

I've only been using druid for a few weeks and thought it was just Like This. I noticed I had to have a menu in order for Cmd+Q to work. Was that not how it used to be?

[cmyr]

Some behaviour has changed.

in 0.7.0, the calc example, which does not get an explicit menu created, still has the default 'application' menu. This is provided by the platform via some mechanism I do not understand. This menu includes the 'Quit' menu item.

In master, calc does not have this menu. The most likely culprit is the menu rework, but I haven't investigated further.

[TonalidadeHidrica]

Just referencing a possibly related issue: andlabs/libui#501 . In this libui issue the "Quit" menu is found in the menu bar, while this druid issue it isn't.

[cmyr]

I've been running into this a bunch lately, and I'm motivated to dig into it soon.

[kud1ing]

Ok, this comes from here:

[cmyr]

thanks.

[kud1ing]

cmd looks problematic (if it is not a debugging artefact):

[cmyr]

yea I'm not sure how I would interpret that. I think maybe it's saying that the superclass in this instance is nil?

edit: no, nevermind. In any case, I'm not sure what's going in.

[kud1ing]

And the full call stack:

do_command_by_selector text_input.rs:268
-[NSTextInputContext(NSInputContext_WithCompletion) doCommandBySelector:completionHandler:] 0x00007fff2317b6e6
-[NSKeyBindingManager(NSKeyBindingManager_MultiClients) interpretEventAsCommand:forClient:] 0x00007fff23098cdf
__84-[NSTextInputContext _handleEvent:options:allowingSyntheticEvent:completionHandler:]_block_invoke_5 0x00007fff230a2172
__84-[NSTextInputContext _handleEvent:options:allowingSyntheticEvent:completionHandler:]_block_invoke_3.816 0x00007fff23849288
-[NSTextInputContext tryHandleEvent_HasMarkedText_withDispatchCondition:dispatchWork:continuation:] 0x00007fff230a1f3d
__84-[NSTextInputContext _handleEvent:options:allowingSyntheticEvent:completionHandler:]_block_invoke.813 0x00007fff2384920e
__TSMProcessRawKeyEventWithOptionsAndCompletionHandler_block_invoke_5 0x00007fff288cdab1
invocation function for block in DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*) 0x00007fff288dfacb
__55-[NSTextInputContext handleTSMEvent:completionHandler:]_block_invoke.290 0x00007fff23844922
__55-[NSTextInputContext handleTSMEvent:completionHandler:]_block_invoke_2 0x00007fff2309ab28
-[NSTextInputContext tryHandleTSMEvent_HasMarkedText_withDispatchCondition:dispatchWork:continuation:] 0x00007fff2309aab0
-[NSTextInputContext handleTSMEvent:completionHandler:] 0x00007fff2309a008
_NSTSMEventHandler 0x00007fff23099832
DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*) 0x00007fff2886ddc1
SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*) 0x00007fff2886d1e3
SendEventToEventTargetWithOptions 0x00007fff2886d08c
SendTSMEvent_WithCompletionHandler 0x00007fff288c9f26
__SendUnicodeTextAEToUnicodeDoc_WithCompletionHandler_block_invoke 0x00007fff288ca3d1
__SendFilterTextEvent_WithCompletionHandler_block_invoke 0x00007fff288ca229
SendTSMEvent_WithCompletionHandler 0x00007fff288c9f74
SendFilterTextEvent_WithCompletionHandler 0x00007fff288c9d7c
SendUnicodeTextAEToUnicodeDoc_WithCompletionHandler 0x00007fff288c9a59
__utDeliverTSMEvent_WithCompletionHandler_block_invoke_2 0x00007fff288c9806
__utDeliverTSMEvent_WithCompletionHandler_block_invoke 0x00007fff288c9666
TSMKeyEvent_WithCompletionHandler 0x00007fff288c94ba
__TSMProcessRawKeyEventWithOptionsAndCompletionHandler_block_invoke_4 0x00007fff288c9250
__TSMProcessRawKeyEventWithOptionsAndCompletionHandler_block_invoke_3 0x00007fff288c90cf
__TSMProcessRawKeyEventWithOptionsAndCompletionHandler_block_invoke_2 0x00007fff288c8e9b
__TSMProcessRawKeyEventWithOptionsAndCompletionHandler_block_invoke 0x00007fff288c8c4e
TSMProcessRawKeyEventWithOptionsAndCompletionHandler 0x00007fff288b7a53
__84-[NSTextInputContext _handleEvent:options:allowingSyntheticEvent:completionHandler:]_block_invoke_3.811 0x00007fff2384910c
__204-[NSTextInputContext tryTSMProcessRawKeyEvent_orSubstitution:dispatchCondition:setupForDispatch:furtherCondition:doubleSpaceSubstitutionCondition:doubleSpaceSubstitutionWork:dispatchTSMWork:continuation:]_block_invoke.768 0x00007fff23848e63
-[NSTextInputContext tryTSMProcessRawKeyEvent_orSubstitution:dispatchCondition:setupForDispatch:furtherCondition:doubleSpaceSubstitutionCondition:doubleSpaceSubstitutionWork:dispatchTSMWork:continuation:] 0x00007fff2309832f
-[NSTextInputContext _handleEvent:options:allowingSyntheticEvent:completionHandler:] 0x00007fff23097d3b
-[NSTextInputContext _handleEvent:allowingSyntheticEvent:] 0x00007fff2309776e
-[NSView interpretKeyEvents:] 0x00007fff2309766c
invoke<*mut objc::runtime::Object,()> mod.rs:128
send_unverified<objc::runtime::Object,(*mut objc::runtime::Object),()> mod.rs:27
key_down mod.rs:178
key_down window.rs:786
-[NSWindow(NSEventRouting) _reallySendEvent:isDelayedEvent:] 0x00007fff22ffaee8
-[NSWindow(NSEventRouting) sendEvent:] 0x00007fff22ff9376
-[NSApplication(NSEvent) sendEvent:] 0x00007fff22ff81f1
-[NSApplication _handleEvent:] 0x00007fff232d0979
-[NSApplication run] 0x00007fff22e6069e
invoke<()> mod.rs:128
send_unverified<objc::runtime::Object,(),()> mod.rs:27
run mod.rs:178
run appkit.rs:438
run application.rs:75
run application.rs:153
launch<flex::AppState> app.rs:264
main flex.rs:343
call_once<fn(),()> function.rs:227
__rust_begin_short_backtrace<fn(),()> backtrace.rs:125
{{closure}}<()> rt.rs:66
lang_start_internal function.rs:259
lang_start_internal panicking.rs:379
lang_start_internal panicking.rs:343
lang_start_internal panic.rs:431
lang_start_internal rt.rs:51
lang_start<()> rt.rs:65
main 0x0000000100c940d6
start 0x00007fff20597f3d
start 0x00007fff20597f3d

[kud1ing]

I have no understanding of this but - (void)doCommandBySelector:(SEL)selector; only receives 1 selector, so maybe the signature of do_command_by_selector() is wrong?

[nathanwhit]

I was looking into this a bit the other day, and I have a bit of a view of what's going wrong. The key error is in this bit of code. This attempts to get the superclass of the current object, then tries to call doCommandBySelector on it. This doesn't make sense because doCommandBySelector is an instance method, not a class method. The code attempts to send a message to the class itself, not the instance, which is what causes the crash in #1720 .

I assume the intent of that snippet was to achieve the equivalent of

[super doCommandBySelector: cmd]

which calls doCommandBySelector on the current instance, but attempts to call the implementation from the superclass.

The proper translation to rust would be

if let Some(superclass) = this.class().superclass() {
    unsafe { msg_send![super(this, superclass), doCommandBySelector: cmd] }
}

However, this still doesn't make much sense. Apple's documentation makes it clear that implementations of doCommandBySelector: should not call super.

I'm not quite sure what the intended behavior was here and so I'm not sure what the correct replacement is, but it definitely doesn't seem correct to call the super.

Hope that helps some!

[cmyr]

Cool @nathanwhit thanks for the write-up. This will definitely be one of my major priorities once I'm back to focusing on druid stuff, and I feel this should be relatively easy to track down now.

[TonalidadeHidrica]

@cmyr Just a friendly reminder. Are you planning to fix this in the near future?

[inferiorhumanorgans]

@nathanwhit Forwarding the message up the chain is what NSResponder does and this is trying to achieve similar. That it's currently being forwarded to the wrong object (class instead of instance) is a bit of a red herring. As was pointed out above the message is a no-op so it doesn't really matter where it gets forwarded.

The issue is that the default macOS menu contains and registers the Cmd+Q hotkey. Without the menu the hotkey is unbound and nothing happens. There's basically two choices: always create the default menu (or a menu with a Cmd+q -> Quit item) or register the hotkey separately. For the latter registering a callback with addLocalMonitorForEventsMatchingMask is probably the way to go.

https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/EventOverview/MonitoringEvents/MonitoringEvents.html

[xStrom]

This seems to have been fixed with #2221. @TonalidadeHidrica could you spare a moment to verify that this is no longer an issue with master?