Remove ComposeSceneAccessible and refactor accessibility implementation

m-sasha · m-sasha · commit dd52aac601ff · 2026-01-23T18:40:04.000+02:00
diff --git a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposeWindow.desktop.kt b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposeWindow.desktop.kt
@@ -36,7 +36,6 @@ import java.awt.event.MouseListener
 import java.awt.event.MouseMotionListener
 import java.awt.event.MouseWheelListener
 import java.util.Locale
-import javax.accessibility.Accessible
 import javax.swing.JFrame
 import org.jetbrains.skiko.GraphicsApi
 import org.jetbrains.skiko.SkiaLayerAnalytics
@@ -79,10 +78,6 @@ class ComposeWindow @ExperimentalComposeUiApi constructor(
     internal val windowContext by composePanel::windowContext
     internal var rootForTestListener by composePanel::rootForTestListener
 
-    // Don't override the accessible context of JFrame, since accessibility work through HardwareLayer
-    internal val windowAccessible: Accessible
-        get() = composePanel.windowAccessible
-
     /**
      * Returns the [SemanticsOwner]s corresponding to the roots of the semantics trees in this
      * [ComposeWindow].
diff --git a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposeWindowPanel.desktop.kt b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposeWindowPanel.desktop.kt
@@ -74,7 +74,6 @@ internal class ComposeWindowPanel(
         }
     private val contentComponent by composeContainer::contentComponent
 
-    val windowAccessible by composeContainer::accessible
     val windowContext by composeContainer::windowContext
     var rootForTestListener by composeContainer::rootForTestListener
     var fullscreen by composeContainer::fullscreen
diff --git a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/a11y/Accessibility.desktop.kt b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/a11y/Accessibility.desktop.kt
@@ -17,18 +17,18 @@ import org.jetbrains.skiko.initializeCAccessible
  */
 internal class AccessibleFocusHelper(
     private val component: Component,
-    private val regularAccessible: Accessible,
+    private val regularAccessibleContext: AccessibleContext,
 ) {
 
     private var focusedAccessible: Accessible? = null
 
     val accessibleContext: AccessibleContext
-        get() = (focusedAccessible ?: regularAccessible).accessibleContext
+        get() = focusedAccessible?.accessibleContext ?: regularAccessibleContext
 
     private var resetFocusAccessibleJob: Job? = null
 
     @OptIn(DelicateCoroutinesApi::class)
-    fun requestFocusOnAccessible(accessible: Accessible?) {
+    fun requestFocusOnAccessible(accessible: Accessible) {
         focusedAccessible = accessible
 
         when (hostOs) {
@@ -50,6 +50,19 @@ internal class AccessibleFocusHelper(
         }
     }
 
+    /**
+     * When [focusedAccessible] is set, for the accessible hierarchy to be correct, its parent must
+     * be reported as the scene's accessible context. This function returns it if [accessible] is
+     * [focusedAccessible].
+     */
+    fun accessibleParentOverride(accessible: Accessible): Accessible? {
+        return if (accessible == focusedAccessible) {
+            regularAccessibleContext.accessibleParent as Accessible
+        } else {
+            null
+        }
+    }
+
     private fun requestAccessBridgeFocusOnAccessible() {
         val focusEvent = FocusEvent(component, FocusEvent.FOCUS_GAINED)
         component.focusListeners.forEach { it.focusGained(focusEvent) }
diff --git a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/a11y/AccessibilityController.kt b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/a11y/AccessibilityController.kt
@@ -18,6 +18,7 @@ package androidx.compose.ui.platform.a11y
 
 import androidx.collection.mutableScatterMapOf
 import androidx.compose.ui.platform.PlatformComponent
+import androidx.compose.ui.platform.a11y.ComposeSceneAccessibility.ComposeSceneAccessibleContext
 import androidx.compose.ui.semantics.ProgressBarRangeInfo
 import androidx.compose.ui.semantics.SemanticsConfiguration
 import androidx.compose.ui.semantics.SemanticsNode
@@ -50,13 +51,13 @@ import kotlinx.coroutines.launch
  * when a [SemanticsNode] from [owner] received a focus. `null` is passed when no nodes in the scene
  * are focused.
  *
- * @see ComposeSceneAccessible
+ * @see ComposeSceneAccessibleContext
  * @see ComposeAccessible
  */
 internal class AccessibilityController(
     val owner: SemanticsOwner,
     val desktopComponent: PlatformComponent,
-    val parentAccessible: ComposeSceneAccessible,
+    val sceneAccessibility: ComposeSceneAccessibility,
     private val onFocusReceived: (ComposeAccessible?) -> Unit,
 ) {
 
@@ -92,7 +93,22 @@ internal class AccessibilityController(
      * Returns the index of this [AccessibilityController]'s root node in the scene.
      */
     fun indexInScene(): Int {
-        return parentAccessible.indexOfChild(this)
+        return sceneAccessibility.indexOfChild(this)
+    }
+
+    /**
+     * Returns the scene's [Accessible].
+     */
+    fun sceneAccessible(): Accessible? {
+        return sceneAccessibility.accessible()
+    }
+
+    /**
+     * Returns an override of the given [Accessible]'s parent; `null` if there is no override and
+     * its parent should be reported as its normal parent from the node hierarchy.
+     */
+    fun accessibleParentOverride(accessible: ComposeAccessible): Accessible? {
+        return sceneAccessibility.accessibleParentOverride(accessible)
     }
 
     /**
diff --git a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/a11y/ComposeAccessible.kt b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/a11y/ComposeAccessible.kt
@@ -266,7 +266,9 @@ internal class ComposeAccessible(
         }
 
         override fun getAccessibleParent(): Accessible? {
-            val parentNode = semanticsNode.parent ?: return controller.parentAccessible
+            controller.accessibleParentOverride(this@ComposeAccessible)?.let { return it }
+
+            val parentNode = semanticsNode.parent ?: return controller.sceneAccessible()
             return controller.accessibleByNodeId(parentNode.id)!!
         }
 
@@ -279,47 +281,49 @@ internal class ComposeAccessible(
             return this
         }
 
-        // we have to store a reference to AccessibleAction, because AWT itself uses weak
-        // references and GC could delete an object which is, in fact, in use
-        private var accessibleAction: AccessibleAction? = null
+        private var accessibleActions: List<Pair<String, ActionKey>>? = null
 
-        override fun getAccessibleAction(): AccessibleAction? {
-            val actions = mutableListOf<Pair<String?, ActionKey>>()
+        private fun updateAccessibleActions() {
+            val actions = mutableListOf<Pair<String, ActionKey>>()
 
-            fun addActionIfExist(key: SemanticsPropertyKey<AccessibilityAction<() -> Boolean>>) {
-                semanticsConfig.getOrNull(key)?.let {
-                    actions.add(Pair(it.label, key))
-                }
-            }
-            semanticsConfig.getOrNull(SemanticsActions.OnClick)?.let {
-                // AWT expects "click" label for click actions, at least on macOS...
-                actions.add(Pair("click", SemanticsActions.OnClick))
+            fun addActionIfExist(
+                key: SemanticsPropertyKey<AccessibilityAction<() -> Boolean>>,
+                label: String? = null,
+            ) {
+                val action = semanticsConfig.getOrNull(key) ?: return
+                val label = label ?: action.label ?: return  // No point to add an action without a label
+                actions.add(Pair(label, key))
             }
 
+            // AWT expects a "click" label for click actions, at least on macOS...
+            addActionIfExist(SemanticsActions.OnClick, AccessibleAction.CLICK)
             addActionIfExist(SemanticsActions.OnLongClick)
             addActionIfExist(SemanticsActions.Expand)
             addActionIfExist(SemanticsActions.Collapse)
             addActionIfExist(SemanticsActions.Dismiss)
 
-            if (actions.isEmpty()) {
-                return null
-            }
-            accessibleAction = object : AccessibleAction {
-                override fun getAccessibleActionCount(): Int = actions.size
+            accessibleActions = actions.takeIf { it.isNotEmpty() }
+        }
 
-                override fun getAccessibleActionDescription(i: Int): String? {
-                    val (label, _) = actions[i]
-                    return label
-                }
+        override fun getAccessibleAction(): AccessibleAction? {
+            updateAccessibleActions()
+            return if (accessibleActions == null) null else this
+        }
 
-                override fun doAccessibleAction(i: Int): Boolean {
-                    val (_, actionKey) = actions[i]
-                    return semanticsConfig.getOrNull(actionKey)?.let {
-                        it.action?.invoke()
-                    } ?: false
-                }
-            }
-            return accessibleAction
+        override fun getAccessibleActionCount(): Int = accessibleActions?.size ?: 0
+
+        override fun getAccessibleActionDescription(i: Int): String? {
+            val actions = accessibleActions!!
+            val (label, _) = actions[i]
+            return label
+        }
+
+        override fun doAccessibleAction(i: Int): Boolean {
+            val actions = accessibleActions!!
+            val (_, actionKey) = actions[i]
+            return semanticsConfig.getOrNull(actionKey)?.let {
+                it.action?.invoke()
+            } ?: false
         }
 
         override fun getAccessibleValue(): AccessibleValue? {
@@ -884,20 +888,6 @@ internal class ComposeAccessible(
             TODO("Not yet implemented")
         }
 
-        // For some reasons JDK's CAccessibility does not call getAccessibleAction
-        // and performs actions on current component itself
-        override fun getAccessibleActionCount(): Int {
-            return accessibleAction?.accessibleActionCount ?: 0
-        }
-
-        override fun getAccessibleActionDescription(i: Int): String {
-            return accessibleAction?.getAccessibleActionDescription(i) ?: ""
-        }
-
-        override fun doAccessibleAction(i: Int): Boolean {
-            return accessibleAction?.doAccessibleAction(i) ?: false
-        }
-
         private fun List<CharSequence>.mergeText() = joinToString(", ")
     }
 }
diff --git a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/a11y/ComposeSceneAccessibility.kt b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/a11y/ComposeSceneAccessibility.kt
@@ -35,48 +35,46 @@ import javax.accessibility.AccessibleState
 import javax.accessibility.AccessibleStateSet
 import org.jetbrains.skiko.OS
 import org.jetbrains.skiko.hostOs
+import androidx.compose.ui.scene.ComposeSceneMediator
 
 /**
- * This is a root [Accessible] for a [ComposeScene]
- *
- * It provides [AccessibleContext] to Swing accessibility support.
- * This context has no parents and provides [ComposeAccessible] as its children.
- *
- * The main purpose of this class is to support screen readers that read text under the mouse (not only, but mostly).
- * To support it [_accessibleContext] provides custom [ComposeSceneAccessibleContext.getAccessibleAt] implementation.
- *
- * Note about a11y for focus-based tools (e.g. VoiceOver).
- * Now focus-based tools are supported on [org.jetbrains.skiko.HardwareLayer] side.
- * When Compose's [androidx.compose.ui.semantics.SemanticsNode] is focused
- * [AccessibilityController.onFocusReceived] is called and
- * [org.jetbrains.skiko.HardwareLayer] provides mapped [ComposeAccessible] to accessibility tool.
+ * Manages the accessibility aspects of the Compose scene for [ComposeSceneMediator].
  *
  * @see AccessibilityController
  * @see ComposeAccessible
  */
-internal class ComposeSceneAccessible(
+internal class ComposeSceneAccessibility(
     private val isWindowLevel: Boolean = false,
-    private val forceEnableA11y: Boolean = false,
     private val sceneRoot: () -> Component,
     private val accessibilityControllersProvider: () -> List<AccessibilityController>,
-) : Accessible {
-    private val a11yEnabled by lazy {
-        forceEnableA11y || (
-            (System.getProperty("compose.accessibility.enable") != "false") &&
-            (System.getenv("COMPOSE_DISABLE_ACCESSIBILITY") == null)
-        )
+) {
+    val enabled by lazy {
+        (System.getProperty("compose.accessibility.enable") != "false") &&
+        (System.getenv("COMPOSE_DISABLE_ACCESSIBILITY") == null)
     }
 
-    private val _accessibleContext by lazy {
+    // Exposed for the benefit of tests
+    val sceneAccessibleContext by lazy {
         ComposeSceneAccessibleContext()
     }
 
-    // Declare ComposeSceneAccessibleContext as the return type for the benefit of tests
-    override fun getAccessibleContext(): ComposeSceneAccessibleContext? {
-        if (!a11yEnabled) {
-            return null
-        }
-        return _accessibleContext
+    private val accessibleFocusHelper by lazy {
+        AccessibleFocusHelper(sceneRoot(), sceneAccessibleContext)
+    }
+
+    val accessibleContextProvider: ((Component) -> AccessibleContext)?
+        get() = if (enabled) { _ -> accessibleFocusHelper.accessibleContext } else null
+
+    fun requestFocusOnAccessible(accessible: Accessible) {
+        accessibleFocusHelper.requestFocusOnAccessible(accessible)
+    }
+
+    fun accessibleParentOverride(accessible: Accessible): Accessible? {
+        return accessibleFocusHelper.accessibleParentOverride(accessible)
+    }
+
+    fun accessible(): Accessible? {
+        return sceneRoot() as? Accessible
     }
 
     fun indexOfChild(controller: AccessibilityController): Int {
@@ -151,7 +149,7 @@ internal class ComposeSceneAccessible(
                 }
             }
 
-            return this@ComposeSceneAccessible
+            return sceneRoot() as Accessible
         }
 
         override fun contains(p: Point): Boolean = true
diff --git a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/ComposeContainer.desktop.kt b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/ComposeContainer.desktop.kt
@@ -168,7 +168,6 @@ internal class ComposeContainer(
 
     val contentComponent by mediator::contentComponent
     val focusManager by mediator::focusManager
-    val accessible by mediator::accessible
     var rootForTestListener by mediator::rootForTestListener
     // TODO: Changing fullscreen probably will require recreate our layers
     //  It will require add this flag as remember parameters in rememberComposeSceneLayer
diff --git a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.desktop.kt b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.desktop.kt
@@ -59,8 +59,7 @@ import androidx.compose.ui.platform.PlatformWindowContext
 import androidx.compose.ui.platform.ViewConfiguration
 import androidx.compose.ui.platform.WindowInfo
 import androidx.compose.ui.platform.a11y.AccessibilityController
-import androidx.compose.ui.platform.a11y.AccessibleFocusHelper
-import androidx.compose.ui.platform.a11y.ComposeSceneAccessible
+import androidx.compose.ui.platform.a11y.ComposeSceneAccessibility
 import androidx.compose.ui.scene.skia.SkiaLayerComponent
 import androidx.compose.ui.semantics.SemanticsOwner
 import androidx.compose.ui.unit.Density
@@ -148,25 +147,19 @@ internal class ComposeSceneMediator(
     private val _platformContext = DesktopPlatformContext()
     val platformContext: PlatformContext get() = _platformContext
 
+    val accessibility = ComposeSceneAccessibility(
+        isWindowLevel = isWindowLevel,
+        sceneRoot = { skiaLayerComponent.contentRoot },
+        accessibilityControllersProvider = { semanticsOwnerManager.accessibilityControllers }
+    )
+
     private val skiaLayerComponent: SkiaLayerComponent by lazy { skiaLayerComponentFactory(this) }
     val contentComponent by skiaLayerComponent::hierarchyRoot
     var fullscreen by skiaLayerComponent::fullscreen
     val windowHandle by skiaLayerComponent::windowHandle
     val renderApi by skiaLayerComponent::renderApi
     val semanticsOwners: Collection<SemanticsOwner> by semanticsOwnerManager::semanticsOwners
 
-    val accessible: ComposeSceneAccessible = ComposeSceneAccessible(
-        isWindowLevel = isWindowLevel,
-        sceneRoot = { skiaLayerComponent.contentRoot },
-        accessibilityControllersProvider = { semanticsOwnerManager.accessibilityControllers }
-    )
-
-    private val accessibleFocusHelper by lazy {
-        AccessibleFocusHelper(skiaLayerComponent.contentRoot, accessible)
-    }
-
-    fun getAccessibleContext() = accessibleFocusHelper.accessibleContext
-
     /**
      * @see ComposeFeatureFlags.useInteropBlending
      */
@@ -785,17 +778,17 @@ internal class ComposeSceneMediator(
             _accessibilityControllers[semanticsOwner] = AccessibilityController(
                 owner = semanticsOwner,
                 desktopComponent = platformComponent,
-                parentAccessible = accessible,
+                sceneAccessibility = accessibility,
                 onFocusReceived = {
                     // requestFocusOnAccessible fires focusGained events, which in turn
                     // can call this method themselves, so we need to prevent infinite recursion
                     if (requestingFocus) return@AccessibilityController
-                    requestingFocus = true
 
-                    val target = it ?: accessible.defaultAccessibilityFocusTarget()
+                    val target = it ?: accessibility.defaultAccessibilityFocusTarget()
                     if (target != null) {
+                        requestingFocus = true
                         try {
-                            accessibleFocusHelper.requestFocusOnAccessible(target)
+                            accessibility.requestFocusOnAccessible(target)
                         } finally {
                             requestingFocus = false
                         }
diff --git a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/skia/SwingSkiaLayerComponent.desktop.kt b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/skia/SwingSkiaLayerComponent.desktop.kt
@@ -47,7 +47,7 @@ internal class SwingSkiaLayerComponent(
         object : SkiaSwingLayer(
             renderDelegate = renderDelegate,
             analytics = skiaLayerAnalytics,
-            accessibleContextProvider = { mediator.getAccessibleContext() },
+            accessibleContextProvider = mediator.accessibility.accessibleContextProvider
         ) {
             private var endCompositionWorkaround: InputMethodEndCompositionWorkaround? = null
 
diff --git a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/skia/WindowSkiaLayerComponent.desktop.kt b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/skia/WindowSkiaLayerComponent.desktop.kt
diff --git a/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/platform/AccessibilityTest.kt b/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/platform/AccessibilityTest.kt
diff --git a/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/platform/ApplicationAccessibilityTest.kt b/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/platform/ApplicationAccessibilityTest.kt

Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,6 @@ internal class ComposeWindowPanel(`
`74`	`74`	`}`
`75`	`75`	`private val contentComponent by composeContainer::contentComponent`
`76`	`76`
`77`		`- val windowAccessible by composeContainer::accessible`
`78`	`77`	`val windowContext by composeContainer::windowContext`
`79`	`78`	`var rootForTestListener by composeContainer::rootForTestListener`
`80`	`79`	`var fullscreen by composeContainer::fullscreen`