feat: Test `priority: "important"` notifications in NVDA using Guidepup by smockle · Pull Request #7 · github/arianotify-polyfill

smockle · 2024-10-22T02:25:17Z

Overview

Part of https://github.com/github/core-ux/issues/346 (requires Hubber login).
Fixes https://github.com/github/core-ux/issues/374 (requires Hubber login).

Follow-up to #4, which added a similar test for VoiceOver.

This PR adds a test for priority: "important" notifications in NVDA. The test is written using Guidepup and Playwright. It can be run with npm test.

Below is a screen recording, with audio, of the test running (and passing). The “Suggested Text” example page is opened, and NVDA is started. The textarea is focused, and “a” is typed. The word “acceptable” is suggested as a completion, and the explanatory message “Press right arrow to commit suggestion” is conveyed too. The test validates the suggestion is conveyed.

Screen.Recording.2024-10-21.at.10.16.39.PM.mov

Next steps

Run tests in CI; this is tracked by https://github.com/github/core-ux/issues/375 (requires Hubber login).

… in Windows in Git, GitHub Desktop, Explorer, VS Code, etc.—even so, they won’t work in file:// urls in Edge.

smockle · 2024-10-22T02:31:21Z

tests/nvda.spec.mjs

+// - Install the NVDA Remote Access addon (https://nvdaremote.com/download/)
+// - In NVDA Tools > Remote > Options…:
+//   - Check "Auto-connect to control server on startup"
+//   - Select "Host control server"
+//   - Set "Port" to "6837"
+//   - Set "Key" to "guidepup"
+// (settings are from https://github.com/guidepup/nvda/blob/main/nvda/userConfig/remote.ini)


guidepup/nvda is a portable NVDA install with this add-on preinstalled and preconfigured. guidepup/setup installs that version: https://github.com/guidepup/setup/blob/82179ec8915680344d0db320422dd18e29593eb9/src/windows/installNvda.ts#L11.

However, I wanted to use upstream NVDA and NVDA Remote Access, so I worked out this manual alternative. If you don’t do these steps, Guidepup can open NVDA, but it can’t connect to it (for control and logging).

accessibility-bot · 2024-10-22T02:34:07Z

👋 Hello and thanks for pinging us! You've entered our first responder queue. An accessibility first responder will review this soon.

💻 On PRs for our review: please provide a review environment with steps to validate, screenshots (with alt text), or videos demonstrating functionality we should be checking. This will help speed up our review and feedback cycle.
⚠️ If this is urgent, please visit us in #accessibility on Slack and tag the first responder(s) listed in the channel topic.

smockle · 2024-10-22T02:36:33Z

tests/nvda.spec.mjs

+// - In Windows Settings, turn on “Developer Mode” (this allows symbolic links)
+// - In an PowerShell (Administrator session):
+//   - Run `mkdir "C:\Program Files (x86)\NVDA\userConfig"`
+//   - Run `cmd /c mklink /d "C:\Program Files (x86)\NVDA\userConfig\addons" "%APPDATA%\nvda\addons"`


guidepup looks for a cert file at a very specific path: https://github.com/guidepup/guidepup/blob/fadd8faa97222bc716d717063f7cfb9ea541cb83/src/windows/NVDA/NVDAClient.ts#L113-L129.

If you install upstream NVDA and NVDA Remote Access—instead of guidepup/nvda—the cert file won’t be where guidepup expects, and guidepup will throw a misleading “NVDA not installed” error.

These steps create a symbolic link at the necessary path, so guidepup can find the cert file.

smockle · 2024-10-22T02:40:49Z

tests/nvda.spec.mjs

+// - In cmd:
+//   - Run `REG ADD HKCU\Software\Guidepup\Nvda`
+//   - Run `REG ADD HKCU\Software\Guidepup\Nvda /v guidepup_nvda_0.1.1-2021.3.1 /t REG_SZ /d "C:\Program Files (x86)\NVDA\\"`
+// (version is from https://github.com/guidepup/setup/blob/82179ec8915680344d0db320422dd18e29593eb9/package.json#L60C27-L60C41)


guidepup checks the Windows Registry to find where NVDA is installed: https://github.com/guidepup/guidepup/blob/fadd8faa97222bc716d717063f7cfb9ea541cb83/src/windows/NVDA/getNVDAInstallationPath.ts#L13, because guidepup/setup writes to a key there: https://github.com/guidepup/setup/blob/82179ec8915680344d0db320422dd18e29593eb9/src/windows/updateNvdaRegistryData.ts#L7-L13.

These steps are a manual alternative to running guidepup/setup.

smockle · 2024-10-22T02:44:14Z

tests/nvda.spec.mjs

+import { test, expect } from "@playwright/test";
+import { nvda, WindowsKeyCodes, WindowsModifiers } from "@guidepup/guidepup";
+
+// Pre-requisites:


The purpose of these manual “pre-requisites” is twofold:

To avoid running scripts off the Internet (specifically, guidepup/setup).

To avoid installing/using a non-official NVDA distribution (specifically, guidepup/nvda).

smockle · 2024-10-22T02:45:37Z

tests/nvda.spec.mjs

+    // Assert that the spoken phrases are as expected
+    const spokenPhraseLog = JSON.stringify(await nvda.spokenPhraseLog());
+    expect(spokenPhraseLog.includes("Suggestion: acceptable")).toBe(true);
+    // expect(spokenPhraseLog.includes("Press right arrow to commit suggestion")).toBe(true); // FIXME: Commenting because this fails, though it _should_ pass.


