Opened 15 months ago

Closed 11 months ago

Last modified 11 months ago

#15003 closed defect (fixed)

wxOSX-Cocoa is not allowing customized key handling in wxComboBox

Reported by: rowbearto Owned by: csomor
Priority: normal Milestone: 2.9.5
Component: wxOSX-Cocoa Version: stable-latest
Keywords: keyboard wxComboBox wxOSX cocoa wxEVT_KEY_UP wxEVT_KEY_DOWN Cc:
Blocked By: Blocking: #3821
Patch: yes

Description

I'm trying to catch when the user hits the "ENTER" or UP/DOWN arrow keys in a wxComboBox and execute some of my application code on this event.

I've attached a small modification to the keyboard sample that allows the problem to be reproduced.

In this sample program, instead of using a plain wxWindow to catch keyboard events, a class that is derived from wxComboBox is used.

The sample works great in Windows, you can see that all 4 events - wxEVT_CHAR_HOOK, wxEVT_KEY_DOWN, wxEVT_KEY_UP, and wxEVT_KEY_CHAR are caught.

However, none of the handlers get called in wxOSX with Cocoa. I did not try with Carbon.

I did some debugging, and I found that the OS is calling wxOSX_keyEvent() (in window.mm) only for the wxEVT_KEY_UP, but not for the other events. In the unmodified keyboard sample, the OS is calling wxOSX_keyEvent() for all of the 4 different types of events.

But for wxEVT_KEY_UP, even though wxOSX_keyEvent() is getting called, my event handler for wxEVT_KEY_UP is still not getting called.

I did some debugging, and I traced the problem to this statement in the wxWidgetCocoaImpl::keyEvent() function in src/osx/cocoa/window.mm:

if ( GetFocusedViewInWindow([slf window]) != slf
m_hasEditor DoHandleKeyEvent(event) )

The problem is that for the wxComboBox, m_hasEditor is 'true', so 'DoHandleKeyEvent()' does not get called (since the boolean in the 'if' is already true, no need to perform the final OR ).

A simple transpose of the last 2 parameters (moving m_hasEditor to the end) will fix the issue, and my handler for wxEVT_KEY_UP gets called. This transpose is shown below:

if ( GetFocusedViewInWindow([slf window]) != slf
DoHandleKeyEvent(event) m_hasEditor )

After this change, I am able to get the wxEVT_KEY_UP handler to execute, but I still can't get the wxEVT_KEY_DOWN or other handlers to execute. It seems that the OS is not even calling wxOSX_keyEvent() (I have breakpoint set in the debugger, thats how I know) when those events occur.

So:

1) For wxEVT_KEY_UP, I think my suggested patch should be applied to the trunk, unless there is a good reason to not call DoHandleKeyEvent() if m_hasEditor is true?

2) It is still a mystery why the OS is only triggering the callback for wxEVT_KEY_UP, and it does not invoke the callback for wxEVT_KEY_DOWN, wxEVT_CHAR, or wxEVT_CHAR_HOOK. Any ideas for why that does not work?

I've attached the modification as a patch. Its just a partial solution though, as only wxEVT_KEY_UP is getting handled.

Attachments (3)

keyboard.cpp download (13.3 KB) - added by rowbearto 15 months ago.
Modified keyboard sample to reproduce the problem.
keyboard_cocoa.patch download (671 bytes) - added by rowbearto 15 months ago.
combokey_events.patch download (3.0 KB) - added by johnr 14 months ago.

Download all attachments as: .zip

Change History (15)

Changed 15 months ago by rowbearto

Modified keyboard sample to reproduce the problem.

Changed 15 months ago by rowbearto

comment:1 in reply to: ↑ description ; follow-up: Changed 15 months ago by johnr

This is similar to #13672 which has not been addressed I think because there are few people who understand the code. I am not one of them.
Interestingly, at first look I do see NSKeyDown events generated for the combobox in utils.mm (as long as the combobox is not read only).
I modified

- (void)sendEvent:(NSEvent *)anEvent
{
    if ([anEvent type] == NSKeyUp && ([anEvent modifierFlags] & NSCommandKeyMask))
        [[self keyWindow] sendEvent:anEvent];
    else if ([anEvent type] == NSKeyDown ) // added lines
        [super sendEvent:anEvent];         // set a breakpoint
    else
        [super sendEvent:anEvent];
}

and get a hit on the above breakpoint on key down before raising the key. With the exception of the delete/backspace and modifier keys they get lost suggesting some filter mechanism.

When the sequence "m_hasEditor
DoHandleKeyEvent(event)" is reversed in osx/cocoa/window.mm as you suggest then the delete and modifiers KeyDown and all the KeyUp events get through to my test combobox functions. My look so far has been more trying to figure out how things work and I should have given it away a couple of hours ago.

comment:2 in reply to: ↑ 1 Changed 15 months ago by johnr

