chore: [SDK-4406] prepare Unity demo app for Appium E2E tests

[fadi-george]

Description

One Line Summary

Prepare the Unity demo app for Appium E2E test automation by adding accessibility bridges, stabilizing UI element naming, and improving iOS/Android test reliability.

Details

Motivation

SDK-4406: enable Appium-driven E2E tests against the Unity demo app. Unity's UI Toolkit does not expose native accessibility identifiers, so Appium cannot reliably locate elements on iOS or Android. This PR adds native accessibility bridges (iOS and Android), standardizes UI element names, and fixes a number of demo-side bugs uncovered while wiring up the test harness.

Scope

• Only examples/demo/ is touched. No SDK source changes.
• Adds a C# AccessibilityBridge plus iOS (OneSignalDemoKeyboard.mm) and Android (OneSignalUnityE2EAccessibility.java) native helpers used to expose UI elements to Appium.
• Adds an iOS signing post-processor and arm64 simulator config so the demo builds cleanly on Apple Silicon CI.
• Demo refactors: removes the in-app LogView/LogManager (replaced with Debug.Log), renames TrackEventSectionController to CustomEventsSectionController, consolidates upsert helpers, replaces the loading overlay with inline loading states, and tightens dialog/keyboard behavior.
• Bumps Unity editor version and demo packages.

Testing

Manual testing

• Built and ran the demo on iOS Simulator (arm64, Apple Silicon) and Android emulator.
• Verified Appium can locate and interact with home screen sections, dialogs, toggles, and toasts via the accessibility bridge.
• Smoke-tested core demo flows: login/logout, aliases, tags, triggers, push send, IAM, custom events, location, live activities.

Affected code checklist

• None - demo app only, no SDK changes.

Checklist

Overview

• I have filled out all REQUIRED sections above
• PR does one thing (demo prep for Appium E2E)
• No public API changes

Testing

• Manually tested on iOS Simulator and Android emulator
• No SDK behavior changes, so no new unit tests needed

Final pass

• Code reviewed

Made with Cursor

[claude]

🔴 MultiPairInputDialog names new TextFields using $"multipair_key_{_rows.Count}" / $"multipair_value_{_rows.Count}", but RemoveRow removes an entry from _rows without renumbering the survivors. After Add×3 → Remove(middle) → Add, the new row collides with the still-present original row 2, and AccessibilityBridge.WalkAndUpsert dedupes by name (first-wins), so the new row is silently dropped from the a11y tree and Appium taps targeting multipair_key_2 land on the stale original row. Fix: use a monotonic counter (_nextRowIndex++) instead of _rows.Count, or renumber survivors on remove.

Extended reasoning...

Bug

MultiPairInputDialog.AddRow at examples/demo/Assets/Scripts/UI/Dialogs/MultiPairInputDialog.cs:92-100 names the new row's TextFields:

keyField.name = $"multipair_key_{_rows.Count}";
// ...
valueField.name = $"multipair_value_{_rows.Count}";

The suffix is _rows.Count captured before the new entry is appended (line 116 _rows.Add(entry)). RemoveRow at line 134 removes an entry from _rows and rebuilds _rowsContainer from the remaining rows, but it never renames the survivors — they keep their original suffixes.

Why it manifests

This only matters because the PR's new AccessibilityBridge.WalkAndUpsert (Assets/Scripts/Services/AccessibilityBridge.cs ~line 895-898) dedupes by name:

if (seenNames.Add(name))
{
    // ... AddNode / re-bind ...
    _map[el] = node;
}

HashSet<string>.Add returns false for duplicates, so the second VisualElement with the same name is silently skipped — no accessibility node, no Android sync entry. _rowsContainer is walked in order, so the original row (added first, still present after RemoveRow re-adds in _rows order) wins, and the newly-added row is invisible to XCUITest / UiAutomator2.

Step-by-step proof

1. Open Add Multiple Tags dialog. AddRow runs once during BuildContent (line 65): _rows.Count == 0 → creates multipair_key_0 / multipair_value_0.
2. Tap "Add Row" (multipair_add_row_button). InvokeAddRow → AddRow: _rows.Count == 1 → creates multipair_key_1 / multipair_value_1.
3. Tap "Add Row" again. _rows.Count == 2 → creates multipair_key_2 / multipair_value_2. _rows is now [r0, r1, r2].
4. Type "old_value" into row 2 (multipair_value_2).
5. Tap the close button on row 1 (the middle row). RemoveRow(r1) calls _rows.Remove(entry), then clears and re-adds _rowsContainer from _rows. _rows is now [r0, r2]. The surviving rows still carry names multipair_key_0/2 and multipair_value_0/2.
6. Tap "Add Row". _rows.Count == 2 → creates a new multipair_key_2 / multipair_value_2 — duplicate of the surviving r2.
7. AccessibilityBridge's next BuildHierarchy walks _rowsContainer in tree order (the old r2 was added before the new row), so seenNames.Add("multipair_key_2") returns true for the old r2 and false for the new one. Only the old r2 is published to the a11y tree.
8. Test types into multipair_key_2 expecting the freshly-added empty row → input lands on the stale row carrying "old_value".

