Protect notices: announce errors to screen readers

[CGastrell]

Follows #52160, and stacks on it — review that one first.

Proposed changes

Error notices in Jetpack Protect never reach screen readers. This makes them announce.

• Notice announces its own message with speak(), in an effect keyed on the notice, and hands Notice.Root null unconditionally.
• NoticeState gains a plain-text spokenMessage and an id. showErrorNotice sets the first from the error string it already receives; the provider stamps the second on every notice it stores.

Why the component announces rather than Notice.Root. Notice.Root renders whatever it is handed for spokenMessage to a string during its own render. Every error message interpolates a support Link, and rendering a hook-using component that way appends its hooks to Notice.Root's hook list — the useEffect on the next line then reads a mismatched slot, throws, and React unmounts the whole Protect dashboard. That was a live bug in #52160, guarded there by announcing plain strings only, which left errors silent.

Notice.Root also announces only when the value changes, so two identical errors in a row would announce once. data/scan/use-fixers-query.ts:153 reaches exactly that: its polling error fires the same message with no other notice in between.

Announcing from the component settles both. Nothing is serialized mid-render, so the crash is gone by construction rather than by guard; the effect is keyed on the notice's id, so a repeat announces again; speak() handles the repeated string itself; and the element is never remounted, so focus inside a notice survives.

Three details worth knowing if you touch this again:

• The announcement carries the whole sentence, including "Please try again or contact support." Stripping the interpolation tags off the already-translated string costs no new translation.
• Errors announce assertively, everything else politely. That register is chosen in Notice, not by the design system.
• @wordpress/a11y is newly declared, but costs nothing: it externalizes to the wp-a11y script handle the bundle already carried, and the build grows by 333 bytes.

Related product discussion/links

• Jetpack admin UI: standardize every notice and smooth the frame transitions
• Protect notices: back the notice component with the @wordpress/ui Notice #52160 — the migration this completes.

Does this pull request change what data or activity we track or use?

No.

Testing instructions

On a connected site with Jetpack Protect active, running this branch. I ran these steps on a local site; the readings below are what I measured.

Watch the live regions with a MutationObserver rather than reading their text afterwards — a region keeps its previous content, so reading it cannot tell a fresh announcement from a leftover one:

for ( const id of [ 'a11y-speak-polite', 'a11y-speak-assertive' ] ) {
	new MutationObserver( ms => ms.forEach( m => {
		const t = m.type === 'characterData' ? m.target.textContent : [ ...m.addedNodes ].map( n => n.textContent ).join( '' );
		if ( t ) console.log( id, JSON.stringify( t ) );
	} ) ).observe( document.getElementById( id ), { childList: true, characterData: true, subtree: true } );
}

