-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Fixes ios accessibility send focus request to an unfocusable node #25738
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -100,6 +100,29 @@ - (void)testPlainSemanticsObjectWithLabelHasStaticTextTrait { | |
XCTAssertEqual([object accessibilityTraits], UIAccessibilityTraitStaticText); | ||
} | ||
|
||
- (void)testNodeWithImplicitScrollIsAnAccessibilityElementWhenItisHidden { | ||
fml::WeakPtrFactory<flutter::AccessibilityBridgeIos> factory( | ||
new flutter::MockAccessibilityBridge()); | ||
fml::WeakPtr<flutter::AccessibilityBridgeIos> bridge = factory.GetWeakPtr(); | ||
flutter::SemanticsNode node; | ||
node.flags = static_cast<int32_t>(flutter::SemanticsFlags::kHasImplicitScrolling) | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these casts necessary? They should already be int32_t's. (fwiw I think people use unsigned ints for flags to avoid 2's compliment weirdness) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I got this error if i get rid of the static cast
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. weird, thanks |
||
static_cast<int32_t>(flutter::SemanticsFlags::kIsHidden); | ||
FlutterSemanticsObject* object = [[FlutterSemanticsObject alloc] initWithBridge:bridge uid:0]; | ||
[object setSemanticsNode:&node]; | ||
XCTAssertEqual(object.isAccessibilityElement, YES); | ||
} | ||
|
||
- (void)testNodeWithImplicitScrollIsNotAnAccessibilityElementWhenItisNotHidden { | ||
fml::WeakPtrFactory<flutter::AccessibilityBridgeIos> factory( | ||
new flutter::MockAccessibilityBridge()); | ||
fml::WeakPtr<flutter::AccessibilityBridgeIos> bridge = factory.GetWeakPtr(); | ||
flutter::SemanticsNode node; | ||
node.flags = static_cast<int32_t>(flutter::SemanticsFlags::kHasImplicitScrolling); | ||
FlutterSemanticsObject* object = [[FlutterSemanticsObject alloc] initWithBridge:bridge uid:0]; | ||
[object setSemanticsNode:&node]; | ||
XCTAssertEqual(object.isAccessibilityElement, NO); | ||
} | ||
|
||
- (void)testIntresetingSemanticsObjectWithLabelHasStaticTextTrait { | ||
fml::WeakPtrFactory<flutter::AccessibilityBridgeIos> factory( | ||
new flutter::MockAccessibilityBridge()); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -216,31 +216,14 @@ void PostAccessibilityNotification(UIAccessibilityNotifications notification, | |
} | ||
|
||
if (layoutChanged) { | ||
SemanticsObject* nextToFocus = nil; | ||
// This property will be -1 if the focus is outside of the flutter | ||
// application. In this case, we should not refocus anything. | ||
if (last_focused_semantics_object_id_ != kSemanticObjectIdInvalid) { | ||
// Tries to refocus the previous focused semantics object to avoid random jumps. | ||
nextToFocus = [objects_.get() objectForKey:@(last_focused_semantics_object_id_)]; | ||
if (!nextToFocus && root) { | ||
nextToFocus = FindFirstFocusable(root); | ||
} | ||
} | ||
ios_delegate_->PostAccessibilityNotification(UIAccessibilityLayoutChangedNotification, | ||
nextToFocus); | ||
FindNextFocusableIfNecessary()); | ||
} else if (scrollOccured) { | ||
// TODO(chunhtai): figure out what string to use for notification. At this | ||
// point, it is guarantee the previous focused object is still in the tree | ||
// so that we don't need to worry about focus lost. (e.g. "Screen 0 of 3") | ||
SemanticsObject* nextToFocus = nil; | ||
if (last_focused_semantics_object_id_ != kSemanticObjectIdInvalid) { | ||
nextToFocus = [objects_.get() objectForKey:@(last_focused_semantics_object_id_)]; | ||
if (!nextToFocus && root) { | ||
nextToFocus = FindFirstFocusable(root); | ||
} | ||
} | ||
ios_delegate_->PostAccessibilityNotification(UIAccessibilityPageScrolledNotification, | ||
nextToFocus); | ||
FindNextFocusableIfNecessary()); | ||
} | ||
} | ||
|
||
|
@@ -324,19 +307,34 @@ static bool DidFlagChange(const flutter::SemanticsNode& oldNode, | |
VisitObjectsRecursivelyAndRemove(child, doomed_uids); | ||
} | ||
|
||
SemanticsObject* AccessibilityBridge::FindFirstFocusable(SemanticsObject* object) { | ||
if (object.isAccessibilityElement) { | ||
return object; | ||
SemanticsObject* AccessibilityBridge::FindNextFocusableIfNecessary() { | ||
// This property will be -1 if the focus is outside of the flutter | ||
// application. In this case, we should not refocus anything. | ||
if (last_focused_semantics_object_id_ == kSemanticObjectIdInvalid) { | ||
return nil; | ||
} | ||
|
||
// Tries to refocus the previous focused semantics object to avoid random jumps. | ||
return FindFirstFocusable([objects_.get() objectForKey:@(last_focused_semantics_object_id_)]); | ||
} | ||
|
||
SemanticsObject* AccessibilityBridge::FindFirstFocusable(SemanticsObject* parent) { | ||
SemanticsObject* currentObject = parent ?: objects_.get()[@(kRootNodeId)]; | ||
; | ||
if (!currentObject) | ||
return nil; | ||
|
||
if (currentObject.isAccessibilityElement) { | ||
return currentObject; | ||
} | ||
|
||
SemanticsObject* candidate = nil; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure what had got into my head when i wrote this code. The pr change it to be more readable without changing the functionality |
||
for (SemanticsObject* child in [object children]) { | ||
for (SemanticsObject* child in [currentObject children]) { | ||
SemanticsObject* candidate = FindFirstFocusable(child); | ||
if (candidate) { | ||
break; | ||
return candidate; | ||
} | ||
candidate = FindFirstFocusable(child); | ||
} | ||
return candidate; | ||
return nil; | ||
} | ||
|
||
void AccessibilityBridge::HandleEvent(NSDictionary<NSString*, id>* annotatedEvent) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was a typo.