Why existing code doesn't prevent it

There is no rename step in RemoveRow, and AddRow reads _rows.Count rather than tracking a monotonically-increasing index. The dedup in WalkAndUpsert is intentional (the comment explicitly says "first one wins; only one accessibility node per name to keep ids unambiguous"), so the bridge cannot disambiguate post-hoc.

Impact

E2E tests that exercise the multi-pair dialogs (Add Multiple Aliases / Tags / Triggers) and perform a remove-then-add sequence will silently target the wrong row, producing a wrong-target failure instead of a clean error. Demo-only — no SDK impact. The PR description explicitly identifies preparing the demo for Appium E2E tests as the motivating goal, and remove-then-add is a plausible test scenario for these dialogs.

Fix

One-line fix: replace _rows.Count in the two $"multipair_..." interpolations with a monotonic counter:

private int _nextRowIndex;
// ...
keyField.name = $"multipair_key_{_nextRowIndex}";
valueField.name = $"multipair_value_{_nextRowIndex}";
_nextRowIndex++;

Alternative: renumber surviving rows in RemoveRow after the _rowsContainer.Clear() + replay.

[claude]

🔴 AccessibilityBridge is a DontDestroyOnLoad singleton and _scrollViewHooksInstalled / _mainSv are never reset across scenes. After the Main → Secondary → Main flow wired up by the new next_screen_button and back_button, Initialize re-runs BuildHierarchy on the new root but the if (!_scrollViewHooksInstalled) guard at line 466 skips re-finding main_scroll_view — so the new ScrollView never gets WheelEvent blocking, touchScrollBehavior = Clamped, decel/elasticity = 0, or the pointer-activity watchdog, and _mainSv keeps pointing at the now-orphaned ScrollView from the previous Main scene. Fix is to clear _scrollViewHooksInstalled and _mainSv at the top of Initialize (or detect a _root change), so the hooks are re-installed on the new ScrollView.

Extended reasoning...

What the bug is

AccessibilityBridge.EnableForE2E creates its singleton with DontDestroyOnLoad (line 161), so the bridge instance survives scene transitions. On every subsequent EnableForE2E(root) the if (_instance == null) branch is skipped and only Initialize(root) runs. Initialize sets _root = root and calls BuildHierarchy(), but it does not reset _scrollViewHooksInstalled (or clear _mainSv). The one-shot ScrollView taming block in BuildHierarchy (lines 461–478) is therefore skipped on the second Main load, and _mainSv still points at the orphaned ScrollView from the previous Main scene.

The inline comment at line 464 (The flag is reset only by OnDestroy — a fresh bridge instance re-applies them.) is incorrect for this codebase: OnDestroy on a DontDestroyOnLoad singleton only fires on application quit, never on scene unload.

Trigger path

The demo wires up a real multi-scene flow:

1. HomeScreenController.BuildSections() adds a next_screen_button that calls SceneManager.LoadScene("Secondary") (HomeScreenController.cs:253).
2. SecondaryScreenController.OnEnable adds a back_button that calls SceneManager.LoadScene("Main") (SecondaryScreenController.cs:27).
3. Both controllers call AccessibilityBridge.EnableForE2E(root) in their OnEnable (HomeScreenController.cs:64, SecondaryScreenController.cs:58).

Step-by-step proof (Main → Secondary → Main)

1. First Main load. HomeScreenController.OnEnable runs EnableForE2E(R1). The singleton is created, Initialize(R1) runs, BuildHierarchy() finds main_scroll_view S1, calls InstallScrollViewE2EHooks(S1), sets _mainSv = S1, and sets _scrollViewHooksInstalled = true. S1 now has WheelEvent.StopImmediatePropagation, touchScrollBehavior = Clamped, scrollDecelerationRate = 0, elasticity = 0, and the PointerDown/Move/Up activity watchdog.
2. Navigate to Secondary. next_screen_button calls LoadScene("Secondary"). The Main scene's UIDocument is destroyed, so S1 is destroyed. The bridge GameObject persists (DontDestroyOnLoad). SecondaryScreenController.OnEnable runs EnableForE2E(R2); Initialize(R2) sets _root = R2 and runs BuildHierarchy(). The if (!_scrollViewHooksInstalled) guard is false, so the block is skipped — but Secondary has no main_scroll_view anyway, so this is benign here.
3. Navigate back to Main. back_button calls LoadScene("Main"). A fresh Main scene loads with a new ScrollView S3. HomeScreenController.OnEnable runs EnableForE2E(R3). The singleton already exists, so only Initialize(R3) runs. Initialize sets _root = R3 and runs BuildHierarchy(). The guard if (!_scrollViewHooksInstalled) is still true from step 1 → the block at lines 466–478 is skipped entirely. S3 receives none of the taming hooks, and _mainSv is still pointing at the destroyed S1.

