Fix top 3 React Doctor findings: keyboard a11y for click targets + collapse redundant size axes#430
Merged
Merged
Conversation
… plaintext XEP-0373 signcrypt mandates both signing and encryption, but decrypt() was delivering the plaintext regardless of signature status. A compromised server could inject unsigned messages encrypted to the recipient's public key (published on PEP) that would display with only a subtle "untrusted" badge. Add a signature enforcement gate in OpenPGPPluginBase.decrypt() that rejects messages with no signature (Case B) or with a failed signature when the sender key is available (Case A). The deferred verification path for first-contact (sender key not yet cached) is preserved. Introduce a 'rejected' trust level so the UI can unambiguously distinguish "pending verification" from "signature integrity failure", rendered as a red ShieldAlert icon with a client-side placeholder body.
Three issues found by Codex review of the signature enforcement commit: 1. Rejections still included encryptedPayloadXml in the stanzaDecrypt return, causing retryDecryptSingle to short-circuit (return null) before reaching the rejection guard — the retry path could never produce a rejection for a previously-stashed message. 2. drainPendingVerifications reported only a SecurityContextUpdate on rejection but the store binding only patched securityContext, leaving the optimistically-delivered plaintext body in place. Extend the SecurityContextUpdate event with an optional body field so the drain can expunge the plaintext when rejecting. 3. Update SequoiaPgpPlugin test that expected 0 security updates and trust='untrusted' for signature-mismatch cases — these now correctly throw (Case A) or emit rejected (Case D).
Mirror the SequoiaPgpPlugin signature/trust test coverage on the openpgp.js backend: round-trip with verified signature, untrusted when sender key not cached, Case A rejection (signature mismatch with cached cert), timestamp skew rejection, pending buffer drain with upgrade, drain with rejection (Case D), and no-stash-without- messageId.
…t size axes
Address the top 3 React Doctor findings across the app:
- design-no-redundant-size-axes (578 → 0): collapse bare `w-N h-N` to
`size-N` across ~100 components (responsive variants, compound utilities
like max-w-, and mismatched/fractional axes left untouched).
- click-events-have-key-events (37 → 5) and no-static-element-interactions
(41 → 8):
- Modal backdrops: replace the static div click-to-dismiss with a native
`<button>` overlay (aria-hidden + tabIndex=-1; mouse-only convenience,
keyboard users already have Escape and in-dialog buttons). Panel raised
with relative z-10.
- Clickable rows/avatars/triggers: add role="button", tabIndex and
Enter/Space handlers; the sidebar profile name becomes a real <button>.
- Decorative Christmas overlay: aria-hidden.
- Resize handles: role="separator" + aria-hidden.
Remaining occurrences are intentionally deferred (documented as design
decisions): container-keyboard-nav list items (useListKeyboardNav),
focusable scroll regions (would relocate to no-noninteractive-*), and a
clickable wrapper around an interactive MessageBubble.
Update affected snapshots and modal backdrop-dismiss tests. No regressions
in sibling a11y rules (no-noninteractive-element-interactions 1 → 0).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the three highest-volume React Doctor findings across the app, verified against
npx react-doctor@latest.Results
design-no-redundant-size-axesclick-events-have-key-eventsno-static-element-interactionsTypecheck, lint, and the full test suite (2821 tests) all pass. No regressions in sibling a11y rules —
no-noninteractive-element-interactionsactually drops from 1 to 0.Changes
Redundant size axes (578 → 0) — collapse bare
w-N h-Ntosize-Nacross ~100 components via a codemod that deliberately skips responsive variants (md:w-4), compound utilities (max-w-), and mismatched/fractional axes (w-32 h-1,w-1/2).Click/static-element accessibility:
<div>click-to-dismiss becomes a native<button>overlay (aria-hidden+tabIndex={-1}: a mouse-only convenience, since every dialog already closes via Escape and an in-dialog button). Panels raised withrelative z-10. This also removes the now-unneededmouseDownTargetRefdrag guard (a real<button>only fires when press and release land on it).role="button",tabIndex, and Enter/Space handlers so keyboard and screen-reader users can reach them; the sidebar profile name becomes a real<button>.aria-hidden(screen readers shouldn't announce particles; it auto-dismisses).role="separator"+aria-hidden.Intentionally deferred (13 occurrences)
These would regress UX or merely relocate the warning to a sibling rule, so they're left for a follow-up design decision:
no-noninteractive-tabindex.useListKeyboardNav(ContactList, ConversationList, RoomsList, BrowseRoomsModal) — keyboard activation lives on the container; per-item button roles would break the single-tab-stop roving model. Correct fix is alistbox/optionrefactor of the shared hook.MessageBubble(would create nested interactive controls).Snapshots and modal backdrop-dismiss tests updated to match the new structure.