Reversing the sequence "m_hasEditor
DoHandleKeyEvent(event)" in osx/cocoa/window.mm just might have ramifications with other controls that do have editors but I would gladly defer to someone more familiar with this code. Another temporary fix for anyone that needs it is to add
- (void) keyUp:(NSEvent*) event
{
    wxWidgetCocoaImpl* impl = (wxWidgetCocoaImpl* ) wxWidgetImpl::FindFromWXWidget( (WXWidget) [self delegate] );
    if ( impl == NULL || !impl->DoHandleKeyEvent(event) )
        [super keyUp:event];
}

to cocoa/combobox.mm after line 55 which will give you keyUp events. It doesn't work for keyDown as the combo eats the events at present.

I think that we probably need to get a hold of the wxNSTextFieldEditor associated with the combo for a proper fix.

comment:3 Changed 15 months ago by csomor

  • Owner set to csomor
  • Status changed from new to accepted

comment:4 Changed 15 months ago by johnr

I have attached a combokey_events.patch that enables keyDown and keyUp events for wxComboBox in OSX.
It was an interesting project but needs a review by someone with more than my self assessed learner status with objective C and mac.

Rather than intercepting events at application or panel level, I have followed much of what the wxTextCtrl does using wxNSTextFieldEditor and its existing methods.

A couple of points:

  1. It will not work when the combo has the wxCB_READONLY flag as the editor is then disabled. I guess we could catch and remove the flag and make a home grown routine for wxMAC that worked similarly to how wxMSW works with the same flag but it is probably best left to the user.
  2. I am not sure whether the dealloc function in cocoa/combox.mm is necessary.
  3. I have made no explanatory comments in the code as yet but I had to move @implementation wxNSComboBox to cocoa/private.h so it could be used in nonownedwnd.mm.
  4. No changes to the interface docs.
  5. Finally, I have no Carbon installation and so have not looked at it there.

Changed 14 months ago by johnr

comment:5 Changed 14 months ago by johnr

Added a few explanatory comments to the patch.
In reply to myself:

  1. See ticket #10519
  2. dealloc function in cocoa/combox.mm is mandatory.
  3. That should have been "move @interface to cocoa/private.h"
  4. No help doc changes are required as no new user methods have been added
  5. Carbon remains untested by me.

comment:6 Changed 14 months ago by johnr

In case this is being considered I need to add that the existing patch is part 1. The routines so far work well when the drop down list is closed but we don't get all the keys when the drop down list is open. Hence we may get textbox display and selection index miss-match.

comment:7 Changed 14 months ago by johnr

  • Blocking 3821 added

(In #3821) This doesn't work properly in cocoa with either TAB or Shift-TAB because of the absence of wxComboBox OnChar event generation and a routine such as is used in textctrl_osx.cpp wxTextCtrl::OnChar(). In a simple layout the default navigation is often correct but in a more complex layout with columns and several combos together navigation does not follow wxWidgets' tab order or object creation order.

A fix needs #15003 fixed first to conform with wxTextCtrl's approach.

In cocoa there other controls where this happens eg wxSpinCtrl buttons, wxButtons and likely any control without dedicated key navigation control. I thought the parent window/panel could get the key events if not handled by its children and do navigation from there and maybe it does somewhere in there but I haven't found it.

comment:8 Changed 11 months ago by SC

(In [73961]) applying editor part of patch, see #15003

comment:9 Changed 11 months ago by csomor

  • Status changed from accepted to infoneeded

Thanks a lot for your patch John, adding the editor is the right thing to do IMHO.

What I don't see is the reason you've changed the filterSendEvent - you're now using the return value of the HandleMouseEvent of the captured window for 'handled' - do you have a bug that was related to the former way of never dealing with the mouse event in another window if there's a captured window.

comment:10 Changed 11 months ago by johnr

  • Status changed from infoneeded to accepted

Ah yes, it is not proper nor needed. I had almost forgotten about the patch let alone the changes to filterSendEvent :) i.e.

handled = ((wxWidgetCocoaImpl*)cw->GetPeer())->DoHandleMouseEvent( event); 
//handled = true; 

The above code doesn't exist in my working copy anymore and was used originally as a rough workaround for a problem using wxRichTextToolTip over controls that need to receive mouse events and so made it into this patch. Sorry for not noticing and thanks for weeding it out.

I did initially think more work might be needed for combo events but it works well for me. A couple of keys do not produce a keydown event but do produce a keyup event. From memory, I think the Enter key is the main protagonist and maybe adding wxTE_PROCESS_ENTER to the user's constructor might help but I didn't try it.

comment:11 follow-up: Changed 11 months ago by csomor

  • Resolution set to fixed
  • Status changed from accepted to closed

I don't see any problems anymore, all keystrokes (including ENTER) seem to be logged correctly, so I'm closing this

comment:12 in reply to: ↑ 11 Changed 11 months ago by johnr

Replying to csomor:

I don't see any problems anymore, all keystrokes (including ENTER) seem to be logged correctly

From memory the drop down list, when open, swallowed the ENTER key down stroke to close the list but understandably not the key up component. I am not sure how this compares to the MSW version.

Note: See TracTickets for help on using tickets.