Repository navigation
fix(ios): Duo tab selection, sheet bars, custom titles, search freeze and unfold crash (5.0.0-t3.31–t3.36) - #3
Merged
Merged
Conversation
…nsactions (software-mansion#4862) ## Description On the `UITab` API path (iOS 26.1+), once a tab with `role: 'search'` has been activated (`automaticallyActivatesSearch` with a search bar in the nested stack header), later user selections of that tab can be dropped: UIKit shows the search tab selected, but `onTabSelected` is never sent and JS stays on the previous tab. `tabBarController:didSelectTab:previousTab:` fires for both user and programmatic selections, so user selections were detected with a flag set in `tabBarController:shouldSelectTab:` and consumed in `didSelectTab:`. Every container update (`-performContainerUpdate`, run after each Fabric mount transaction) cleared that flag. On iOS 27, once search has been activated, UIKit lets a mount transaction run between `shouldSelectTab:` and `didSelectTab:` - the flag is cleared and the user selection is treated as programmatic. This PR inverts the filter: the controller marks its own `setTabs:` / `selectedTab` writes - UIKit sends `didSelectTab:` synchronously from inside them (verified at runtime and in the UIKitCore disassembly on iOS 26.1 and 27.0) - and treats every other `didSelectTab:` as a user selection. No flag stays set across the asynchronous should -> did window anymore. Reported in software-mansion#4855, with the first fix proposed in software-mansion#4856, by @zhelezkov - thank you! The scenario steps and the "Selected in JS" readout of the v4 test come from software-mansion#4856. Closes software-mansion#4855. ## Changes - `RNSTabBarController`: replace `_isHandlingUserTabSelection` (set in `shouldSelectTab:`, cleared on every container update) with `_isApplyingProgrammaticTabSelection`, set around the controller's own `setTabs:` / `selectedTab` writes and checked in `didSelectTab:`. - `test-tabs-search-tab-activation-ios` (Stack v4 & tabs): JS-side selection readout on the List screen, scenario steps for returning through the collapsed tab bar, and an e2e test for that flow. - New `test-tabs-navigation-state-search-tab` (tabs): verifies that each user and programmatic selection of a search-role tab advances the navigation state exactly once with the right action origin, with an e2e test. It hosts no search controller, so it does not reproduce software-mansion#4855 itself - it guards the programmatic-selection filtering this fix relies on. ## Test plan e2e (Detox, `ios.sim.release`, iPhone 17): - `FabricExample/e2e/component-integration-tests/tabs-stack-v4/test-tabs-search-tab-activation-ios.e2e.ts` - iOS 27.0: fails without the fix ("Selected in JS: no" after reselecting the search tab), passes with it. - iOS 26.5: passes with and without the fix - the race was not observed on iOS 26.x, so CI (iOS 26.x for now) does not guard it until it moves to iOS 27. - `FabricExample/e2e/single-feature-tests/tabs/test-tabs-navigation-state-search-tab.e2e.ts` - passes with the fix on iOS 27.0 and 26.5. - `FabricExample/e2e/single-feature-tests/tabs/test-tabs-simple-nav.e2e.ts` - passes with the fix on iOS 27.0 and 26.5. Manual, `test-tabs-search-tab-activation-ios` (Component Integration Tests -> Stack V4 & Native Tabs), iOS 27.0 simulator, without the fix: 1. Enable `automaticallyActivatesSearch`. 2. Tap the search tab, dismiss the keyboard by dragging the list, tap the collapsed `Config` button. 3. Tap the search tab again and check "Selected in JS" on the List screen. Without the fix 5 of 5 reselections showed "Selected in JS: no"; with the fix the e2e loop above passes. ## Checklist - [x] Included code example that can be used to test this change. - [ ] For visual changes, included screenshots / GIFs / recordings documenting the change. - [ ] For API changes, updated relevant public types. - [ ] Ensured that CI passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Nick Zhelezkov <5163918+zhelezkov@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 6430b67)
…on while More is active (software-mansion#4890) (generated by Claude) ## Description On the `UITab` API path (iOS 26.1+) a programmatic tab selection made while the More tab is active emits `onTabSelected` twice: a `user`-origin event first, then the `programmatic-js` one, and the navigation state provenance skips a value. It happens when the current tab overflows into More and was itself selected programmatically (e.g. `Select Sixth` from JS, then `Select Second`). Entering More through the More list and then selecting programmatically gives a single event. Found during the 4.30.0 release testing (software-mansion#4884), regression from software-mansion#4862; 4.29.0 and 4.28.0 emit one event. Root cause: `-[RNSTabBarController applySelectedScreenController:]` sets `_isApplyingProgrammaticTabSelection` only around `self.selectedTab = …`, the branch taken when the More item is NOT selected. While More is selected it falls back to `[self setSelectedViewController:]` without the flag. UIKit sends `tabBarController:didSelectTab:previousTab:` synchronously from inside that write, and since software-mansion#4862 every unflagged `didSelectTab:` is treated as a user selection: `userDidSelectViewController:` progresses the state once more and emits a `user` event, after which the programmatic path emits its own event with the already-bumped provenance. Before software-mansion#4862 the filter worked the other way round (flag set in `shouldSelectTab:`), so the programmatic `setSelectedViewController:` write was ignored. ## Changes - `RNSTabBarController -applySelectedScreenController:`: `_isApplyingProgrammaticTabSelection` now wraps the whole `UITab` branch, including the `setSelectedViewController:` fallback used while More is active. Same pattern as `installScreenControllers:` around `setTabs:` / `selectedTab`. ## Before & after - visual documentation | Before | After | | --- | --- | | <video src="https://github.com/user-attachments/assets/3336fc09-d892-4436-acd4-9b6d8fef900a" /> | <video src="https://github.com/user-attachments/assets/a3e3e738-3a59-4afb-bec5-d10ccdeac7b6" /> | ## Test plan - `tabs/test-tabs-more-navigation-controller-ios` (SFT), iPhone simulator, iOS 27.0 and 26.5: `Select Sixth`, then `Select Second`. Before: two `onTabSelected` toasts for `Second`; after: one, with the provenance advanced by exactly one (check the `onTabSelected` payload). Also More → Fifth → `Select First` (one event, unchanged) and user taps between More and regular tabs (unchanged). - `tabs-stack-v4/test-tabs-search-tab-activation-ios` on iOS 27.0, the software-mansion#4862 scenario: reselecting the search tab through the collapsed tab bar still shows "Selected in JS: yes". - `tabs/test-tabs-navigation-state-search-tab` and `tabs/test-tabs-simple-nav` e2e: unchanged. Draft: the root cause comes from the triage done for software-mansion#4884 (A/B against 4.28.0 / 4.29.0 / 4.30 with the same app and only the library sources swapped), the patch builds (Debug) but has not been run against the scenario yet. ## Checklist - [x] Included code example that can be used to test this change (`tabs/test-tabs-more-navigation-controller-ios`). - [ ] For visual changes, included screenshots / GIFs / recordings documenting the change. - [ ] For API changes, updated relevant public types. - [ ] Ensured that CI passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 4a87a71)
fix(ios): make horizontal bars configurable for Duo sheets
Brings upstream's fix for user tab selections dropped by a mount transaction (software-mansion#4862, software-mansion#4890) and the opt-in horizontal sheet bars from #2. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A screen that returned to the top of a collapsed split column while its tab was hidden kept its custom title view, but the navigation bar showed an empty title in its place: the view had no superview or window. After the screen appears, the header coordinator sets that view again, which makes the bar host it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
UIKit's search text field registers a main-queue observer for its text storage's edits, filtered by the storage's address, and never removes it. The search bar replaces its field whenever the search placement changes, and every dismissed screen with a search frees one. React Native's JS thread then creates measurement text storages, sometimes at a freed address. An edit to such a storage synchronously waits for the main thread while React Native holds its text measurement lock, and a main thread that measures text at that moment deadlocks. On the iPhone Duo this froze the app during folds. An observer filtered by a freed object can never fire for that object, so RNSSearchBar now removes main-queue text storage observers as their storage is freed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The search field observer fix removed every queued observer registered for any notification on any text storage, whoever registered it, and the remover kept a strong reference to the observer. It now acts only on UIKit's own registration: the default center, the main queue, NSTextStorageDidProcessEditingNotification, and the block function UIKit registers while RNSSearchBar creates its first search controller, checked to live in UIKit. Fields UIKit creates later for placement changes register through the same function. The remover holds the observer weakly, so it adds no reference to UIKit's observer and no cycle through the observer's block. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
software-mansion#4990) ## Description Closes software-mansion#4989. On an iPhone Duo cold-launched unfolded, `TabsHost` comes up without its tab bar: the side rail is never shown. A folded launch is fine, and folding then unfolding brings the tab bar back. Cause: `RNSTabBarController`'s view is loaded while the controller is still detached, at the main screen's size (on the Duo launched unfolded, the cover display's 466x678), and the tabs are installed at that size. `-[RNSTabsHostComponentView didMoveToWindow]` then adds the view to the host and pins it with constraints without setting a frame. The later resize to the host (951x669) doesn't make UIKit rebuild the tab bar, so the rail tab bar never appears. The repro repo has a single-file UIKit app without React Native that shows the same thing (0/3 stock, 3/3 with the frame set before `addSubview`), so this is a UIKit issue that the mount order triggers, and setting the frame first is a safe workaround. There's no OS or device guard: this is a UIKit ordering issue reproducible in plain UIKit, not a version regression, and the frame assignment is a no-op whenever the sizes already match. The same `didMoveToWindow` code is in 4.30.0 and on `v4-main`, so this is probably worth a v4 backport. ## Changes - `-[RNSTabsHostComponentView didMoveToWindow]`: set `_controller.view.frame = self.bounds` before attaching the controller and activating the constraints. - `Test4989`: three tabs in a `TabsContainer`, to be rendered directly from `apps/App.tsx` and cold-launched on the Duo while unfolded. ## Before & after - visual documentation iPhone Duo simulator, unfolded cold launch, inner display: | | Before | After | | --- | --- | --- | | FabricExample `Test4989` |  |  | | 4.30.0 |  |  | | 5.0.0-alpha.4 |  |  | ## Test plan FabricExample, Release build, iPhone Duo simulator (Xcode 27.1, iOS 27.1), `Test4989` rendered directly from `apps/App.tsx`: 1. Unfold the device, quit the app, cold-launch it. 2. The side rail tab bar with First, Second and Third should be on the inner display. Unfolded cold launches with the rail tab bar shown: 0/3 without the fix, 3/3 with it. A folded cold launch and fold then unfold show the tab bar with and without the fix. Regression check, cold launch of each scenario with and without the fix (FabricExample Release, iPhone 18 Pro and iPad Pro 11" on iOS 27.0, iPhone Duo folded and unfolded): `test-tabs-tab-bar-controller-mode-ios`, `test-tabs-layout-appearances-ios`, `test-tabs-bottom-accessory-layout-ios`, `test-tabs-simple-nav`, `test-stack-tabs-tab-bar-sidebar-preferred-placement-stack-in-tabs-ios`. Identical screens on iPhone 18 Pro, iPad and the folded Duo. On the unfolded Duo the fix adds the missing rail tab bar in four of them; `test-tabs-bottom-accessory-layout-ios` already showed it without the fix and is unchanged. The same fix was also checked in an Expo app: https://github.com/nkt/rns-duo-tabs-repro (Expo SDK 58, RN 0.88, expo-router `NativeTabs`, Release), with 4.30.0 and 5.0.0-alpha.4 each going from 0/3 to 3/3 unfolded cold launches with the rail tab bar. ## Checklist - [x] Included code example that can be used to test this change (`Test4989`). - [x] For visual changes, included screenshots / GIFs / recordings documenting the change. - [ ] For API changes, updated relevant public types. - [ ] Ensured that CI passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_0164Zdw6abcSU2WKSirxHNb4 --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 06656d7)
…g header shadow state (software-mansion#4851) ## Description `-[RNSScreenStackHeaderConfig updateHeaderStateInShadowTreeInContextOfNavigationBar:]` loops over `self.reactSubviews` and updates the shadow state of each header subview. Since software-mansion#3933 (4.19.0) that update is synchronous, so it can commit and mount right away, and the mount includes any React commit that's still pending. If that commit adds or removes a header subview, the array changes while we're still looping over it, and iOS throws `NSGenericException: Collection was mutated while being enumerated`. Fast enumeration only detects the change when it moves on to the next element, so it only crashes when another header subview comes after the one being updated. In practice the item that commits is a left or right item that was just mounted. With native-stack, the only subview mounted after the right items is the search bar, so there it takes custom header items that change often plus `headerSearchBarOptions`. With `ScreenStackItem` the order of the subviews is up to the app, and any subview after the item is enough. Closes software-mansion#4811. ## Changes - Loop over a copy of `reactSubviews` in `updateHeaderStateInShadowTreeInContextOfNavigationBar:`. - Add `Test4811`, which reproduces the crash. I only changed this one loop on purpose. The other `reactSubviews` loops in the file (`hitTest:withEvent:`, `hasSubviewOfType:` and `loadBackButtonImageInViewController:`) only read values and don't call anything that can commit, so they can't hit this. Happy to change them too if you'd rather be defensive everywhere. Copying the array is cheap, since a header only has a handful of subviews. ## Test plan `Test4811`: press "Start". The screen commits as fast as possible. The left and right header items are mounted and unmounted at different rates, and the title is placed after the right item. Tested in FabricExample on `main` (iOS 26.5 simulator): - Without the fix: it crashed right after pressing "Start" in 5 out of 5 runs, with the same stack as in the issue. - With the fix: no crash in a 60-second run (about 4,500 updates), and the header items keep rendering correctly. I also checked the native-stack case with a search bar on `main`: without the fix it crashed in 5 out of 5 runs within 6 seconds, with the fix there were no crashes in 3 runs of 60 seconds. ## Checklist - [x] Included code example that can be used to test this change. - [ ] For visual changes, included screenshots / GIFs / recordings documenting the change. - [ ] For API changes, updated relevant public types. - [ ] Ensured that CI passes --------- Co-authored-by: ilp0 <ilpo@0m.fi> (cherry picked from commit 979b511)
…d stack is recycled (software-mansion#4997) ## Description When a native stack nested in a modal is recycled and then reused, its screen appears before the modal has finished presenting. UIKit doesn't forward appearance to a child view controller that is added after its parent appeared while a transition is still running, so such a child never appears and never gets a safe area. This breaks `Host` from `@expo/ui` in expo/expo#51288: from the second `fullScreenModal` presentation of a screen with a nested stack, a `Host` that mounts during the presentation lays out its content under the status bar and the large title. ### Cause - `prepareForRecycle` removes `_controller` from its parent view controller but leaves `_controller.view` inside the stack view. - When the recycled stack is reused, its view enters the window with the navigation controller's view already inside and no parent controller. UIKit treats it as a standalone controller whose view entered a window and makes it appear right away (`viewDidMoveToWindow:shouldAppearOrDisappear:`), while the modal is still animating. `maybeAddToParentAndUpdateContainer` adds it to the parent only afterwards. - On first use, `reactAddControllerToClosestParent` adds the controller to its parent before adding its view, so it appears with the modal when the presentation finishes. Breakpoints on `-[RNSScreen viewDidAppear:]` confirm this: on the first presentation it's called from `-[UIPresentationController transitionDidFinish:]`, on later ones from the window-move path above, before the modal finishes. ## Changes - `prepareForRecycle` also removes `_controller.view`, so a reused stack attaches the controller before its view, as on first use. ## Test plan Repro: https://github.com/brentvatne/expo-ui-host-safe-area-repro (expo-router `fullScreenModal` routes with a nested `Stack`, and a SwiftUI `Host` that mounts 400 ms after the screen). Open A, close, open B, close, open A. - Before (4.30.0): from the second presentation on, the hosted list starts under the status bar and the title. - After (4.30.0 with this change): the list starts below the title on every presentation. Tested on an iOS 27.0 simulator (Xcode 27.1). The changed code is the same on `main`. ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Updated documentation - [ ] Ensured that CI passes --------- Co-authored-by: expo-tuft[bot] <288127324+expo-tuft[bot]@users.noreply.github.com> Co-authored-by: intergalacticspacehighway <23293248+intergalacticspacehighway@users.noreply.github.com> (cherry picked from commit d267a1a)
…ndler On an iPhone Duo, unfolding right after presenting and dismissing sheets could abort debug builds. UIKit re-parents the presented screens during the split transition, and -[RNSScreenView didMoveToWindow] detached the screen's RCTSurfaceTouchHandler even when it was no longer attached, so RCTSurfaceTouchHandler's "attached to another view" assertion failed. A screen that moves straight from one window to another would likewise attach a second time and fail the "already has attached view" assertion. Both screen types that own a touch handler, legacy screens and split inspector columns, now attach it only when it is not attached and detach it only when it is attached to them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
juliusmarminge
changed the title
This was referenced Oct 21, 2026
juliusmarminge
marked this pull request as ready for review
October 21, 2026 23:52
The replacement for addObserverForName:object:queue:usingBlock: became callable from other threads the moment method_setImplementation installed it, before its return value filled in the original implementation. A notification registration on another thread in that window called through a null function pointer. The original is now read first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| int reserved; | ||
| void *invoke; | ||
| }; | ||
| return ((__bridge struct RNSBlockLayout *)block)->invoke; |
There was a problem hiding this comment.
🟠 High legacy/RNSSearchBar.mm:68
On arm64e, UIKit’s same callback compares unequal when its invoke pointer comes from blocks at different addresses, so later search-field storages do not get a remover and retain the stale observers this workaround is meant to remove. RNSBlockInvoke preserves the address-diverse authentication bits; normalize the invoke pointer before caching and comparing it.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @ios/legacy/RNSSearchBar.mm around line 68:
On arm64e, UIKit’s same callback compares unequal when its invoke pointer comes from blocks at different addresses, so later search-field storages do not get a remover and retain the stale observers this workaround is meant to remove. `RNSBlockInvoke` preserves the address-diverse authentication bits; normalize the invoke pointer before caching and comparing it.
Member
Author
There was a problem hiding this comment.
Valid. On arm64e, UIKit blocks carry address-diversified signed invoke pointers, so comparing raw invoke values can miss later search fields, and dladdr on a signed pointer can fail. The workaround would then do nothing on device; it would not crash. The follow-up release strips the signature (ptrauth_strip(..., ptrauth_key_function_pointer)) before caching and comparing.
juliusmarminge
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
T3 Code's iPhone Duo PRs vendor react-native-screens tarballs 5.0.0-t3.31 through t3.36, but those tarballs were built from commits that only existed in a local clone. This puts that source on GitHub and gives it one PR to merge when the Duo changes land in T3 Code.
What each release adds on top of
t3/main(5.0.0-t3.30):-[NSNotificationCenter addObserverForName:object:queue:usingBlock:]process-wide.-[RNSScreenView didMoveToWindow]detached a handler that was no longer attached, andRCTSurfaceTouchHandlerasserts on that.-x:NSNotificationCenter's original implementation before installing itself. A registration on another thread in between used to call through a null pointer.Which T3 Code branches use which tarball:
Each vendored tarball's
t3-fork.jsonnames its source commit on this branch. Every tracked source file in each tarball (889 of them) is byte-identical to that commit. Each release is also taggedt3-v5.0.0-t3.Nand has at3/release-t3.Nbranch.Merge this together with T3 Code's Duo stack. A merge commit or fast-forward keeps the recorded commits on
t3/main. The release branches and tags pin each tarball's source either way. Merging this also lands #2.Prepared with Claude Opus 5.5 in Claude Code, running in T3 Code.
🤖 Generated with Claude Code
Note
Fix iOS Duo tab selection, sheet bars, custom titles, search freeze and unfold crash
RNSTabBarControllernow tracks programmatic-selection writes separately, so UITab selections admitted by UIKit forward as user selections and programmatic writes no longer emit duplicate events. Also sizes the tabs controller view to host bounds on mount.sheetPrefersHorizontalBarsprop (default false, iOS 27.1+) toScreen,FormSheet, andFormSheetHosttypes, disabling vertical bars for modal, page-sheet, and form-sheet presentations. Android managers expose matching no-op setters.RNSSearchBarswizzles block-basedNSNotificationCenterobserver registration to track UIKit text-storage editing observers and remove them when the text storage is deallocated.RNSHeaderCoordinator.restoreDetachedTitleView, called on stack-screen and split-screen appearance. Also fixes touch-handler attach/detach logic inRNSScreen.didMoveToWindowand split screens, and removes the stack controller view before parent-controller removal.RNSSearchBar.initializeswizzles the process-wide block-based observer registration path on non-TV targets; the swizzled path forwards to the original implementation for non-matching observers. AlsoRNSScreenStackHeaderConfigiterates a copy ofreactSubviewsduring shadow-state updates.Macroscope summarized 8edcc5c.