Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Navigation with TAB does not work well in wxOSX/Cocoa #17341

Closed
wxtrac opened this issue Jan 20, 2016 · 13 comments
Closed

Navigation with TAB does not work well in wxOSX/Cocoa #17341

wxtrac opened this issue Jan 20, 2016 · 13 comments
Labels
macOS Specific to Cocoa macOS port v3.0.2

Comments

@wxtrac
Copy link
Collaborator

wxtrac commented Jan 20, 2016

Issue migrated from trac ticket # 17341

component: wxOSX | priority: normal | resolution: fixed | keywords: Cocoa TAB navigation

2016-01-20 09:34:17: ikamakj created the issue


In the Carbon version, navigation between dialog or panel fields follows the logic in wxControlContainer regardless of where Tab is pressed. This is implemented in wxApp::MacSendCharEvent, in case the container has wxTAB_TRAVERSAL flag. The Cocoa version does not contain similar code but relies on native Cocoa navigation. I don't know the Cocoa logic exactly, but it is clearly different from the wxWidgets navigation order, which corresponds to the child window order. The Cocoa navigation seems to depend on geometrical positions rather than child order. As a result, the navigation does not always work in the way the programmer intended.

The situation is particularly bad if the Full keyboard access setting in System Preferences is set to "All controls", because then the successor or predecessor field is chosen by the wxWidgets logic for some controls and by the Cocoa logic for others. In other words, there is no unique ordering relation for all the controls. Often the result is that some controls cannot be reached by Tab at all. The navigation may also get stuck in a local loop, because if field B is after A from A's viewpoint, A may still be after B from B's viewpoint. The visiting order in backward navigation is not always the opposite of that in forward navigation, because if B is the successor of A from A's viewpoint, A is not necessarily the predecessor of B from B's viewpoint.

If the full keyboard access setting is "Text boxes and lists only", problems are less likely, but one problem that occurs even in this case is navigation into and out of a book control if the dialog contains both a book control and controls outside it. (My observations are from wxAuiNotebook, I have not tested other book controls.) Pressing Tab in the last focusable control before the notebook should always move the focus to the first control inside the notebook, but this actually happens only if the former control is one that generates navigation events itself (i.e., lets the wxControlContainer code to determine the successor), such as a wxTextCtrl. (wxComboBox is an example of a class that participates in native navigation regardless of the setting but does not generate navigation events.) Pressing Tab in the last control of the current notebook page should move focus to the first control after the notebook, but actually the focus goes to a control on the next notebook page, unless the focused control generates navigation events itself. There are similar problems in backward navigation with Shift+Tab.

The fact that there is no global Tab-processing code like MacSendCharEvent also has the consequence that Tab navigation does not work at all from controls that are not native Cocoa controls but do not have their own processing for Tab. One example is wxTreeCtrl.

I have also observed these navigation problems with individual control classes:

  • Navigation from multi-line wxTextCtrl backwards with Shift+Tab does not work.
  • wxListBox does not participate in Tab navigation unless "All controls" is selected (in Carbon it always does).

The version with which I have tested is based on 3.0.2, but I have modified it with all the recent keyboard and focus related changes in window.mm, nonownedwnd.mm, combobox.mm, textctrl.mm, plus the necessary header files.

Is there some technical problem in making Tab navigation follow the wxControlContainer logic for all controls? If somebody prefers the Cocoa logic, could this at least be made an option via wxSystemOptions? Let me finally note that for those who want to use the Cocoa logic, it should be done consistently for every control to ensure unique ordering, i.e., no control class should send a wxWidgets navigation event on a Tab press. (For me the wxControlContainer logic would be fine, so this last issue is irrelevant.)

@wxtrac
Copy link
Collaborator Author

wxtrac commented Jan 20, 2016

2016-01-20 18:44:39: paulclinger (Paul K) commented


This is a good summary. I've opened a similar defect (#17064), but mine is specifically about Ctrl-(Shift-)TAB navigation, although I suspect the underlying cause may be the same.

