Repository navigation
[stable] [iOS] Preserve semantics parents after reparenting (#189686) + Fix OverlayPortal semantics bounds corruption (#192139) - #193420
Conversation
Fixes flutter#100946. ## Problem Reordering a `ReorderableListView` on iOS could move VoiceOver focus to the upper-left corner of the screen. The affected items could no longer be focused at their visual positions. During a reorder, semantics children move through temporary containers used by the drag overlay and drop animation. A child may already have been attached to its replacement `SemanticsObject` when its previous parent is updated or deallocated. The previous parent unconditionally cleared the child's `parent` property. This overwrote the new parent relationship and disconnected the child from the native semantics hierarchy. `SemanticsObject` calculates its global accessibility frame by walking this parent chain and composing each ancestor's transform. Once the chain was disconnected, the element retained only its local physical-pixel rectangle. On a 3x simulator, the expected frame: ``` {{0, 286}, {402, 56}} ``` became: ``` {{0, 0}, {134, 18.67}} ``` This accounts for both the missing translation and the frame being reduced by the device pixel ratio. ## Fix Only clear a child's parent when the `SemanticsObject` performing cleanup still owns that parent relationship: ```objc child.parent == self ``` This guard is applied when: - A `SemanticsObject` is deallocated. - Its traversal-order children are replaced. - Its hit-test-order children are replaced. A previous parent can now release its stale child references without detaching a child that has already moved under a replacement container. The complete ancestor transform remains available when calculating the native accessibility frame. ## Testing Added An iOS XCUITest that performs the default mobile long-press reorder gesture, verifies the resulting native semantic traversal order, and asserts that the reordered item retains the expected accessibility frame. The integration app also includes a deterministic direct-drag fixture for exercising XCUITest drag synthesis. Validation performed: - `testReorderableListAccessibilityHierarchyUpdates` passes with the locally built engine. - `packages/flutter/test/widgets/reorderable_list_test.dart` passes. - `packages/flutter/test/material/reorderable_list_test.dart` passes. - `ios_test_flutter` builds successfully. - Engine formatting and Dart analysis pass. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. --------- Co-authored-by: LongCatIsLooong <[email protected]> (cherry picked from commit 03ebdcd)
fixes flutter#182604 Historically, the configuration/order of children and parents for widgets is identical across both traversal and hit test order. However, with introduction over OverlayPortal, flutter@fccfa97, this is no longer always the case. For example when traversing, the actual portal child, eg, a floating drop down box, is a child of the `OverlayPortal`. However when hit testing, the portal child is a child of the `Overlay`. Prior to this pr, both setChildrenInHitTestOrder and setChildren have identical implementations. ``` for (SemanticsObject* child in _childrenInHitTestOrder) { if (child.parent == self) { child.parent = nil; } } _childrenInHitTestOrder = [childrenInHitTestOrder copy]; for (SemanticsObject* child in _childrenInHitTestOrder) { child.parent = self; } ``` On every frame, setChildren and setChildrenInHitTestOrder is run sequentially on every widget. So first, when setChildren is run for `OverlayPortal`, it sets `portalChild.parent = overlayPortal`. This is the correct traversal order. Then, sometime later, it runs setChildrenInHitTestOrder for `Overlay`, which sets portalChild.parent = overlay`. This is incorrect. This is why in the bug the accessibility nodes still exist, just weirdly offset - because its now anchored to 0, 0 of `Overlay` instead of the the position over `OverlayPortal`. Because hittest order strictly walks top down (parent isnt ever accessed), we don't actually have to do any mutation to the parents. https://github.com/user-attachments/assets/df093797-7482-42ba-ba4c-b9a172484031 (cherry picked from commit 20c755d)
|
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
github-actions
Bot
added
platform-ios
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies iOS platform view semantics handling to prevent an old parent from detaching a reparented child, and introduces integration tests for reorderable list semantics and dragging. The review feedback correctly identifies a potential runtime crash in the Objective-C test helper if element.label is nil, as well as logic errors in the Dart reordering implementations where newIndex is not decremented when oldIndex < newIndex.
| static NSArray<NSString *> *ElementLabels(XCUIElementQuery *query) { | ||
| NSMutableArray<NSString *> *labels = [[NSMutableArray alloc] init]; | ||
| for (XCUIElement *element in query.allElementsBoundByIndex) { | ||
| [labels addObject:element.label]; | ||
| } | ||
| return labels; | ||
| } |
There was a problem hiding this comment.
The element.label property can potentially return nil. Attempting to add a nil object to an NSMutableArray will cause a runtime crash. To prevent this, check if the label is non-nil before adding it to the array.
static NSArray<NSString *> *ElementLabels(XCUIElementQuery *query) {
NSMutableArray<NSString *> *labels = [[NSMutableArray alloc] init];
for (XCUIElement *element in query.allElementsBoundByIndex) {
NSString *label = element.label;
if (label) {
[labels addObject:label];
}
}
return labels;
}| void _onReorderItem(int oldIndex, int newIndex) { | ||
| setState(() { | ||
| final int item = items.removeAt(oldIndex); | ||
| items.insert(newIndex, item); | ||
| }); | ||
| } |
There was a problem hiding this comment.
When reordering items in a list, if oldIndex < newIndex, the newIndex is reported as if the item has already been removed, which shifts the remaining items' indices down by 1. To avoid out-of-bounds or incorrect insertions, you must decrement newIndex by 1 when oldIndex < newIndex.
void _onReorderItem(int oldIndex, int newIndex) {
setState(() {
if (oldIndex < newIndex) {
newIndex -= 1;
}
final int item = items.removeAt(oldIndex);
items.insert(newIndex, item);
});
}| onReorderItem: (int oldIndex, int newIndex) { | ||
| setState(() { | ||
| _items.insert(newIndex, _items.removeAt(oldIndex)); | ||
| }); | ||
| }, |
There was a problem hiding this comment.
When reordering items in a list, if oldIndex < newIndex, the newIndex is reported as if the item has already been removed, which shifts the remaining items' indices down by 1. To avoid out-of-bounds or incorrect insertions, you must decrement newIndex by 1 when oldIndex < newIndex.
onReorderItem: (int oldIndex, int newIndex) {
setState(() {
if (oldIndex < newIndex) {
newIndex -= 1;
}
_items.insert(newIndex, _items.removeAt(oldIndex));
});
},
|
@googlebot I signed it! |
cbracken
added
cp: beta
|
Thanks — all three suggestions are on files that this PR does not modify relative to the commit being cherry-picked, so I'm leaving them as-is. Verifiable:
Two of the three would also be incorrect:
Both constructors also assert that exactly one of the two is provided (
|
|
Gentle ping on cherry-pick triage. Per
On the acceptance bar — the doc scopes this to "regressions from the previous release" that are "highly impactful and critical":
3.47.6 shipped on 2026-09-30, two days after this was opened, so it missed that hotfix. Happy to retarget or to split the two picks into separate PRs if either makes this easier to take. |
|
@JoX23 Do you have a changelog description for this? https://github.com/flutter/flutter/blob/main/docs/releases/Hotfix-Documentation-Best-Practices.md |
|
@justinmc Added under Changelog Description in the PR description: flutter/182604 When a route shows an |
|
Thank you! Let's land this. |
justinmc
added
the
autosubmit
deb2874
into
flutter:flutter-3.47-candidate.0
6 participants
Cherry-picks #189686 and #192139 into
flutter-3.47-candidate.0(stable). Both applied cleanly withgit cherry-pick -x; both touch onlySemanticsObject.mm(+ tests).Issue Link
#182604 · #100946
Impacted Users
End users of iOS apps built with stable who rely on VoiceOver / Switch Control, and teams driving XCUITest / Maestro against Flutter apps. Any route that mounts a Material
Slideris affected out of the box:Sliderkeeps its value-indicatorOverlayPortalshown from construction (slider.dart:OverlayPortalController(...)..show()), so no popover, menu or reorder is needed to trigger it.Impact Description
Since fccfa97 an
OverlayPortalchild has a different parent in traversal order and in hit-test order. The iOS accessibility bridge overwrote the nativeSemanticsObject.parentwith the hit-test one (and cleared parents of moved children), so the parent chain used to computeaccessibilityFrameno longer reaches the root: every element of the route reports a frame scaled by 1/devicePixelRatio (exactly 1/3 on a 3x device) and anchored top-left. VoiceOver touch exploration focuses the wrong place;XCUIElement.tap()misses the visible control. Measured on iPhone 16 Pro Max / iOS 26.7 with Flutter 3.44.8 (stable 3.47.5 has the sameSemanticsObject.mm): a full-width CTA laid out at{{32, 834}, {376, 56}}is published as{{10.7, 278.0}, {125.3, 18.7}}; a player route with aSlideris 1/3 from its first frame. The corruption persists after the portal unmounts and after background/foreground.Changelog Description
flutter/182604 When a route shows an
OverlayPortalon iOS (including any MaterialSlider), accessibility frames are shrunk by the device pixel ratio and anchored to the top-left, so VoiceOver focus and XCUITest taps miss the visible controls.Workaround
Replace Material
SliderwithCupertinoSlider(noOverlayPortal) on affected routes and avoidOverlayPortalaltogether. Not viable for apps using Material menus / dropdowns or third-party popovers.Risk
Low. Parent bookkeeping only: #189686 clears
child.parentonly when the object still owns it; #192139 stops the hit-test-order setter from mutatingparent. No behaviour change when traversal and hit-test parents coincide.Cherry-pick diff composition
The two upstream PRs touch the same function, so the composed diff on
SemanticsObject.mm(+6 -8) matches neither of them individually — upstream, #189686 is+9 -3on this file and #192139 is+0 -8. Net against theflutter-3.47-candidate.0base:dealloc+3 -1— guard becomesif (child.parent == self) { child.parent = nil; }setChildren+3 -1— same guardsetChildrenInHitTestOrder-6— #189686 adds the guard here too, then #192139 deletes both parent loops outright, so those 3 added lines never survive and the block's 6 original lines go awayAdditions
3 + 3 = 6; deletions1 + 1 + 6 = 8. The three "missing" additions are the ones the second commit removes from the first. No conflict was resolved by hand — both applied cleanly withgit cherry-pick -x.Test Coverage
Yes. #189686 adds an iOS XCUITest (
testReorderableListAccessibilityHierarchyUpdates) asserting the accessibility frame after reparenting; #192139 addsSemanticsObjectTest.mmcoverage. Both have been on master since 2026-09-01/03 and ship in beta 3.49.0-0.1.pre.Validation Steps
MaterialApp.router+GoRoute) whose body contains a MaterialSlider.XCUIApplication().debugDescriptionfrom an XCUITest, or inspect with Accessibility Inspector.XCUIElement.tap()/ VoiceOver touch exploration hit the visible controls.