test(video-player): wait for widget after tab clicks, fix poster race

[samuelreichert]

Pull request type

Test related change (New E2E test, test automation, etc.)

---

Description

Fixes intermittent nightly failures in video-player-web and accessibility-helper-web E2E tests. Both share the same class of bug: actions fired before Mendix finishes re-rendering the DOM.

video-player-web — three root causes:

1. Tab page tests — after .click() on a tab, tests asserted toBeVisible() on iframes immediately. Mendix renders tab content asynchronously; the fixture's waitForMendixApp only patches page.goto, not tab navigation clicks. Added waitForWidget after each tab click.

2. Aspect ratio test — same tab-click race: boundingBox() called on elements in tabs 2 and 3 without waiting for them to be present in the DOM.

3. Poster screenshot — page.evaluate with a standalone new Image() resolves when the image loads into memory, not when the <video> element has painted the poster frame. Switched to videoLocator.evaluate(el => ...) to tie the wait to the actual element. Also added scrollIntoViewIfNeeded so the widget is in viewport before screenshotting.

accessibility-helper-web — one root cause, three affected tests:

4. Double-click on actionButton2 — first click triggers a Mendix re-render that detaches the button from the DOM; second click immediately after races against re-attach and times out after 10s. Added waitForMendixApp between the two clicks in all three with multiple targets tests that used this pattern.

[github-actions]

AI Code Review

⚠️ Approved with suggestions — low-severity items only, safe to merge

---

What was reviewed

File	Change
packages/pluggableWidgets/accessibility-helper-web/e2e/AccessibilityHelper.spec.js	Added waitForMendixApp + DOM-stability assertion between double-clicks in three with multiple targets tests
packages/pluggableWidgets/video-player-web/e2e/VideoPlayer.spec.js	Replaced waitForDataReady with waitForWidget; added waitForWidget after tab clicks; rewrote poster-image wait to use in-page videoLocator.evaluate

Skipped (out of scope): dist/, pnpm-lock.yaml, CHANGELOG (test-only changes — no runtime/XML/behavior change, entry not required)

All CI checks: could not fetch — gh pr checks requires interactive approval in this environment.

---

Findings

⚠️ Low — if (i.complete) r() races against onload

File: packages/pluggableWidgets/video-player-web/e2e/VideoPlayer.spec.js lines 122–131

Problem: The guard if (i.complete) r() fires synchronously after assigning i.onload = r, so if the browser already cached the image and set complete = true before the assignment, r() is invoked while onload is also set to r — meaning r could be called twice (once immediately, once by the load event). Promises ignore double-resolution, so this is harmless in practice, but it is a subtle correctness issue and could mask future bugs if the surrounding promise is refactored.

Fix: Check i.complete before wiring the handlers, or use the pattern below to guarantee single resolution:

await videoLocator.evaluate(el =>
    el.poster
        ? new Promise(r => {
              const i = new Image();
              i.onload = i.onerror = r;
              i.src = el.poster; // assign src after handlers so onload fires if cached
              if (i.complete) r();
          })
        : Promise.resolve()
);
// ↑ current code is fine because Promise.resolve() ignores the double call,
// but for clarity prefer assigning src last (as already done) and removing the
// i.complete guard in favour of: i.decode().then(r, r)

Or the cleaner modern form:

await videoLocator.evaluate(el =>
    el.poster
        ? new Promise(r => { const i = new Image(); i.onload = i.onerror = r; i.src = el.poster; })
        : Promise.resolve()
);

(i.complete + onload is redundant — if the browser has it cached, onload still fires synchronously on the next microtask when src is assigned.)

---

⚠️ Low — not.toHaveAttribute("data-disabled", "true") assertion intent may not match actual race guard purpose

File: packages/pluggableWidgets/accessibility-helper-web/e2e/AccessibilityHelper.spec.js lines 62, 95, 126

Problem: The assertion expect(...actionButton2).not.toHaveAttribute("data-disabled", "true") is added as a DOM-stability guard to wait for Mendix to finish re-rendering after the first click. However, if the actual re-render sets data-disabled to "true" at any point during the cycle, this assertion could pass and still let the second click fire too early. The assertion checks attribute state, not that the element has re-attached and is interactive. A more robust guard would be toBeEnabled() or toBeVisible() which Playwright auto-retries against the live DOM.

Fix:

await waitForMendixApp(page);
await expect(page.locator(".mx-name-actionButton2")).toBeVisible(); // ensures element is re-attached and interactive
await page.click(".mx-name-actionButton2");

The toBeVisible() form also communicates intent more clearly.

---

Positives

• Using waitForWidget (which wraps expect(locator).toBeVisible()) after tab clicks is exactly the right pattern — it leverages Playwright's built-in auto-retry instead of a hardcoded waitForTimeout, eliminating the flakiness class entirely.
• The videoLocator.evaluate(...) approach for poster-readiness is a meaningful improvement over waitForDataReady: it ties the wait to the specific element's state rather than the whole page's network idle, making the test faster and more accurate.
• waitForMendixApp between double-clicks in the accessibility-helper tests is a well-targeted fix; it addresses the exact root cause (DOM detach on first click) without over-waiting.
• waitForDataReady import removed cleanly — no dead code left behind.