Consequences on the second Main load

• iOS XCUITest implicit-scroll resurfaces. InstallScrollViewE2EHooks was specifically designed to swallow the synthetic WheelEvent that XCUITest injects before tap dispatch (see the doc-comment on InstallScrollViewE2EHooks). S3 has no WheelEvent handler, so the tap-shift bug returns.
• Bounce/inertia mid-tap. S3 keeps engine-default touchScrollBehavior, scrollDecelerationRate, and elasticity, so PostPointerUpAnimation runs during the next queued tap.
• Stale _mainSv references. Update's RefreshScrollAccessibilityState reads _mainSv.scrollOffset off the orphaned S1 every frame. InvokeAndroidNativeScroll (Android) operates on S1, so UiAutomator2 swipes scroll the dead element instead of S3. The ResetScrollViewPanState watchdog reflects into S1's m_PointerCaptureScheduled / m_Pressed fields, leaving S3's real stuck-capture state untouched. _mainSv is a VisualElement (not a UnityEngine.Object), so if (_mainSv == null) guards never trip — the reference simply outlives the panel that owned it.
• Touch-activity watchdog is neutered. The PointerDown/Move/Up callbacks are registered on S1; S3's events never feed _lastTouchActivityMs, so the stuck-capture timer can't fire on the live ScrollView.

Severity

Demo-only, but the PR's stated motivation is "enable Appium-driven E2E tests against the Unity demo app" and this directly defeats that on any multi-scene test path. A test that exercises NEXT SCREEN → back loses every guarantee InstallScrollViewE2EHooks was written to deliver.

Fix

Reset _scrollViewHooksInstalled = false; _mainSv = null; at the top of Initialize (or, more precisely, only when root != _root), so BuildHierarchy re-finds main_scroll_view and re-installs the hooks against the new ScrollView instance.

[claude]

🔴 The info icon (*_info_icon) was changed from new Button(onInfoTap) to a plain new Label(MaterialIcons.Info) whose tap is dispatched exclusively through AccessibilityBridge.RegisterE2ETapTarget. That dispatch path is fully gated by E2E mode (and is a compile-time no-op under UNITY_EDITOR), so in any non-E2E run — Editor play, plus any device build with the default E2E_MODE=false — tapping any section's info icon does nothing and the tooltip dialog never opens. Fix by registering an unconditional ClickEvent/PointerDownEvent callback on the Label in addition to the E2E bridge registration (or restore the Button).

Extended reasoning...

What changed

In examples/demo/Assets/Scripts/UI/Sections/SectionBuilder.cs:38-47 the info icon was changed from a Button(onInfoTap) to a plain Label(MaterialIcons.Info):

var infoBtn = new Label(MaterialIcons.Info);
infoBtn.name = $"{SectionKeyFromName(name)}_info_icon";
infoBtn.AddToClassList("info-button");
infoBtn.pickingMode = PickingMode.Position;
AccessibilityBridge.RegisterE2ETapTarget(
    infoBtn,
    () => infoBtn.panel?.visualTree.Q("tooltip_title") == null,
    onInfoTap
);

The previous Button(Action) constructor wired onInfoTap through UI Toolkit's built-in Clickable manipulator, which fires on every platform/run-mode. A plain Label has no Clickable and no direct RegisterCallback<ClickEvent> / RegisterCallback<PointerDownEvent> — so the only wiring left is AccessibilityBridge.RegisterE2ETapTarget.

Why that dispatch path is empty outside E2E mode

AccessibilityBridge.RegisterE2ETapTarget (AccessibilityBridge.cs:179-202) has the whole body wrapped in #if UNITY_ANDROID && !UNITY_EDITOR / #elif UNITY_IOS && !UNITY_EDITOR, so in the Unity Editor (regardless of build target) the call compiles to a no-op.

On an iOS device build the function stores the action in _iosInfoTapByName, but the actual dispatch happens via the panel-root TrickleDown handler _onIosInfoIconPointerDown. That handler is only registered inside BuildHierarchy (visualTree.RegisterCallback(_onIosInfoIconPointerDown, ...)). BuildHierarchy runs only from Initialize (line 227), which runs only from EnableForE2E (line 165). And EnableForE2E early-returns at lines 150-156 when !DotEnv.IsE2EMode:

if (!DotEnv.IsE2EMode)
{
#if UNITY_ANDROID && !UNITY_EDITOR
    Debug.LogWarning("[OneSignalDemo] E2E accessibility bridge disabled");
#endif
    return;
}

