Opened 3 years ago

Last modified 6 months ago

#14269 accepted defect

wxEVT_KILL_FOCUS absent and wxFocusEvent GetWindow returns NULL or incorrectly in wxOSX-Cocoa

Reported by: aasselin Owned by: csomor
Priority: high Milestone:
Component: wxOSX-Cocoa Version: stable-latest
Keywords: EVT_KILL_FOCUS wxFocusEvent GetWindow Cc:
Blocked By: Blocking:
Patch: yes

Description

the controls sample demonstrate the problem on the choice and the radio box (the Focus Got log is well printed but then Focus Lost is never)

it seems related to #12267, however removing the if otherWindow==this does not fix entirely the problem (although it fixes the problem in my app)
there is something bizarre in this test, because resignFirstResponder is sent _before_ becomeFirstResponder and hence I don't see how FindFocus function could return something valid (after adding some logs, it always return otherWindow==this in fact for both resign/become events), so of course the otherWindow==this would block all events and remove the wxGrid problem...

what it means by the way is that in wxCocoa, the wxFocusEvent::GetWindow is always bad currently, even once the events are (nearly) well sent.

in addition the resignFirstResponder for combo does not work because once the first responder becomes the editor inside it (we ignore this first event), we don't catch anymore the event for the editor, and thus there is no kill focus event sent at all when leaving the combo
I don't know however how to fix that part.

Attachments (3)

combobox-textdidendediting.patch download (816 bytes) - added by aasselin 2 years ago.
make combo have kill focus
nonownedwindow-deactivate.patch download (986 bytes) - added by aasselin 2 years ago.
handle kill focus on deactivation
killfocus.patch download (6.9 KB) - added by aasselin 2 years ago.
make "otherWindow" work, fixed multi line text ctrl, many logs for debug

Download all attachments as: .zip

Change History (21)

comment:1 Changed 3 years ago by aasselin

the m_hasEditor trick seems to be completely flawed for the focus related stuff.

when the focus is lost by an editor/combobox/whatever hainvg m_hasEditor, the EVT_KILL_FOCUS event is never sent, even if the focus would be given to a completely different widget

comment:2 Changed 3 years ago by aasselin

continuing my investigations, I found that windowDidResignKey does strange things.

It sends a wxEVT_KILL_FOCUS event to the window being "key resigned", however there is almost no chance that that the "window" actually had focus as it is the wxFrame containing the focused item which gets this event.

I called that in place of sending the event:

[window makeFirstResponder:nil];

it seems to work as expected.
again a Cocoa professional should probably do the patch, as i am not sure at all it is the good thing to do.

comment:3 Changed 2 years ago by vadz

  • Status changed from new to infoneeded_new

Is this the same as #14142 or another problem?

comment:4 Changed 2 years ago by aasselin

  • Patch set

It seems these are the same problems indeed.
The fact is that there is a pile of problems there, no just one. And there is a pretty complex history in our svn repo, so I join the patches here. There are good chances that they won't apply against trunk as is, please take them more as an example.

Changed 2 years ago by aasselin

make combo have kill focus

Changed 2 years ago by aasselin

handle kill focus on deactivation

Changed 2 years ago by aasselin

make "otherWindow" work, fixed multi line text ctrl, many logs for debug

comment:5 Changed 22 months ago by johnr

  • Keywords wxFocusEvent GetWindow added
  • Status changed from infoneeded_new to new
  • Summary changed from wxEVT_KILL_FOCUS on wxCocoa to wxEVT_KILL_FOCUS absent and wxFocusEvent GetWindow returns NULL or incorrectly in wxOSX-Cocoa

Changed the title and keywords to make this easier to find.

comment:6 Changed 22 months ago by csomor

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

comment:7 Changed 7 months ago by vadz

  • Priority changed from normal to high

This is really important as not getting the expected focus events breaks a lot of other things and does it non obvious ways. I don't know if the approach in these patches is correct, but if they work, it would be great to apply them.

