Repository navigation
Commit d121e06
Protect notices: announce errors to screen readers (#52167)
* Protect notices: back the notice component with the @wordpress/ui Notice
The hand-rolled chassis — an icon switch, a message div and a dismiss button
over a dark banner — becomes `Notice.Root` with a `Notice.Description` and a
`Notice.CloseIcon`. The public props are unchanged, so all three call sites
keep working: `type` picks the intent, `message` fills the description,
`dismissable` draws the close control, and `duration` keeps its own timeout.
`type="warning"` now reaches an intent of its own. The old switch had no case
for it, so the fix-threat modal's inactive-extension warning fell through to
the info icon over the default chassis.
Only the floating toast passes a `spokenMessage`. It appears without a focus
change and no call site wraps it in a live region; the other two sit inside a
modal that is announced when it opens.
The stylesheet keeps the floating placement, the info notice's bottom margin
and the stacking context, and loses everything that painted the old chassis.
`container-type: normal` is new: `Notice.Root` sets `container-type:
inline-size`, and a size-contained child reports no width to the shrink-to-fit
fixed box, which collapsed the toast to 26px.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx
* Protect notices: keep the notice silent and its dismiss label
The Protect e2e suite failed on both counts, and both are this PR's doing.
`spokenMessage` defaulted the toast into `speak()`, which copies the message
into `#a11y-speak-polite` — so `getByText( 'Changes saved' )` matched two nodes
and Playwright's strict mode rejected it. The legacy notice never announced and
no call site sits in a live region, so this restores that. Announcing a toast is
a real improvement, but it belongs in its own change, applied consistently.
`Notice.CloseIcon`'s default label is "Dismiss", which renamed the control the
suite clicks. The existing translated string is passed through instead, so the
accessible name users already have survives the chassis swap.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx
* Protect notices: let the toast announce, and follow the three review notes
`speak()` keeps its own live region, so whether a call site sits in one was
never the right test — the reason the previous commit gave was wrong. The toast
reports an async result with no focus change, which is what `speak()` is for, so
it announces again; the two modal notices stay quiet, since their dialog is read
when it opens.
That is what broke the e2e suite: `speak()` copies the message into
`#a11y-speak-polite`, so `getByText( 'Changes saved' )` matches twice. Verified
in a browser that the notice is the earlier of the two, and scoped the
assertion to it.
Also: capitalise the changelog entry after its prefix and drop the repeated
word, and add `warning` to the component README's list of types, which this
change makes a real intent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx
* Protect notices: never hand a JSX message to the announcer
`Notice.Root` renders `spokenMessage` to a string during its own render. When
the message holds a component with hooks — every error notice interpolates a
`Link` — that component's hooks land in `Notice.Root`'s hook list, and the
`useEffect` on the next line reads a mismatched slot and throws. React then
unmounts the whole Protect dashboard, so any failed request blanked the page.
`safeRenderToString`'s try/catch does not help: the throw comes from the hook
after it.
Only a plain string is announced now, which covers the saved and saving toasts.
Announcing errors needs a spoken string carried alongside the JSX; that is worth
doing separately. The same trap is recorded in
`packages/newsletter/_inc/subscribers/components/modals/add-subscribers-modal.tsx`.
Two follow-ups from the same review: the e2e assertion is scoped to the app root,
because `speak()` leaves its text in the live region after the notice goes and
the helper runs seven times; and `NoticeState.type` gains `warning`, which the
component and README already offer.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx
* Protect notices: bundle the two packages the close control pulls in
`Notice.CloseIcon` reaches `@wordpress/theme` and `@wordpress/private-apis`
through `IconButton` and its tooltip, so the build started externalising them to
the `wp-theme` and `wp-private-apis` script handles. Protect registers no shim
for those, and WordPress refuses to enqueue a bundle with an unmet dependency —
the admin page would render empty wherever core does not supply them.
They are bundled instead, which is what the Jetpack plugin's own admin bundles
do for the same reason. Verified by rebuilding: both handles are gone from
`build/index.asset.php`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx
* Protect notices: correct a comment the e2e scoping outdated
The Firewall block still said the "Changes saved" assertion stays page-scoped
on purpose. It does not any more.
The changelog entry becomes `minor`, matching the same migration for the
Jetpack plugin in #52015: every Protect notice is restyled, one turns amber,
and the toasts start announcing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx
* Protect notices: announce error notices to screen readers
The announcement was derived from the message, and only a plain string
was ever handed to `Notice.Root` — anything else corrupts its hook order.
`showErrorNotice` always builds JSX, because it interpolates a support
link, so failed saves stayed silent while "Changes saved." did not.
Carry the announcement in the notice state instead: `showErrorNotice`
sets `spokenMessage` from the plain string it already receives, and the
component prefers it over the message. It still resolves to null rather
than undefined, so `Notice.Root` never falls back to rendering children.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx
* Protect notices: speak the whole error, and speak it every time
The announcement dropped the "Please try again or contact support." sentence
because it only exists with interpolation tags. Stripping the tags off the
translated string gives the plain sentence at no translation cost, so the
announcement now carries what the notice says.
Each notice also gets an identity, and the toast is keyed on it. Without that a
repeat of the same error re-renders with identical props, the announcing effect
never re-runs, and the second failure is silent. `use-fixers-query` is the path
that reaches it: its polling error fires the same message with no other notice
in between.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx
* Protect notices: re-announce without stealing focus
Keying the toast on the notice id remounted it, which destroyed the node focus
was in: a keyboard user reading the error, with focus on its support link or
dismiss button, lost focus to the body when the next notice arrived. In an
accessibility change that is not a fair trade.
The announcement now varies instead of the element. A repeat alternates a
trailing non-breaking space — the same device `@wordpress/a11y` uses for this —
so the announcing effect re-runs while the element stays mounted. Two identical
errors in a row take consecutive ids, so the parity always flips between them;
anything that arrives in between announces on its own text.
Two more from the same review: the provider stamps the id, so no future writer
can forget it and silently break the repeat, and the tag strip no longer names
`supportLink`, so renaming the tag cannot silently stop it working.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx
* Protect notices: announce per notice, not per distinct message
The parity nonce was wrong. `Notice.Root` announces when the value it is handed
changes, so the previous revision alternated a trailing space keyed on the
notice id — but the id advances once per stored notice while the announcement
happens once per committed render. Any event that stores an even number of
notices in one commit leaves the parity where it was, and the repeat is silent
again. `use-fixers-query` is exactly that shape: one instance per threat row
plus the modals, all sharing a query, all raising the same error in one commit.
So the announcement moves here, keyed on the notice, and `Notice.Root` is handed
`null` unconditionally — it never renders a message to a string now, which is
what the crash came from. `speak()` handles an identical repeat itself.
Two more from the same review: the id is stamped outside the state updater, and
the component README documents `spokenMessage` and `id`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx
* Protect notices: tie the dismiss timer to the notice as well
`duration` keyed its timer on the message, so a second identical success
inherited whatever was left of the first one's 7.5 seconds instead of starting
again. The notice's id is what says "this is a different notice", and the
announcement already uses it.
Also from review: the mid-render rationale is stated once, on the prop that
carries it, and the new dependency is filed in alphabetical order.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx
* Protect notices: drop a comment's claim of a nonexistent dialog
The comment above `spokenMessage` said the modal notices are read when their
dialog opens. Protect has no dialog: `components/modal/index.jsx` renders a
plain `<div>` with no `role="dialog"`, no `aria-modal` and no focus move into
the window, so nothing reads those notices. Keep the half that is true and
load-bearing -- why the prop takes strings only -- and drop the rest.
Also cut the `requestMap` comment in the plugin's webpack config to the live
constraint. The mechanism is already spelled out in three other configs, and a
fourth copy drifts. The code is unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx
* Protect notices: keep the joint-bundling constraint on the requestMap
The pair has to be bundled together: `@wordpress/theme` opts in and calls
`lock()` at module scope, and `@wordpress/private-apis` keeps its consent map
per module instance. Bundle one and externalize the other and the two maps
diverge, which surfaces at runtime as "Cannot unlock an object that was not
locked before" or a duplicate opt-in, with nothing pointing back at the build
config. The full account lives in #48173.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx
* Protect notices: point at the requestMap rationale instead of restating it
Three other webpack configs already carry this reasoning in full. A fourth copy
is the one outcome that guarantees they drift, so this leaves the constraint and
the reference and nothing else.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx
* Protect notices: name the tag the spoken message strips
CodeQL reads a `<[^>]+>` strip as HTML sanitization and files it high severity
(js/incomplete-multi-character-sanitization). Nothing here reaches innerHTML —
the result goes to `speak()`, which writes textContent — but the generic form
buys nothing over naming the one placeholder the string actually carries, and
`createInterpolateElement` breaks just as loudly if that name ever changes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx
* Protect notices: answer the review on the announcement
Rename the per-notice stamp from `id` to `noticeId`. It is a sequence number
rather than an identity, and `id` would have become a real DOM attribute the
day `Notice.Root` is handed rest props.
Say why only the toast announces, and drop the comparison with `Notice.Root`'s
own announcement: WordPress/gutenberg#82737 proposes removing that, so the
comparison would describe a mechanism a reader cannot reach.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lo29yrft9KqShYK3p1wQe6
* Protect notices: announce a failing fixers poll once per streak
Every failed poll produces a new Error object, so keying the effect on it
re-announced the same message assertively every five seconds for as long as
the poll kept failing. Key on `isError` instead: one announcement per streak,
and another only once a success has come in between.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lo29yrft9KqShYK3p1wQe6
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Douglas <douglas.henri@automattic.com>1 parent c38909e commit d121e06
7 files changed
Lines changed: 66 additions & 28 deletions
File tree
- projects/plugins/protect
- changelog
- src/js
- components/notice
- data/scan
- hooks
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
32 | 32 | | |
33 | 33 | | |
34 | 34 | | |
| 35 | + | |
35 | 36 | | |
36 | 37 | | |
37 | 38 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
8 | | - | |
9 | | - | |
10 | | - | |
11 | | - | |
| 8 | + | |
12 | 9 | | |
13 | 10 | | |
14 | 11 | | |
15 | 12 | | |
16 | | - | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
1 | 2 | | |
2 | 3 | | |
3 | 4 | | |
| |||
17 | 18 | | |
18 | 19 | | |
19 | 20 | | |
| 21 | + | |
| 22 | + | |
20 | 23 | | |
21 | 24 | | |
22 | 25 | | |
| |||
25 | 28 | | |
26 | 29 | | |
27 | 30 | | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
28 | 41 | | |
29 | 42 | | |
30 | 43 | | |
| |||
36 | 49 | | |
37 | 50 | | |
38 | 51 | | |
39 | | - | |
| 52 | + | |
40 | 53 | | |
41 | 54 | | |
42 | 55 | | |
| |||
46 | 59 | | |
47 | 60 | | |
48 | 61 | | |
49 | | - | |
50 | | - | |
51 | | - | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
52 | 65 | | |
53 | 66 | | |
54 | 67 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
144 | 144 | | |
145 | 145 | | |
146 | 146 | | |
147 | | - | |
| 147 | + | |
| 148 | + | |
148 | 149 | | |
149 | | - | |
| 150 | + | |
150 | 151 | | |
151 | 152 | | |
152 | 153 | | |
153 | 154 | | |
154 | | - | |
| 155 | + | |
155 | 156 | | |
156 | 157 | | |
157 | 158 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
11 | 14 | | |
12 | 15 | | |
13 | 16 | | |
| |||
18 | 21 | | |
19 | 22 | | |
20 | 23 | | |
| 24 | + | |
| 25 | + | |
21 | 26 | | |
22 | 27 | | |
23 | 28 | | |
24 | | - | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
25 | 39 | | |
26 | 40 | | |
27 | 41 | | |
| |||
66 | 80 | | |
67 | 81 | | |
68 | 82 | | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
69 | 89 | | |
70 | 90 | | |
71 | 91 | | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
72 | 95 | | |
73 | 96 | | |
74 | | - | |
75 | | - | |
76 | | - | |
77 | | - | |
78 | | - | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
79 | 105 | | |
80 | | - | |
81 | | - | |
82 | | - | |
83 | | - | |
84 | | - | |
85 | | - | |
86 | | - | |
87 | | - | |
88 | | - | |
89 | | - | |
| 106 | + | |
90 | 107 | | |
91 | 108 | | |
92 | 109 | | |
| |||
0 commit comments