The Android route is gated identically: dispatch goes through the OneSignalUnityE2EAccessibility Java overlay populated by SyncAndroidNativeAccessibility, which runs only from the same E2E-gated Initialize path. .env.example ships E2E_MODE=false, and a missing key defaults to empty/false in DotEnv.IsE2EMode.

Step-by-step proof — Editor play of the demo

1. AppBootstrapper.Start calls DotEnv.Load(). E2E_MODE is false (default).
2. HomeScreenController.OnEnable calls AccessibilityBridge.EnableForE2E(_root). Hits the !DotEnv.IsE2EMode branch and returns. _instance is never created; the iOS panel-root handler is never registered.
3. SectionBuilder.CreateSection("Push", "push_section", ...) constructs push_info_icon as a Label and calls RegisterE2ETapTarget. Both #if branches are excluded in the Editor — no-op. Even on a non-E2E device build, the entry that does get stored in _iosInfoTapByName is unreachable because no panel-root handler exists to read it.
4. User taps push_info_icon. UI Toolkit hit-tests to the Label (PickingMode.Position is set), no Clickable manipulator runs, no ClickEvent/PointerDownEvent callback is registered, no panel-root handler exists — the tap is silently absorbed. Tooltip dialog never opens. Same outcome for iam_info_icon, aliases_info_icon, tags_info_icon, triggers_info_icon, etc.

A grep for ClickEvent / PointerDownEvent / Clickable handlers on info icons or the info-button class returns nothing else — there is no other dispatch path.

Impact

Demo-only regression, but it silently breaks the tooltip feature for every developer running the demo outside the E2E pipeline — Editor play sessions and any public non-E2E device build. Those are exactly the discoverability paths an SDK user takes when learning the demo.

Fix

Register an unconditional event callback on the Label in addition to the E2E bridge, e.g.:

infoBtn.RegisterCallback<ClickEvent>(_ => onInfoTap());

(or restore new Button(onInfoTap) and re-style the button). This preserves the E2E dispatch path while making the tooltip work in Editor play and in non-E2E device builds.

[claude]

🟡 Indentation nit: AccessibilityBridge.RegisterE2ETapTarget(deleteBtn, () => true, onDelete); is indented at 12 spaces in SectionBuilder.cs:167 (inside CreateKeyValueItem's if (onDelete != null) block) and SectionBuilder.cs:223 (inside CreateSingleItem's if (onDelete != null) block), while the sibling statements use 16 spaces. Purely cosmetic — braces enclose it correctly so behavior is unaffected, but the line visually pops out of the if-block. A re-indent (or dotnet format pass) fixes both.

Extended reasoning...

What and where

In SectionBuilder.CreateKeyValueItem (line 167) and SectionBuilder.CreateSingleItem (line 223), the newly-added AccessibilityBridge.RegisterE2ETapTarget(deleteBtn, () => true, onDelete); call is indented at column 12 (matching the enclosing if keyword), while the sibling statements around it — deleteBtn.text = ..., deleteBtn.AddToClassList("delete-button"), and item.Add(deleteBtn) — are at column 16, the expected inner indent for statements inside if (onDelete != null) { ... }.

Step-by-step proof (CreateKeyValueItem, lines 160-169 of SectionBuilder.cs):

            if (onDelete != null)              // col 12 — if keyword
            {                                  // col 12 — open brace
                var deleteBtn = new Button(onDelete);                    // col 16 — sibling
                if (sectionKey != null && itemKey != null)               // col 16 — sibling
                    deleteBtn.name = $"{sectionKey}_remove_{itemKey}";   // col 20 — nested
                deleteBtn.text = MaterialIcons.Close;                    // col 16 — sibling
                deleteBtn.AddToClassList("delete-button");               // col 16 — sibling
            AccessibilityBridge.RegisterE2ETapTarget(deleteBtn, () => true, onDelete);  // col 12 — OFF
                item.Add(deleteBtn);                                     // col 16 — sibling
            }                                  // col 12 — close brace

The same pattern repeats at lines 218-224 in CreateSingleItem.

Why behavior is unaffected

C# treats whitespace as insignificant; the braces from the surrounding if (onDelete != null) { ... } unambiguously include this statement. deleteBtn is declared inside the same block, so the call would not even compile if it had really escaped the if. Both call sites compile and run correctly — this is purely visual.

Why it still matters

A reader scanning the method sees a statement at the if keyword's column and naturally reads it as belonging to the outer scope. That bias makes the next maintainer hesitate when answering "is this called only when onDelete is non-null?". In a file that is otherwise consistently formatted at 16-space inner indentation, the inconsistency reads as a bug even though it isn't.

Fix

Add four spaces to the start of each affected line (167 and 223), or run dotnet format over the file. Two-character edit per line, no functional change. Severity: nit.