Hit testing ignores clips_children, so descendants clipped out by an ancestor are still hit

Open Beginner friendly
  • Is something wrong?
#848 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
78/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
rust
Domain
accessibility

Research direction

Start with NodeRef::hit_test in accesskit_consumer/src/node.rs and the existing node_at_point test there. Add coverage for the clipped-out and partly visible child cases described in the issue, plus the case where the parent does not clip children. Done means those clipped descendants are not hit while overflow under a non-clipping parent still is.

Written by the indexing model from the issue text.

Description

What happens

NodeRef::hit_test (accesskit_consumer/src/node.rs, used by node_at_point) tests each node's raw bounds and visits every child. It never consults clips_children. A descendant that its parent clips out of view is still returned for points where it would be if nothing were clipped. Often that's on top of UI that is actually visible there.

clips_children is used today only by common_filter (filters.rs, from #574). That filter excludes a child of a clipping parent when the child lies entirely outside the parent, unless a neighbouring sibling is inside, so that off-screen items stay reachable for scrolling. Three cases still hit-test wrongly:

  • Off-screen neighbours: a child kept by that rule (the one just before or after the visible ones) is hit at its off-screen position.
  • Partly visible children: the clipped-off part of a partly visible child is hit.
  • Overflowing descendants: a partly visible child's descendants that extend past the clipping ancestor are hit. The filter only looks at the immediate parent.
What should happen

A point outside a node that clips its children shouldn't hit any of that node's descendants.

Platforms

This affects every platform. The macOS, Windows, Unix (AT-SPI), iOS and Android adapters all resolve hit tests through node_at_point.

How to reproduce

The tree: a header button above a clipping scroll view, which holds one child scrolled up under the header and one visible child. Sketched in the style of the filters.rs tests:

let update = TreeUpdate {
    nodes: vec![
        (NodeId(0), {
            let mut node = Node::new(Role::Window);
            node.set_bounds(Rect::new(0.0, 0.0, 200.0, 300.0));
            node.set_children(vec![NodeId(1), NodeId(2)]);
            node
        }),
        // Header, drawn over the top of the scroll view's content area.
        (NodeId(1), {
            let mut node = Node::new(Role::Button);
            node.set_bounds(Rect::new(0.0, 0.0, 200.0, 50.0));
            node
        }),
        (NodeId(2), {
            let mut node = Node::new(Role::ScrollView);
            node.set_clips_children();
            node.set_bounds(Rect::new(0.0, 50.0, 200.0, 300.0));
            node.set_children(vec![NodeId(3), NodeId(4)]);
            node
        }),
        // Scrolled above the viewport, under the header. Kept by
        // common_filter because its next sibling is visible.
        (NodeId(3), {
            let mut node = Node::new(Role::Button);
            node.set_bounds(Rect::new(0.0, 10.0, 200.0, 40.0));
            node
        }),
        (NodeId(4), {
            let mut node = Node::new(Role::Button);
            node.set_bounds(Rect::new(0.0, 60.0, 200.0, 90.0));
            node
        }),
    ],
    tree: Some(TreeInfo::new(NodeId(0))),
    tree_id: TreeId::ROOT,
    focus: NodeId(0),
};
let tree = crate::Tree::new(update, false);
let hit = tree
    .state()
    .root()
    .node_at_point(Point::new(100.0, 25.0), &common_filter)
    .map(|node| node.id());
// Expected: the header, NodeId(1). Today: NodeId(3), which is clipped out.

A second case: give node 3 bounds of 0..200 × 30..80 (partly visible), and hit (100, 40). That point is above the viewport, so the header should win. Today node 3 does.

We hit this in a GPUI app. With a panel scrolled, VoiceOver and other accessibility clients pressing a header button activated a hidden list item beneath it. We worked around it in the toolkit by clipping each node's bounds before handing them to AccessKit. But toolkits that already set clips_children shouldn't have to do that.

Proposed plan
  • The change: in hit_test's Visit frame, when a node has clips_children() and has bounds, push its children only if the point (already in that node's coordinate space at that stage) lies inside its bounds. The node's own Test frame is unchanged.
  • Why this is enough: applied recursively, this means descendants are only hit within the intersection of their clipping ancestors. That covers all three cases above, without changing the filter or which nodes are reachable for scrolling into view.
  • Chromium comparison: BrowserAccessibility::ApproximateHitTest (ui/accessibility/platform/browser_accessibility.cc) only descends into a child whose GetClippedScreenBoundsRect() contains the point, so it already hit-tests against ancestor-clipped bounds. The change would bring AccessKit in line.
  • Platform parity: all adapters call node_at_point, so a consumer-only change applies everywhere. No adapter changes are needed.
  • Tests: the two cases above as unit tests next to the existing node_at_point test in node.rs. Plus a regression check that a node whose parent doesn't set clips_children still hits outside the parent's bounds, as overflow does today.

I'm happy to implement this if the approach works for you.

Dominant language
Rust
Stars
1.6k
Forks
135
Avg merge
10h 53m
Merged PRs (30d)
11

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from AccessKit/accesskit

All issues in AccessKit/accesskit

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.