In my case the biggest issue is that none of the workarounds I tried work; not even using EVT_CHAR_HOOK (I don't get Ctrl-TAB event in my handlers no matter what I try) and because I'm using wxwidgets through another layer, I'm limited to using only the public API. EVT_KEY_UP is the only place where Ctrl-TAB shows up, but by that time all the navigation is already done by the internal wxwidgets logic; I'm still hunting for where exactly it happens...

@wxtrac
Copy link
Collaborator Author

wxtrac commented Jan 20, 2016

2016-01-20 19:29:34: @JulianSmart commented


Just to second/third this problem with looping and navigation failure when panels are embedded in dialogs; one of the major issues remaining in my OSX application beta, along with the focus issue. Thanks for describing it in detail. Sorry I don't have the technical know-how to solve it but I'd be very happy to help pay for consultancy to smooth out these issues should that be appropriate and possible.

@wxtrac
Copy link
Collaborator Author

wxtrac commented Jan 20, 2016

2016-01-20 21:17:47: paulclinger (Paul K) commented


I'd chip in as well, if this directly or indirectly fixes #17064. ("directly" would fix Ctrl-(Shift-)TAB to navigate wxAuiNotebook tabs as it does on Windows and "indirectly" would at least fix EVT_CHAR_HOOK to intercept Ctrl-(Shift-)TAB.)

@wxtrac
Copy link
Collaborator Author

wxtrac commented Jan 26, 2016

2016-01-26 10:14:14: @bigjohnr commented


Key handling was changed in 2013 to handle keyboard entry using IME. This had adverse effects that remain for some controls. Tab navigation can be absent or bizarre in some layouts.

See #15903 for revision details.

@wxtrac
Copy link
Collaborator Author

wxtrac commented Apr 19, 2016

2016-04-19 19:29:51: scrawford1968 (Scott Crawford) commented


Changing the Full keyboard access setting in System Preferences to "All controls" and using the sample code "dialogs", replacing the MyFrame::DlgCenteredScreen function with the sample code below reproduces the issue.

Steps:

  1. Run the dialogs sample.
  2. Click the menu "Dialogs->Generic Dialogs->Centered on screen".

TAB key navigation works in this sample between the four buttons but after pressing TAB while on the "Button 4" does not loop back to "Button 1" as it should. If the "green" panel and its child buttons are removed the navigation appears to work.

When I last looked into this the wxTextCtrl processes the TAB and does its own widgets navigation calls. It worked in wxWidgets 2.8 because all navigation used to be handled by widgets.


Sample Code:

class TestDialog : public wxDialog
{
public:
    TestDialog(wxWindow* parent)
    : wxDialog(parent, wxID_ANY, wxT("Test Dialog"))
    {
        wxBoxSizer* dialogSizer = new wxBoxSizer(wxVERTICAL);
        wxPanel* panel = new wxPanel(this, wxID_ANY);
        panel->SetBackgroundColour(*wxGREEN);
        wxBoxSizer* boxSizer = new wxBoxSizer(wxHORIZONTAL);
        boxSizer->Add(new wxButton(panel, wxID_ANY, "Button 1"), 0, wxALL, 5);
        boxSizer->Add(new wxButton(panel, wxID_ANY, "Button 2"), 0, wxALL, 5);
        panel->SetSizer(boxSizer);
        dialogSizer->Add(panel, 0, wxALL, 5);
        boxSizer = new wxBoxSizer(wxHORIZONTAL);
        boxSizer->Add(new wxButton(this, wxID_ANY, "Button 3"), 0, wxALL, 5);
        boxSizer->Add(new wxButton(this, wxID_ANY, "Button 4"), 0, wxALL, 5);
        dialogSizer->Add(boxSizer, 0, wxALL, 5);
        SetSizerAndFit(dialogSizer);
    }
};

void MyFrame::DlgCenteredScreen(wxCommandEvent& WXUNUSED(event))
{
    //wxDialog dlg(this, wxID_ANY, wxT("Dialog centered on screen"),
    //             wxDefaultPosition, wxSize(200, 100));
    //(new wxButton(&dlg, wxID_OK, wxT("Close")))->Centre();
    TestDialog dlg(this);
    dlg.CentreOnScreen();
    dlg.ShowModal();
}

@wxtrac
Copy link
Collaborator Author

wxtrac commented May 3, 2016

2016-05-03 11:07:28: ikamakj uploaded file tab_navigation_fix_general.patch (3.3 KiB)

@wxtrac
Copy link
Collaborator Author

wxtrac commented May 3, 2016

2016-05-03 11:07:39: ikamakj uploaded file tab_navigation_fix_password.patch (2.1 KiB)

@wxtrac
Copy link
Collaborator Author

wxtrac commented May 3, 2016

2016-05-03 11:15:07: ikamakj commented


I am now submitting a patch that makes the Cocoa build follow wxWidgets navigation logic in a way that is satisfactory at least as a temporary solution. This does not fix Ctrl(+Shift)+Tab for controls other than wxTextCtrl, and with some specific types of controls there are still problems due to deficiencies in their implementation. Details below.

The fix is based on the fact that, according to wxWidgets API, any control wanting to handle navigation keys itself must tell this to wxWidgets by setting flag wxWANTS_CHARS. Surprisingly, this flag has presently an effect on wxMSW only, but the documentation does not mention any platform restrictions. Therefore, when receiving a Tab press from a field with this flag on, the framework can safely treat it as a navigation key and need not send keydown or char events to the control at all. This is just the wxMSW logic in wxWindowMSW::MSWProcessMessage(). In the patch, a function HandleNavigationByTab(), called from wxWidgetCocoaImpl::DoHandleKeyEvent(), checks the flag and if it is not set, Tab is treated as a navigation key and no further processing is done.

Notes:

  • HandleNavigationByTab is called after sending wxEVT_CHAR_HOOK so it is still possible to preprocess Tab in a char hook handler as before, if someone wants that (and the processing order is the same as in wxMSW).

  • The patch assumes that Tab presses do not need to be passed to IME.

  • The code in HandleNavigationByTab() was taken from the Carbon function wxApp::MacSendCharEvent, but the ControlDown test in it seems meaningless. In the present version ControlDown actually means “Command down”, and Command+Tab is handled by the system to switch between applications so DoHandleKeyEvent() is never entered with this combination. On the other hand, (raw) Control+Tab does not always generate a call to this function, as noted below, and since it is a standard Mac means for navigating between fields (i.e. does the same as plain Tab except possibly in text fields), treating it as “window change” here would be questionable. See also Ctrl-(Shift-)Tab doesn't properly navigate over tabs on OSX #17064.

  • Text controls must be handled separately because wxWANTS_CHARS is not required for them in order to insert tabulator characters; wxTE_PROCESS_TAB is sufficient. (Perhaps this issue should be clarified in the documentation.) HandleNavigationByTab() now detects text controls by a dynamic cast, but a more elegant solution would be a virtual function telling if the control wants to process Tab. Such a function would be analogous to the WM_GETDLGCODE message sent in wxWindowMSW::MSWProcessMessage().

  • Radio boxes are handled as a special case to make navigation work when focus is in them (they can currently be focused only programmatically, see below). Ways to replace this special handling by virtual functions should be investigated. For example, GetMainWindowOfCompositeControl() might be a candidate for getting the “true” focused control, but that function should then be overridden for the buttons of the radio box appropriately, and such a solution would break the logic for e.g. wxGenericListCtrl, where GetMainWindowOfCompositeControl() has been implemented for the list main window but only the main window has wxWANTS_CHARS, the list control itself hasn't. This discussion assumes that one wants to treat a radio box as a single entity in navigation, as is the case in wxMSW and wxOSX/Carbon. (Another issue is navigation between the radio buttons with arrow keys; this is also still unimplemented in wxOSX.)

  • Control(+Shift)+Tab is a problem because from it we do not get a wxWidgetCocoaImpl::keyEvent() call for controls other than wxTextCtrl. For text controls, wxWidgetCocoaImpl::doCommandBySelector() is executed with selector selectNextKeyView or selectPreviousKeyView, but that does nothing because the call occurs outside the keyEvent() call and therefore m_lastKeyDownEvent is null. There is more discussion on this combination in Ctrl-(Shift-)Tab doesn't properly navigate over tabs on OSX #17064. A similar problem is that Shift+Tab does not work in text controls if handling is left to them. Instead of trying to correct these in the text control code, I wrote HandleNavigationByTab() in such a way that if any modifier keys are down, wxTE_PROCESS_TAB is ignored and that key combination is used for navigation. This way, Control(+Shift)+Tab do work for text controls. The assumption is that whenever a text control has wxTE_PROCESS_TAB, it actually only wants plain Tab without modifiers.

Remaining problems with specific control types:

  • Navigation does not work in text controls having wxTE_PASSWORD style, because we do not get a wxWidgetCocoaImpl::keyEvent() call from key down events, only from key up events. However, the doCommandBySelector method of wxNSSecureTextField is executed for certain special keys, and the handling of Enter has been previously added there. I include another patch where the Cocoa commands from Tab key are similarly converted back to wxKeyEvents and these are processed in the same way as in the DoHandleKeyEvent function mentioned above. This patch includes Escape handling and wxEVT_CHAR_HOOK processing so it also fixes the problem that Escape key did not close a dialog when focus was in a password field. This does not solve all problems with password controls: e.g. character filtering by validators does not work due to the lack of keydown and char events (see wxTextValidator does not work if wxTextCtrl has wxTE_PASSWORD style #17185).

  • Navigation does not work in wxTreeCtrl because wxGenericTreeCtrl is not internally consistent: Create() sets wxWANTS_CHARS but OnChar() does not handle Tab. This is straightforward to fix by adding a HandleAsNavigationKey call to wxGenericTreeCtrl::OnChar(). Inspection of the code revealed that at least the following classes have the same defect: wxGenericCalendarCtrl, wxDatePickerCtrlGeneric, wxVListBox (the latter handles Tab in wxMSW build only).

  • Navigation skips radio boxes because they are internally group boxes for which “canBecomeKeyView:” returns NO. This can be fixed by overriding AcceptsFocusFromKeyboard as follows:

bool wxRadioBox::AcceptsFocusFromKeyboard() const
{
return m_radioButtonCycle->AcceptsFocusFromKeyboard();
}

  • Navigation skips wxComboBoxes having wxCB_READONLY flag even when full keyboard access is on, because “canBecomeKeyView:” returns NO regardless of the full keyboard access setting. This seems to be a Cocoa bug and probably the only way of fixing it would be checking the value of the setting. See also the discussion in Preset focus does not work in wxOSX/Cocoa #17340.

Perhaps someone with better Cocoa knowledge can suggest better solutions, but I would like to mention that if there is no other way of making Ctrl(+Shift)+Tab work generally, the last resort is to intercept Tab in WX_filterSendEvent, similarly to the mouse event processing that handles capture. This approach would also make it possible to get keydown and char events from password controls, solving #17185.

@wxtrac
Copy link
Collaborator Author

wxtrac commented May 3, 2016

2016-05-03 15:57:45: @vadz changed status from new to confirmed

2016-05-03 15:57:45: @vadz commented

Thanks a lot for working on this!

I just wanted to give a brief explanation about wxWANTS_CHARS: it's a bad hack which was only added because we couldn't make things work under wxMSW without it due to the way dialog navigation works under this platform. I'm not really thrilled that we now need it under OS X as well, but OTOH it seems very unlikely that we can ever find a way to remove the need for using this flag, so why not.

Concerning the patch itself, could we override DoHandleCharEvent() in wxRadioBox and wxTextCtrl instead of using dynamic casts checking for them?

Other than that, this patch clearly seems to be a big improvement and I'd like to apply it, thanks again!

@wxtrac
Copy link
Collaborator Author

wxtrac commented May 6, 2016

2016-05-06 08:41:02: ikamakj commented


To Vadim: I don't know why you mention DoHandleCharEvent(), that function is executed if the key is not handled by the patch code so it has nothing to do with this issue. If you mean DoHandleKeyEvent(), I would not override the whole function, and not even HandleNavigationByTab, because only a couple of lines in HandleNavigationByTab depend on the control type. I cannot spend more time on this issue right now because I still have many other Cocoa problems to solve, but here are some further notes:

  • For radio boxes the best way might be to have wxRadioBox set wxWANTS_CHARS for the radio buttons in the box, and make those buttons to handle tab in such a way that the wxNavigationKeyEvent is sent to the parent of the radio box, not the direct parent of the button (i.e., the radio box). Such key handling would be necessary anyway to implement arrow key navigation inside the box. This would require either deriving a specialised class from wxRadioButton or make wxRadioButton aware that it may be inside a wxRadioBox. I would rather leave this decision to the wxWidgets core team.

  • For wxTextCtrl, wxTE_PROCESS_TAB could possibly imply wxWANTS_CHARS so testing the latter would be sufficient, but I don't know what implications such a radical change would have, and there would remain the problem with modifier keys. Note that HandleNavigationByTab cannot handle e.g. Shift+Tab unconditionally for every control because some classes, such as wxGrid, use it for their own purposes.

  • In addition to the specific controls I listed previously, wxGenericListCtrl probably needs correction as well for navigation out of it to work. In my tests it did work because I used a version where both wxListMainWindow::OnChar() and wxListMainWindow::OnChildFocus() had been modified in order to fix a navigation problem that existed already in Carbon build. Those fixes were originally done to version 2.8, so I should investigate this issue further with the present version to be able to make any suggestions for correcting it.

  • If one wants wxListBox to take part in the navigation cycle regardless of the full keyboard access setting (see my original posting), this can be done by adding the following override to wxListWidgetCocoaImpl:
    virtual bool CanFocus() const wxOVERRIDE { return true; }

@wxtrac
Copy link
Collaborator Author

wxtrac commented Jun 5, 2017

2017-06-05 07:49:10: sbrowne (Steve) commented


I created a PR for this with some of the requested changes:
#493

@wxtrac
Copy link
Collaborator Author

wxtrac commented Jul 16, 2017

2017-07-16 16:03:08: @vadz changed status from confirmed to closed

2017-07-16 16:03:08: @vadz set resolution to fixed

2017-07-16 16:03:08: @vadz commented

In a07013a:
Tab navigation improvements for wxOSX

Closes #17341.

Closes #493

@wxtrac wxtrac closed this as completed Jul 16, 2017
@wxtrac
Copy link
Collaborator Author

wxtrac commented Jul 16, 2017

2017-07-16 16:03:49: @vadz commented


I've finally applied the patch, sorry again for the delay. And big thanks to both of you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
macOS Specific to Cocoa macOS port v3.0.2
Projects
None yet
Development

No branches or pull requests

1 participant