Refactor iOS text input activation to better work with hardware keyboards #11406
Conversation
|
This generally sounds good to me, but if you can revert the checkkeys change that would be great. You can always make that change locally, but the checkkeys program is specifically designed to test text input, so we want that enabled at start. |
|
Absolutely, I thought it's fine either way since you can start text input by right-clicking the app, but I guess there can be situations where the environment is limited. |
|
Can you do |
frenzibyte
force-pushed
the
ios-hardware-keyboard-handling
branch
from
db5781f
to
da09e82
Compare
| { | ||
| @autoreleasepool { | ||
| SDL_uikitviewcontroller *vc = GetWindowViewController(window); | ||
| [vc setKeyboardProperties:props]; | ||
| [vc showKeyboard]; | ||
| [vc setTextFieldProperties:props]; |
There was a problem hiding this comment.
It seems doing this here is not a good idea, if text input is active but StartTextInput is called again with different properties then the new properties are not applied because text input is already active. I guess this explains why this existed in ShowScreenKeyboard (and why that function takes text input properties).
Thoughts on how this should be fixed? I'm thinking about defining a new function in SDL_VideoDevice that is solely responsible for updating the text input properties (call it SetTextInputProperties), and have that function be called on StartTextInput regardless of text input active state or keyboard visibility state.
There was a problem hiding this comment.
That seems reasonable, and can be done as a separate commit.
There was a problem hiding this comment.
It may be that on some (all?) platforms the on-screen keyboard needs to be hidden and shown with the new properties when they change.
There was a problem hiding this comment.
It may be that on some (all?) platforms the on-screen keyboard needs to be hidden and shown with the new properties when they change.
Well, since SDL_StartTextInput is the one accepting text input properties, I think it's sort of expected for SDL to handle the case where text input is already active. I think the approach of a separate method for updating text properties is flexible enough to handle both platforms that can update without hiding the keyboard (by directly updating the required states), and platforms that require hiding the keyboard first (by internally hiding and showing the keyboard with the new properties).
There was a problem hiding this comment.
Actually looking at the X11/steamdeck implementation, I think I'm better off moving setTextFieldProperties back to be called on showing on-screen keyboard and call it a day.
There was a problem hiding this comment.
Never mind I misread again, ShowScreenKeyboard does not get called if the keyboard is shown already...
I guess I'll add the new functoin and implement it only on iOS for now.
| /* When the user dismisses the software keyboard by the "hide" button in the bottom right corner, | ||
| * we want to reflect that on SDL_TextInputActive by calling SDL_StopTextInput...on certain conditions */ | ||
| if (SDL_TextInputActive(window) | ||
| /* keyboardWillHide gets called when a hardware keyboard is attached, | ||
| * keep text input state active if hiding while there is a hardware keyboard. | ||
| * if the hardware keyboard gets detached, the software keyboard will appear anyway. */ | ||
| && !SDL_HasKeyboard() | ||
| /* When the device changes orientation, a sequence of hide and show transitions are triggered. | ||
| * keep text input state active in this case. */ | ||
| && !rotatingOrientation) { | ||
| SDL_StopTextInput(window); | ||
| } |
There was a problem hiding this comment.
Upon further testing, having this also breaks password manager integrations (when setting SDL_PROP_TEXTINPUT_TYPE_NUMBER to SDL_TEXTINPUT_TYPE_TEXT_USERNAME/SDL_TEXTINPUT_TYPE_TEXT_PASSWORD_HIDDEN)
RPReplay_Final1730711349.MP4
Notice how pressing the "Passwords" button in the keyboard hides the keyboard, which triggers the SDL application to stop text input. This is problematic because when the user selects their password from the keychain, the input is ignored since text input is not active.
I think it would be a better option to keep text input state completely manual rather than tied to the state of the keyboard, and do away with all these conditionals that are set in place here. @slouken thoughts? do other platforms (aka Android) stop text input when the user hides the keyboard?
There was a problem hiding this comment.
Yes, the expectation is that text input state is tied to the visibility of the on-screen keyboard, if there is no hardware keyboard attached.
There was a problem hiding this comment.
Since this is SDL 3.0, we can revisit that assumption, but that's the way it is currently. Bringing up the on-screen keyboard on Android, for example, will enable text input so it can be used to input text into the application.
There was a problem hiding this comment.
With the situation above, there is no possible way to enter the selected password without breaking that assumption, unless SDL can somehow bypass the "text input active" check when sending text events and do that on textFieldTextDidChange events, but I think that's too much of a hacky solution IMO.
Assuming that each application that starts text input has a path for stopping text input, it would be much easier to keep text input state manual and detached from the visibility of the on-screen keyboard, to allow integration with password managers etc.
| // the text field needs to be re-added to the view in order to update correctly. | ||
| UIView *superview = textField.superview; | ||
| [textField removeFromSuperview]; | ||
| [superview addSubview:textField]; |
There was a problem hiding this comment.
This can technically be split to a separate PR but I figured I could include it in this PR since I'm touching the entire file anyway.
Description
Text input in SDL's iOS implementation is directly activated by requesting the hidden
UITextFieldto become first responder. Once it becomes first responder, a software keyboard is shown if there are no hardware keyboard is attached, andtextFieldTextDidChangeevents are raised whenever text is inputted by a hardware/software keyboard.SDL has been doing the
becomeFirstResponder/resignFirstRespondercalls in theShowScreenKeyboard/HideScreenKeyboardpath. These paths are controlled by theAutoShowingScreenKeyboard()function, which returns false when a hardware keyboard is attached (unless theSDL_HINT_ENABLE_SCREEN_KEYBOARDhint is set to 1), therefore the text fields never become first responder and no text input is allowed by hardware keyboard regardless of whetherSDL_StartTextInputis called.This is unexpected behaviour, the text field become/resign first responder calls should not be in the
ShowScreenKeyboard/HideScreenKeyboardpath, they should instead be in theStartTextInput/StopTextInputpath, which makes more sense given what the logic actually does (make the text field first responder / focused to receive text input).Therefore, I've refactored the flow surrounding text input activation to do as proposed above.
I've also rewritten part of the logic of activating/deactivating text input in the
keyboardWillShow/keyboardWillHideevents so that it's clear why text input is stopped/started. As far as I can understand, text input should be stopped when the keyboard is hidden by user's request, unless one of these conditions are true:keyboardWillHideis raised)keyboardWillHide/keyboardWillShowcalls: text input should remain activated in this case.Also, if a
keyboardWillShowtransition is subsequently invoked before akeyboardDidHidecall, this indicates that the hide has been interrupted with a show, and in that case we want to bring back text input.One might ask why
checkKeysstill works with hardware keyboard despite the above, and the answer to that is that the program callsSDL_StartTextInputbefore the hardware keyboard is added by the GCKeyboard event flow. I've confirmed this by checking the state ofSDL_HasKeyboard()when the function is called in the test app, and it returned false. I've changed the test to not start text input immediately, but if that is bad then I can revert that.Also, worthy of note that iOS simulator does not send
GCKeyboardDidConnect/GCKeyboardDidDisconnectnotifications whenever a hardware keyboard is attached/detached by Shift+Cmd+K or the "Connect Hardware keyboard" menu item, andGCKeyboard.coalescedKeyboardis always not null/nil. Therefore it's impossible to test the "text input deactivates when user hides keyboard" behaviour on iOS simulator, due toSDL_HasKeyboard()always being true.Existing Issue(s)