The screen recording attached to this issue body demonstrates that this notification is indeed sent, but for some reason guidepup isn’t retrieving it. I checked both spokenPhraseLog and lastSpokenPhrase.

smockle · 2024-10-22T02:51:32Z

.github/workflows/publish.yml

          registry-url: https://registry.npmjs.org/
          cache: npm
-      - run: npm ci
+      - run: npm ci --legacy-peer-deps


The latest version of guidepup/playwright has a peer dep on @guidepup/guidepup@0.22.1: https://github.com/guidepup/guidepup-playwright/blob/34c3973dd98e19c81f468352e13bac5b8434b28f/package.json#L59. That version has a transient dependency on ffmpeg-static, which is uninstallable on Windows 64-bit ARM.

The latest version of @guidepup/guidepup (0.24.0) dropped the transient dependency on ffmpeg-static (in guidepup/guidepup#90); using --legacy-peer-deps allows that version to satisfy @guidepup/playwright’s peer deps.

smockle · 2024-10-22T02:53:44Z

examples/suggested-text/index.html

      trieRoot.insert("world");
    </script>
-    <script src="./ariaNotify-polyfill.js"></script>
+    <script src="../../ariaNotify-polyfill.js"></script>


Even after configuring symlink support in Git, GitHub Desktop, Explorer, VS Code, etc., Edge would not follow symlinks to load JS. So, I’ve removed the symlinks and updated these paths accordingly. We should double-check that this doesn’t break the examples on https://github.github.com/ariaNotify-polyfill/.

This will break GitHub pages. We should perhaps instead copy the polyfill, or now it's public this can link to the unpkg cdn?

Darn. Thanks for confirming it’d be a problem!

I found a solution:

In 11df7c6, I restored the symbolic links. Now GitHub Pages should keep working.

In 0f87504, I switched to a different approach for loading static files in Playwright tests. Now, instead of using file:// urls, it intercepts requests to http://. This makes symbolic links to the script work in Edge.

I tested this solution in both macOS and Windows, and it works in both. I prefer this approach (vs linking to unpkg), since it allows testing a local, unpublished version.

keithamus

Overall LGTM but this cannot ship while the examples reference content outside of their directory, as it'll break. We need to resolve this somehow before merging, such as copying the poylfill into the examples, or using a hard-link.

keithamus · 2024-10-22T09:21:21Z

examples/suggested-text/index.html

      trieRoot.insert("world");
    </script>
-    <script src="./ariaNotify-polyfill.js"></script>
+    <script src="../../ariaNotify-polyfill.js"></script>


This will break GitHub pages. We should perhaps instead copy the polyfill, or now it's public this can link to the unpkg cdn?

…does_ work with symbolic links, in Edge. Kudos ruifigueira for the suggestion, in microsoft/playwright#13968 (comment).

smockle added 9 commits October 21, 2024 17:32

test: Add NVDA test

a9bc830

fix: Make installable on Windows

1c3e1f0

docs: Update comment

581d208

docs: Update Windows prerequisites

a48ee57

fix: Install a version of Guidepup that doesn’t require ffmpeg-static

b5a588c

chore: Update package-lock.json

53aeaf8

docs: Correct escape characters in registry editor instructions

1cbde58

fix: Remove symbolic links. With configuration, they can be supported…

44da297

… in Windows in Git, GitHub Desktop, Explorer, VS Code, etc.—even so, they won’t work in file:// urls in Edge.

fix: Make NVDA test actually work

738ccd1

smockle requested review from a team as code owners October 22, 2024 02:25

smockle requested a review from accessibility-bot October 22, 2024 02:25

smockle commented Oct 22, 2024

View reviewed changes

keithamus suggested changes Oct 22, 2024

View reviewed changes

smockle added 2 commits October 22, 2024 09:04

fix: Restore symbolic links so GitHub Pages works

11df7c6

fix: Use a different approach to serving static files in tests that _…

0f87504

…does_ work with symbolic links, in Edge. Kudos ruifigueira for the suggestion, in microsoft/playwright#13968 (comment).

smockle requested a review from keithamus October 22, 2024 13:27

keithamus approved these changes Oct 22, 2024

View reviewed changes

smockle merged commit 0f8cc83 into main Oct 22, 2024

smockle deleted the smockle/nvda-test branch October 22, 2024 14:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Test `priority: "important"` notifications in NVDA using Guidepup#7

feat: Test `priority: "important"` notifications in NVDA using Guidepup#7
smockle merged 11 commits intomainfrom
smockle/nvda-test

smockle commented Oct 22, 2024

Uh oh!

smockle Oct 22, 2024 •

edited

Loading

Uh oh!

accessibility-bot commented Oct 22, 2024

Uh oh!

smockle Oct 22, 2024

Uh oh!

smockle Oct 22, 2024

Uh oh!

smockle Oct 22, 2024

Uh oh!

smockle Oct 22, 2024

Uh oh!

smockle Oct 22, 2024

Uh oh!

smockle Oct 22, 2024

Uh oh!

keithamus Oct 22, 2024

Uh oh!

smockle Oct 22, 2024 •

edited

Loading

Uh oh!

keithamus left a comment •

edited

Loading

Uh oh!

keithamus Oct 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

smockle commented Oct 22, 2024

Overview

Next steps

Uh oh!

smockle Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

accessibility-bot commented Oct 22, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smockle Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keithamus left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

smockle Oct 22, 2024 •

edited

Loading

smockle Oct 22, 2024 •

edited

Loading

keithamus left a comment •

edited

Loading