Skip to content

Commit cf0f19b

Browse files
committed
fix(ios): harden landscape accessibility overlays
1 parent 95cbe75 commit cf0f19b

4 files changed

Lines changed: 112 additions & 25 deletions

File tree

packages/client/src/features/accessibility/AccessibilityOverlay.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
AccessibilityOverlay,
77
accessibilityDomTagName,
88
} from "./AccessibilityOverlay";
9+
import type { AccessibilityNode } from "../../api/types";
910

1011
describe("accessibilityDomTagName", () => {
1112
it("uses source and component names for annotator-friendly custom tags", () => {
@@ -104,6 +105,33 @@ describe("AccessibilityOverlay", () => {
104105
expect(markup).not.toContain(" title=");
105106
});
106107

108+
it("tolerates non-string accessibility metadata", () => {
109+
const markup = renderToStaticMarkup(
110+
createElement(AccessibilityOverlay, {
111+
hoveredId: null,
112+
roots: [
113+
{
114+
frame: { height: 844, width: 390, x: 0, y: 0 },
115+
role: "application",
116+
children: [
117+
{
118+
AXValue: 42,
119+
frame: { height: 48, width: 180, x: 105, y: 720 },
120+
placeholder: { text: "Email" },
121+
sourceFile: null,
122+
type: "TextField",
123+
} as unknown as AccessibilityNode,
124+
],
125+
},
126+
],
127+
selectedId: "",
128+
}),
129+
);
130+
131+
expect(markup).toContain('data-simdeck-accessibility-value="42"');
132+
expect(markup).toContain('data-simdeck-accessibility-label="42"');
133+
});
134+
107135
it("draws label-free skeleton frames when requested", () => {
108136
const markup = renderToStaticMarkup(
109137
createElement(AccessibilityOverlay, {

packages/client/src/features/accessibility/AccessibilityOverlay.tsx

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -487,8 +487,12 @@ function accessibilityStateSummary(
487487
return state.join(", ");
488488
}
489489

490-
function cleanAccessibilityText(
491-
value: string | null | undefined,
492-
): string | null {
493-
return value?.trim() || null;
490+
function cleanAccessibilityText(value: unknown): string | null {
491+
if (typeof value === "string") {
492+
return value.trim() || null;
493+
}
494+
if (typeof value === "number" || typeof value === "boolean") {
495+
return String(value);
496+
}
497+
return null;
494498
}

packages/client/src/features/accessibility/accessibilityTree.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -579,9 +579,15 @@ function frameContainsPoint(
579579
);
580580
}
581581

582-
function cleanText(value: string | null | undefined): string | null {
583-
const trimmed = value?.trim();
584-
return trimmed ? trimmed : null;
582+
function cleanText(value: unknown): string | null {
583+
if (typeof value === "string") {
584+
const trimmed = value.trim();
585+
return trimmed ? trimmed : null;
586+
}
587+
if (typeof value === "number" || typeof value === "boolean") {
588+
return String(value);
589+
}
590+
return null;
585591
}
586592

587593
function displayAccessibilityKind(

packages/server/src/accessibility.rs

Lines changed: 67 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,13 @@ impl AccessibilitySource {
6464
/// Not every app is affected: apps that lay their views out natively in the
6565
/// current orientation (e.g. Settings) already report display-space frames.
6666
/// We therefore rotate a root's subtree only when the subtree itself proves it
67-
/// is in portrait space — no descendant frame is wider than the portrait width
68-
/// (which, in landscape, equals the display height). The top-level application
69-
/// element reports its own frame in display space regardless, so it (and any
70-
/// oversized outlier) is left untouched.
67+
/// is in portrait space: a descendant extends below the landscape display
68+
/// height, which is only valid on the native portrait canvas. Conversely, a
69+
/// descendant that extends past the portrait width proves the subtree is
70+
/// already in display space. Ambiguous small left-side subtrees are left alone
71+
/// rather than guessing. The top-level application element reports its own
72+
/// frame in display space regardless, so it (and any oversized outlier) is left
73+
/// untouched.
7174
///
7275
/// Only landscape orientations (odd quarter turns) are handled. Portrait (0)
7376
/// needs no change, and upside-down portrait (2) is left as-is.
@@ -86,10 +89,12 @@ pub fn normalize_native_ax_orientation(snapshot: &mut Value, quarter_turns: i32)
8689
return;
8790
};
8891
for root in roots.iter_mut() {
89-
if subtree_is_display_space(root, display_height) {
90-
continue;
92+
match subtree_orientation(root, display_height) {
93+
SubtreeOrientation::PortraitNative => {
94+
rotate_descendant_frames(root, quarter_turns, display_width, display_height);
95+
}
96+
SubtreeOrientation::DisplaySpace | SubtreeOrientation::Ambiguous => {}
9197
}
92-
rotate_descendant_frames(root, quarter_turns, display_width, display_height);
9398
}
9499
}
95100

@@ -118,22 +123,48 @@ fn display_size_from_roots(roots: &[Value]) -> Option<(f64, f64)> {
118123
Some((width.max(height), width.min(height)))
119124
}
120125

121-
/// True when a root's subtree is already in display space, detected by any
122-
/// descendant frame being wider than the portrait width (== display height in
123-
/// landscape). Portrait content can never exceed the portrait width.
124-
fn subtree_is_display_space(root: &Value, display_height: f64) -> bool {
125-
fn any_wider_than(node: &Value, threshold: f64) -> bool {
126+
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
127+
enum SubtreeOrientation {
128+
PortraitNative,
129+
DisplaySpace,
130+
Ambiguous,
131+
}
132+
133+
/// Classify the coordinate space of a root's descendants.
134+
///
135+
/// In landscape, the natural portrait canvas is `display_height` points wide
136+
/// and `display_width` points tall. A descendant whose right edge exceeds
137+
/// `display_height` must already be in display space. A descendant whose bottom
138+
/// edge exceeds `display_height` proves portrait-native coordinates, because
139+
/// that point would be outside the visible landscape display.
140+
fn subtree_orientation(root: &Value, display_height: f64) -> SubtreeOrientation {
141+
fn visit(node: &Value, threshold: f64) -> SubtreeOrientation {
126142
let Some(children) = node.get("children").and_then(Value::as_array) else {
127-
return false;
143+
return SubtreeOrientation::Ambiguous;
128144
};
129-
children.iter().any(|child| {
130-
frame_rect(child).is_some_and(|(x, _, width, _)| x + width > threshold)
131-
|| any_wider_than(child, threshold)
132-
})
145+
let mut orientation = SubtreeOrientation::Ambiguous;
146+
for child in children {
147+
if let Some((x, y, width, height)) = frame_rect(child) {
148+
if x + width > threshold {
149+
return SubtreeOrientation::DisplaySpace;
150+
}
151+
if y + height > threshold {
152+
orientation = SubtreeOrientation::PortraitNative;
153+
}
154+
}
155+
match visit(child, threshold) {
156+
SubtreeOrientation::DisplaySpace => return SubtreeOrientation::DisplaySpace,
157+
SubtreeOrientation::PortraitNative => {
158+
orientation = SubtreeOrientation::PortraitNative;
159+
}
160+
SubtreeOrientation::Ambiguous => {}
161+
}
162+
}
163+
orientation
133164
}
134165
// +1pt slack so an element spanning exactly the portrait width is not a
135166
// false positive.
136-
any_wider_than(root, display_height + 1.0)
167+
visit(root, display_height + 1.0)
137168
}
138169

139170
fn rotate_descendant_frames(root: &mut Value, quarter_turns: i32, display_w: f64, display_h: f64) {
@@ -495,6 +526,24 @@ mod orientation_tests {
495526
assert_eq!(snapshot, before);
496527
}
497528

529+
#[test]
530+
fn leaves_ambiguous_left_side_subtree_untouched() {
531+
let mut snapshot = json!({
532+
"source": "native-ax",
533+
"roots": [{
534+
"role": "Application",
535+
"frame": { "x": 0, "y": 0, "width": 1210, "height": 834 },
536+
"children": [
537+
{ "role": "Button", "AXLabel": "Narrow",
538+
"frame": { "x": 24, "y": 96, "width": 240, "height": 56 } }
539+
]
540+
}]
541+
});
542+
let before = snapshot.clone();
543+
normalize_native_ax_orientation(&mut snapshot, 3);
544+
assert_eq!(snapshot, before);
545+
}
546+
498547
#[test]
499548
fn portrait_and_upside_down_are_noops() {
500549
for quarter_turns in [0, 2, 4] {

0 commit comments

Comments
 (0)