Repository navigation
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The focused fix is platform-guarded and adequately covered by targeted runtime tests.
0 open findings
What changed in this PR
Fixes iOS WebAssembly soft-keyboard activation by refocusing the hidden native input when a focused TextBox is tapped.
Changes:
- Adds an iOS-only hidden-input refocus bridge.
- Invokes refocus from
TextBoxfocus notifications. - Adds WASM runtime coverage for touch, mouse, focus transfer, and non-iOS behavior.
| File | Description |
|---|---|
Given_TextBox.BrowserRefocusOnTap.cs |
Adds regression tests. |
BrowserSkiaTextBoxNotificationsProviderSingleton.cs |
Triggers native-input refocus. |
BrowserInvisibleTextBoxViewExtension.cs |
Adds managed JavaScript interop. |
BrowserInvisibleTextBoxViewExtension.ts |
Implements guarded iOS refocusing. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
morning4coffe-dev
left a comment
There was a problem hiding this comment.
Verified: the diff at head f47d5c4 (4 files), the TS refocus() and its call sites in BrowserInvisibleTextBoxViewExtension.ts, the managed chain from TextBoxCore (OnFocusStateChangedPartial at TextBoxCore.Input.cs:539, pointer paths at TextBoxCore.cs:314 and :349) to BrowserSkiaTextBoxNotificationsProviderSingleton.OnFocused (:15-18) and NativeMethods.Refocus (:408), the trailing-click guard (TS:105-134), and the 4 new tests. Code review only, nothing built or run. WinUI parity does not apply: WinUI has no soft-keyboard input-session model for TextBox on the web, so I did not look for a WinUI counterpart.
Findings, by severity:
-
Medium (unverified): the whole fix rests on WebKit behavior that the author read from WebKit source (focus() on the focused element goes through elementDidRefocus, and reports to the UI process only with a user gesture and no input view showing). I could not check that here. The device check is reported in the PR body on a 6.8 backport, which is evidence for the tap-brings-up-keyboard outcome. Please confirm the device result on the final head, not only the backport.
-
Low-medium: the iOS gate is navigator.platform (TS:7, /iP(ad|hone|od)/). On iPadOS in desktop-class mode, Safari reports "MacIntel" in navigator.platform, so refocus() returns at TS:485 and the fix does not apply there. The same gate already drives keepsInputOffscreen, so this is not new, but the PR claims iOS. Either note the limit in the PR or check navigator.maxTouchPoints > 1 alongside MacIntel. (From general Safari behavior; not checked in this repo.)
-
Low: OnFocused runs refocus on every focus transition to a non-Unfocused state (TextBoxCore.Input.cs:539), so the JS interop call happens on every platform even though the TS returns early off iOS (provider :18, TS:485). Cheap, but an iOS check on the managed side would avoid the call.
-
Low: refocus() arms swallowNextCanvasClick from lastPointerType (TS:493-495) even when focus came from code with no tap, such as a dialog. The flag is cleared by the next pointerdown and only swallows canvas-targeted mousedown/click, so the impact is small. Same design as the focusin path (TS:117-123); consider a comment saying the pointerdown reset is what bounds it.
-
Low (test hygiene): CountHiddenInputFocusCalls (Given_TextBox.BrowserRefocusOnTap.cs, end of file) replaces input.focus and input.blur as own properties on the element and never restores them. Restore them, or the wrappers stay on the element for later tests in the same run.
-
Info: RichEditBox shares the hidden input but does not raise this notification, so it is not covered. The PR says so.
Tests: 4 tests. The author reports that the first three fail on master and the last passes, and that Given_TextBox, Given_PasswordBox and Given_RichEditBox pass in full (1323 tests). I did not run them. The tests fake the iOS bridge by overwriting isIOS and lastPointerType, so they check the bridge's side only, which the PR also says.
CI: all completed checks green (csharp, js, java analysis, CodeQL, Codacy, Check Docs links, Summary, WIP, WinAppSDK Sync Generator Check). Mergify Merge Queue and Uno.UI - docs NEUTRAL. "review" SKIPPED. Copilot's review on head: "approval recommended, 0 open findings".
Not validated: the WebKit focus behavior, the iOS device result on the final head, the iPad desktop-mode case, and every runtime test in this review.
Tests, on Skia WASM in Chrome with this round's commits: |
By default iOS only shows the soft keyboard for an input that is focused while the user is interacting with the page. A TextBox focused from code at another time, such as the one a dialog opens on, gets no keyboard, and tapping it does not bring one up either: the tap lands on the canvas, and the hidden input, which has focus already, is not focused again. The hidden input is now focused again when a TextBox is tapped while it has focus, and when a tap moves focus to a TextBox the input is handed over to. WebKit treats focus() on the focused element as a refocus, reports it as the user's when it is called from inside the tap, and does nothing when the keyboard is showing already. The trailing-click guard is armed for the tap, as the input raises no focusin for it. Limited to iOS. The keyboard still does not come up by itself with the dialog, which takes a focus made during the tap.
CountHiddenInputFocusCalls replaced focus and blur of the hidden input with counting wrappers and left them there. It now returns a disposable that removes them, so they do not outlive the test that counts.
refocus arms the trailing-click guard from the last pointer type, so a focus from code after a touch arms it too, as the focusin listener does. Say that the next pointerdown is what bounds it.
|
Rebased on master after #26060, which moved iOS detection to |
c479f7c to
fcdde2a
Compare
|
Tick the box to add this pull request to the merge queue (same as
|
Takes Uno's servicing/6.8 from 6951c2c up to 5254ace (42 pull requests). Conflicts, by area: IME and TextBox (unoplatform#25984, backport of unoplatform#25804; unoplatform#25952, backport of unoplatform#25619; unoplatform#25976, backport of unoplatform#25818) - Takes upstream's composition design (unoplatform#25804): the browser's input is the source of truth for the text, compositions included. This drops the composition parts of the unoplatform#24502 port (90746b8): its TypeScript composition handling, the StartIndex/TextChangePending event arguments, the IsReadOnly forwarding to the hidden input, and its Given_TextBox.BrowserSoftKeyboard tests. - Keeps the non-IME browser fixes on top of it, in the form their master pull requests have on unoplatform#25804: Enter raised once per press and tracked by key code (unoplatform#25174, with Given_TextBox.BrowserEnterKey from master in place of the Enter tests that lived in the dropped file), the iOS refocus on tap (unoplatform#25185), pastes without text and isIOS from BrowserInputHelper. - Moves the unoplatform#26005 MaxLength fixes onto unoplatform#25804's native input path, as master has them: a refused browser edit keeps the TextBox's selection, TextBoxView.UpdateTextFromNative reports whether the edit went in, and the IME keeps its range on the applied text (master be85699, which d226897 had ported onto the unoplatform#24502 IME). Adds that commit's browser and platform-applied IME tests. - TextBox.pointers.skia.cs: upstream's file (unoplatform#25619 iOS touch editing, unoplatform#25818, unoplatform#25804), with the unoplatform#25126 port (the hold acts when it starts, a touch right-tap keeps the caret) in the form master's unoplatform#25126 branch has it on unoplatform#25619, including focusing before the iOS hold timer acts and its iOS not-focusable test. Pointers (unoplatform#25976, backport of unoplatform#25818) - Tapped and RightTapped are raised once the PointerReleased bubbled (unoplatform#25818), with the unoplatform#25732 port's window coordinates (2-argument routed event args, RawPosition for ContextRequested), as master's unoplatform#25732 branch has them. - The release wrapper of unoplatform#25818 keeps the unoplatform#25126 port's StopContextMenuTimer at the start of the release. DependencyObject (unoplatform#26142, backport of unoplatform#26123) - Takes upstream, then the code of unoplatform#25839 (97a2f17), which replaces this branch's unoplatform#25102 ports (800247f, be41da6, fd40bf9, 7546379). DependencyObjectStore.Binder.cs, .alc.cs, Application.Alc.cs, ResourceDictionary.alc.cs and their tests now match unoplatform#25839; this drops 800247f's own SingleParentNonFE_Parent_Removes_Value_Then_Next_Parent_Starts_Clean test, which unoplatform#25839 doesn't have. Imaging (unoplatform#25597, unoplatform#25915, unoplatform#25911, unoplatform#25897) - Keeps this branch's side: it has the later forms of these fixes (unoplatform#25124, unoplatform#25559, unoplatform#25633 and unoplatform#26370 ports), where upstream has the 6.8-only unoplatform#25597 design and earlier snapshots. Upstream's shared download, IgnoreImageCache and failed-load fixes are covered by SharedImageDecode. Adds upstream's When_Source_Outlives_Brushes test. ToolTip (unoplatform#26098, backport of unoplatform#25137 and unoplatform#26096) - Keeps this branch's unoplatform#25137 port and adds unoplatform#26096 in master's form: the service releases only a ToolTip it opened, and leaves one the app opened during the show delay to the app. Keeps both sides' tests. Flyout (unoplatform#26062, backport of unoplatform#26060) - This branch had the same commits already; keeps the "Keep non-resizable flyouts in window" port and its tests. Also removes a helper pair in Given_AccessibleTextBox that both sides added.
GitHub Issue: closes #22258, closes #25183
PR Type:
🐞 Bugfix
What changed? 🚀
By default iOS only shows the soft keyboard for an input that is focused while the user is interacting with the page (
-[WKContentView _elementDidFocus:userIsInteracting:...]). ATextBoxfocused from code at another time, such as the one a dialog opens on, gets no keyboard, and tapping it does not bring one up either: the tap lands on the canvas, and the hidden input, which has focus already, is not focused again.BrowserInvisibleTextBoxViewExtension.refocusnow focuses the hidden input again when aTextBoxis tapped while it has focus, and when a tap moves focus to aTextBoxthe input is handed over to. WebKit treatsfocus()on the focused element as a refocus (Element::focuscallselementDidRefocus), reports it to the UI process as long as no input view is showing for the element (WebPage::shouldDispatchUpdateAfterFocusingElement) with the user interacting when it is called from inside the tap, and does nothing when the keyboard is showing already.ITextBoxNotificationsProviderSingleton.OnFocused, which aTextBoxalready raises when it takes focus and when it is tapped while focused, synchronously inside the pointer event.focusinfor it, so the trailing-click guard is armed there for a touch or pen tap, as it is for the tap that focuses the input.MacIntelwith touch points) for iOS. Elsewhere it returns before touching the input.RichEditBox, which shares the hidden input but does not raise this notification, is not covered.An application built from the final head was compared with one built from
masteron iOS 26.5 in the iPhone 17 simulator (Safari; a simulator, not a physical device), tapped by hand:TextBoxfocused from code, tapped once: the keyboard comes up with the change. Onmasterit does not, and the focus visual and the caret go away.ContentDialog, focused when the dialog opens, tapped once: the same.TextBoxwhose keyboard was closed with Done, tapped again: the keyboard comes back on both. On iOS 26.5 Done takes the focus away from the input, so this tap is a first tap and does not tell the two apart.On an Android 15 emulator (Chrome 124), which the change leaves alone, the final head behaves as
master: after Back hides the keyboard, a tap on the field brings it up again. An earlier check on an iOS device, on a 6.8 build with a backport of this change, gave the same result for a tap on the focusedTextBox. What WebKit does with the refocus is read from its source.Why this is the fix for #22258
#22258 reports a
ContentDialogthat opens on a focusedTextBoxwith no keyboard, and asks for the keyboard to open with the dialog. There are two halves to it, and a page can only do something about one of them.The keyboard does not open with the dialog, and a page cannot make it. By default WebKit starts an input session for an element only when it takes focus while a touch is being dispatched, when another element already holds a session, or when a hardware keyboard is attached. The
TextBoxof a dialog takes focus once the dialog has loaded, which is after the tap that opened it is over, so iOS accepts the focus and shows no keyboard. That is WebKit's rule for every page that focuses a field from script. It is not something the hidden input gets wrong, and theTextBoxcannot take focus any earlier: it is not in the tree until the dialog is.Tapping the focused field did nothing, which is what left the user stuck. The tap on the field is the gesture iOS is waiting for, but it never reached the input: the only way to a keyboard was to tap another field and come back, as the issue describes. With this change the first tap on the field brings the keyboard up, which is what a web page gets on iOS for a field it focused from script.
One way around the first half was built and dropped. On every tap that leaves no
TextBoxfocused, it focused a hidden read-only input that asks for no keyboard, so that iOS had an input session going which aTextBoxfocused from code shortly afterwards took over, keyboard included. That focuses an input for taps that have nothing to do with text. Going by WebKit's source it brings up the browser's form accessory bar on an iPhone for each of them, and whether the session is handed over rests on timing inside WebKit that an application has no say in. It would put every tap of every application at risk to save one tap on a field.Tests
Given_TextBox(new fileGiven_TextBox.BrowserRefocusOnTap.cs), Skia WASM. The tests make the bridge take the browser for an iOS one and count the calls that focus or blur the hidden input:When_Focused_TextBox_Tapped_On_iOS_Then_Input_Focused_Again: a tap on aTextBoxthat was focused from code focuses the input again without blurring it, and so does a later tap, with the trailing-click guard armed.When_Another_TextBox_Tapped_On_iOS_Then_Input_Focused_Again: the same for the tap that moves focus from oneTextBoxto another.When_Focused_TextBox_Clicked_With_Mouse_On_iOS_Then_Next_Click_Left_Alone: a mouse click focuses the input again as well and leaves the guard alone.When_Focused_TextBox_Tapped_Off_iOS_Then_Input_Left_Alone: nothing is called off iOS.The first three fail on
masterwithout the change; the last one passes on it. They cover the bridge's side, not WebKit's, which is what the device check is for.Given_TextBox,Given_PasswordBoxandGiven_RichEditBoxpass in full on Skia WASM with the change (1323 tests, none failed). After the review changes,Given_TextBoxandGiven_PasswordBoxwere run again on Skia WASM: 200 passed, none failed, and 30 were inconclusive, the clipboard and pointer-hold tests that can't run in that setup.PR Checklist ✅
Screenshots Compare Test Runresults.