comment:8 follow-up: Changed 7 months ago by csomor

what steps do I have to perform on with trunk to reproduce the current problems ?

For radiobuttons and popup lists there also is platform difference:

neither radioboxes nor popup lists (choice) get the native focus on OSX, ie it still remains on a different control that can receive the native focus.

The patch for adding the focus-lost for combobox is the one that is clear to me, I'll apply accordingly

comment:9 Changed 7 months ago by csomor

a first commit is r76565, having the wrong commit message unfortunately

comment:10 in reply to: ↑ 8 Changed 7 months ago by paulclinger

Replying to csomor:

what steps do I have to perform on with trunk to reproduce the current problems ?

I've seen this issue with wxTreeCtrl that only trigger SET_FOCUS, but not KILL_FOCUS events, but it seems like it applies to many other events. For example, switching between tabs in wxAuiNotebook doesn't trigger KILL_FOCUS for wxSTC controls inserted in those tabs. This can be easily seen in auidemo sample.

comment:11 Changed 7 months ago by johnr

I have been using the following focus lost code in combobox.mm for a year or so since we did the field editor change. The passage of time makes me unsure whether the extra code compared to the ticket patch is necessary?

- (void)controlTextDidEndEditing:(NSNotification *)aNotification
{
    wxUnusedVar(aNotification);
    wxWidgetCocoaImpl* impl = (wxWidgetCocoaImpl* ) wxWidgetImpl::FindFromWXWidget( self );
    if ( impl )
    {
        wxNSTextFieldControl* timpl = dynamic_cast<wxNSTextFieldControl*>(impl);
        if ( timpl )
        {
            NSRange range = [fieldEditor selectedRange];
            timpl->SetInternalSelection(range.location, range.location + range.length);
        }
        
        impl->DoNotifyFocusEvent( false, NULL );
    }
}

comment:12 Changed 7 months ago by csomor

Hi John, thanks a lot, yes that's about what I have in my current sandbox, plus I'm setting the 'otherWindow' as well. But right now I'm getting an additional focus lost event when the combobox is clicked (due to a endEditing by the native combobox) which makes the NSWindow itself the firstResponder …. so I'm still debugging that, also when just switching toplevel windows there's a focus lost but never a focus set event, because the field editor is directly set as firstResponder, so I'll have to move some code to the field editor itself.

comment:13 Changed 7 months ago by SC

In 76575:

refactoring to common code for focus set and lost events, so that changes can be made a single place, see #14269

comment:14 Changed 7 months ago by SC

In 76576:

refactoring to common code for updating selections, using common focus code, see #14269

comment:15 Changed 7 months ago by SC

In 76577:

allowing reentrancy on NSPanels makeResponder as in NSWindow, see #14269

comment:16 Changed 7 months ago by SC

In 76578:

in order to get all focus set events, store field in editor and catch becomeFirstResponder there, see #14269

comment:17 Changed 7 months ago by SC

In 76565:

allowing nested makeFirstResponder calls, see #14269

comment:18 Changed 6 months ago by vaclavslavik

Note that whatever the ultimate fix will be, it should not call makeFirstResponder:nil from windowDidResignKey: as both attachment:nonownedwindow-deactivate.patch​ and r73043 currently in SVN do. Doing so breaks input source handling in NSTextView (i.e. multiline/rich text controls), especially if the user has Automatically switch to a document's input source enabled in keyboard preferences.

In that situation, it becomes impossible to change input keyboard in wx apps, because as soon as you click the Input menu in the menubar, the text control looses focus (and so you can't change it's input source). If you compare the behavior of wx apps with native (e.g. TextEdit's while in Edit Link sheet), they don't loose key indicator when clicking the menu items, only when another text field becomes key.

Reverting r73043 fixes that, but it's clear from this bug, #13495 and #14142 that that isn't a satisfactory solution either :-(

Note: See TracTickets for help on using tickets.