• Open Jetpack Protect → Settings and toggle Account protection. a11y-speak-polite should be written with Saving Changes… and then Changes saved.
• Now make the requests fail — force a 500 on /wp-json/jetpack-protect/** — and toggle again. The red error toast should appear, and a11y-speak-assertive should be written with the error text, ending "Please try again or contact support."
• The repeat case. Trigger the same failure twice with no other notice in between — the Enable Firewall button on the Firewall tab is the path, since it raises no "Saving Changes…" first. Both failures must produce a write; the second carries a trailing non-breaking space, which is how speak() marks a repeat.
• Confirm the dashboard survives it: document.getElementById( 'jetpack-protect-root' ).innerHTML.length must stay non-zero, and the console must show no TypeError. That is the crash Protect notices: back the notice component with the @wordpress/ui Notice #52160 fixed, and this change must not reintroduce it.
• With a screen reader on, the failed save should be spoken. Before this change it was silent.
• Put focus on the toast's dismiss button and trigger another failure from Enable Firewall. Focus must stay on that button — an earlier revision remounted the notice and dropped focus to the body. Use that path, not the Settings toggle: its "Saving Changes…" notice is not dismissable, so the button legitimately goes away there.

Automated: ESLint and tsgo --noEmit are clean, jetpack build plugins/protect succeeds, and build/index.asset.php still carries neither wp-theme nor wp-private-apis.

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

Follow this PR Review Process:

1. Ensure all required checks appearing at the bottom of this PR are passing.
2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
3. You can use GitHub's Reviewers functionality to request a review.
4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

---

Protect plugin:

No scheduled milestone found for this plugin.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

[jp-launch-control]

Code Coverage Summary

This PR did not change code coverage!

That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷

Full summary · PHP report · JS report

[github-actions]

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

• To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack or WordPress.com Site Helper), and enable the update/protect-notice-announce-errors branch.
• To test on Simple, run the following command on your sandbox:

bin/jetpack-downloader test jetpack update/protect-notice-announce-errors

bin/jetpack-downloader test jetpack-mu-wpcom-plugin update/protect-notice-announce-errors

Interested in more tips and information?

• In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
• Read more about our development workflow here: PCYsg-eg0-p2
• Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

[dhasilva]

Reviewed with /jetpack-review-pr (standard depth — 82 lines, 1 project). No blockers; four suggestions, three inline and one below.

Scope note: this targets update/protect-notice-wpds (#52160's head), not trunk, so I reviewed only the incremental diff and judged it against the tree as #52160 leaves it, reading origin/update/protect-notice-wpds whenever I needed the "before" state. Two things I deliberately did not file here because they belong to #52160: the @wordpress/ui migration itself, and the mid-render renderToString hook-order hazard it guards against — I only verified that this PR's spokenMessage={ null } still closes that hole, which it does, since RootProps defaults spokenMessage to children. That guard is a genuine prerequisite, so #52160 should land first.

The mechanism is correct on every axis I could check

Verified against the installed @wordpress/ui@0.21.0 and @wordpress/a11y@4.54.0 sources rather than from the description:

• The announcement actually reaches AT. speak() only writes to #a11y-speak-polite / #a11y-speak-assertive; it doesn't create them. setup() does, under domReady. Protect's built build/index.asset.php already lists wp-a11y on trunk, before this PR — so the regions exist long before any user action creates a notice. No inject-and-populate-in-one-render trap, and this also confirms your claim that the new dependency costs no new script handle.
• No double announcement. Notice.Root deliberately renders no role (its own comment: "no role attribute needed as it would cause double announcements") and announces solely through useSpokenMessage. Passing spokenMessage={ null } makes safeRenderToString return undefined, so that effect no-ops. Exactly one speak() per notice.
• null vs omitting — the comment on that line is load-bearing and correct. RootProps destructures spokenMessage = children, and a destructuring default fires only on undefined, so omitting the prop would feed the JSX message to renderToString during Root's own render. Keep it.
• Politeness reproduces @wordpress/ui's own getDefaultPoliteness() exactly, so the register stays consistent with the design system even though it's now chosen locally.
• Repeat announcements genuinely work: filterMessage appends \u00A0 when the message matches the previous one, and clear() empties the region before the write. The id key is what gets the effect to fire at all in that case — it's necessary, not belt-and-braces.
• Tag stripping isn't redundant: speak() strips tags internally but replaces each with a space, which would yield "contact support ." — your replace( /<\/?[^>]+>/g, '' ) avoids that.

Also cleared: no announcement storms (the effect's deps are all primitives or strings); no remount, since protect-app/index.jsx:95 renders the notice at a static JSX position outside the router <Outlet />, so truthy→truthy is a prop update and focus inside the toast survives; the id stamp is computed outside the state updater, so React re-invoking the updater can't bump it twice; showErrorNotice() with no argument still works via the message || __( 'An error occurred.' ) fallback; id never leaks to the DOM; and there's no StrictMode to double-fire in dev.

The auto-dismiss dep change (message → id) is a small improvement rather than a regression — two identical success messages now restart the 7.5s timer where before they wouldn't. Both modal call sites pass no duration, so nothing changes for them.

Changelog is valid and correctly scoped: I diffed the changelog directory across trunk, update/protect-notice-wpds and this branch — update-protect-notice-wpds is #52160's and inherited, and this PR adds exactly one file. Dependency @wordpress/a11y@4.54.0 is in correct alphabetical position and matches the WordPress train the project is pinned to.

One suggestion that doesn't touch this diff

[suggestion] A failing fixers poll now interrupts the screen reader assertively every 5 seconds.

projects/plugins/protect/src/js/data/scan/use-fixers-query.ts:148-155 is the call site your description names as the motivating repeat case, so it's worth being deliberate about which half of it you want.

That effect fires on every change of fixersQuery.error, and each failed fetch produces a fresh Error — so each one calls showErrorNotice → a new id → a new assertive speak(). Meanwhile refetchInterval (L120-143) returns 5000, then 15000, for as long as any threat is in_progress and not stale. The reset on L152 writes to [ QUERY_FIXERS_KEY ] while the query's key is [ QUERY_FIXERS_KEY, threatIds ], so under TanStack v5's exact matching it doesn't clear the data that keeps the poll alive.

Visually this already re-renders an identical toast and reads as static. Announced assertively it becomes an interruption every 5 seconds, and assertive cuts off whatever the user was reading. Before this PR that path was silent, so the regression is introduced here even though the underlying loop isn't.

Worth separating the two cases: a repeat that follows a user action (the Enable Firewall path in your testing instructions) should announce; a repeat produced by a background poll shouldn't, or should be rate-limited. Simplest lever is at the call site — announce the polling failure once per error streak rather than per error object.

Test coverage

Protect has no JS test files and no jest wiring at all, so this PR couldn't add unit tests without first bootstrapping the infrastructure — pre-existing and not this change's fault. It does mean three subtle invariants (announce once per notice; assertive only for errors; an identical repeat announces again) are guarded only by the manual MutationObserver procedure in the description, which is the right way to test this. projects/js-packages/licensing/components/activation-screen-error/test/component.jsx is the nearest precedent in the monorepo for asserting on @wordpress/ui Notice announcements, if someone stands jest up here later.

Verdict: minor issues — can merge after addressing. The assertive-repeat item is the one with user-visible consequences and the one I'd want a decision on before merge; the other three are small.

Generated by Claude.

[dhasilva]

[suggestion] This explains the alternative the PR rejected rather than the code that's here.

Notice.Root is handed spokenMessage={ null } twenty-five lines below and announces nothing, so the mechanism the comment cites isn't in play in this file at all. The reader has to work out for themselves that the actual constraint is React's own by-value comparison of the dep array — which applies to this effect regardless of what Notice.Root does.

It's the "present-tense contrast with an alternative that isn't in the tree" shape: once this merges, the delegated-announcement version is nowhere a reader can reach. Your PR description already argues it at length, which is where it belongs.

Keep the invariant, drop the comparison:

// Keyed on the id: effect deps compare by value, so an identical repeat would not re-fire.

(The other new comment, on spokenMessage={ null }, is the opposite case — spokenMessage = children is a live default in RootProps that a reader can reach and would trip over. Keep that one as is.)

Generated by Claude.

[dhasilva]

[suggestion] The floating guard arrived here without its rationale.

#52160 carried the reason on the prop it guarded — // Only the toast announces: the modal notices are read when their dialog opens. This PR deletes that line and moves the floating test into the new effect without carrying anything across, so a reader now meets if ( floating && … ) with no explanation of why fix-threat-modal and user-connection-needed-modal are excluded. It also couples an announcement to what is otherwise a styling flag, which deserves a sentence.

One caveat before you restore the old wording: it isn't true. Reviewing #52160 I checked, and Protect has no dialog — components/modal/index.jsx is a plain <div>, and git grep -n 'role="dialog"\|aria-modal\|Dialog\.' -- projects/plugins/protect/src returns nothing. The modal notices aren't read when their dialog opens; they aren't read at all, and focus stays on the trigger button.

So the honest version of this comment says the modal notices are silent, which is really a behaviour question rather than a comment question — worth deciding which you want here.

The README compounds it either way: README.md:18 documents spokenMessage as "the plain-text announcement for screen readers" with no mention that nothing is announced at all unless floating is set, so a future caller will reasonably expect a modal notice to speak. Worth qualifying that sentence alongside whatever you put here.

Generated by Claude.

[dhasilva]

[suggestion] id is a sequence number, not an identity, and it collides with a DOM attribute name.

++noticeId runs on every setNotice call, so re-showing the same logical notice produces a different value — which is precisely the behaviour you want here, but not what "id" says. README.md states it as "id identifies the notice", which is the opposite of what it does.

It also lands on a component whose root renders a div, so the day someone adds { ...rest } to WPNotice.Root it silently becomes a DOM id attribute holding a bare integer.

noticeId or announcementKey says what it is and can't collide. Cheap to rename now while the only consumers are three files.

(Worth keeping the current shape otherwise — computing the stamp outside the setNoticeState updater is the careful version, since React can re-invoke an updater and would otherwise bump the counter twice.)

Generated by Claude.

[simison]

Notice announces its own message with speak(), in an effect keyed on the notice, and hands Notice.Root null unconditionally.

Note that we're considering removing this:

• UI: Remove automatic Notice announcements WordPress/gutenberg#82737