Skip to content

Commit ba5a8d6

Browse files
authored
Fix NPE in accessibleParentOf during onNodeRemoved (JetBrains#2803)
1 parent 62c8e55 commit ba5a8d6

3 files changed

Lines changed: 142 additions & 47 deletions

File tree

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/a11y/Accessibility.desktop.kt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ internal class AccessibleFocusHelper(
4949
resetFocusAccessibleJob?.cancel()
5050
if (focusedAccessible != null) {
5151
resetFocusAccessibleJob = GlobalScope.launch(MainUIDispatcher) {
52-
delay(100)
52+
delay(RESET_FOCUS_ACCESSIBLE_DELAY)
5353
focusedAccessible = null
5454
}
5555
}
@@ -83,6 +83,10 @@ internal class AccessibleFocusHelper(
8383
fun dispose() {
8484
resetFocusAccessibleJob?.cancel()
8585
}
86+
87+
companion object {
88+
const val RESET_FOCUS_ACCESSIBLE_DELAY = 100L
89+
}
8690
}
8791

8892
/**

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/a11y/SemanticsOwnerAccessibility.kt

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,10 @@ internal class SemanticsOwnerAccessibility(
103103
* Returns the [Accessible] parent of the given [ComposeAccessible].
104104
*/
105105
fun accessibleParentOf(accessible: ComposeAccessible): Accessible? {
106+
// This can happen during onNodeRemoved. When the property change listeners are called, they
107+
// can call `accessible.getAccessibleParent()`.
108+
if (accessible.semanticsNode.id !in accessibleByNodeId) return null
109+
106110
sceneAccessibility.accessibleParentOverride(accessible)?.let { return it }
107111

108112
val parentNode = accessible.semanticsNode.parent ?: return sceneAccessibility.accessible()
@@ -150,27 +154,31 @@ internal class SemanticsOwnerAccessibility(
150154
* Invoked when a [ComposeAccessible] is removed.
151155
*/
152156
private fun onNodeRemoved(accessible: ComposeAccessible) {
153-
if (accessible.composeAccessibleContext.focused == true) {
157+
val accessibleContext = accessible.composeAccessibleContext
158+
if (accessibleContext.focused == true) {
154159
notifyOnFocusLost(accessible)
155160

156161
// Testing showed that when focus is transferred manually when a node is removed (e.g.,
157162
// via FocusRequester), the removed node doesn't report it's focused at this point, but
158163
// just in case, check that no other nodes are focused before calling
159164
// onFocusReceived(null)
160-
val anyNodeFocused = accessibleByNodeId.any { _, accessible ->
161-
accessible.composeAccessibleContext.focused == true
165+
val anyNodeFocused = accessibleByNodeId.any { _, a ->
166+
a.composeAccessibleContext.focused == true
162167
}
163168
if (!anyNodeFocused) {
164169
onFocusReceived(null)
165170
}
166171
}
167-
if (accessible.composeAccessibleContext.isVisible) {
168-
accessible.accessibleContext?.firePropertyChange(
172+
if (accessibleContext.isVisible) {
173+
accessibleContext.firePropertyChange(
169174
ACCESSIBLE_STATE_PROPERTY,
170175
AccessibleState.VISIBLE, null
171176
)
172177
}
173178

179+
// dispose() can only be called after the code above because after dispose(), the
180+
// accessible.accessibleContext returns `null`, but the accessibility system can call it
181+
// in the property change listener(s)
174182
accessible.dispose()
175183
}
176184

@@ -361,7 +369,7 @@ internal class SemanticsOwnerAccessibility(
361369
it.contains(SemanticsProperties.HideFromAccessibility)
362370
}
363371

364-
// Build new mapping of ComposeAccessible by node id
372+
// Build a new mapping of ComposeAccessible by node id
365373
val previous = accessibleByNodeId
366374
val updated = auxAccessibleByNodeId
367375
if (rootSemanticNode.isValid())

0 commit comments

Comments
 (0)