🧪 POC: Per-rule exclusions config for JS scanner checks#1723
🧪 POC: Per-rule exclusions config for JS scanner checks#1723SteveJonesDev wants to merge 3 commits into
Conversation
… of generic JS checks Adds an `exclusions` key to the PHP rule config (AriaHiddenRule as the first consumer). These selectors are collected at enqueue time and passed through the existing scanOptions pipeline (PHP → wp_localize_script → iframe window) to the JS check, which applies them via node.matches() before any other logic runs. Removes the hardcoded wp-block-spacer class check from aria-hidden-valid-usage.js and replaces it with the new config-driven approach alongside the two new WP core block overlay patterns from issue #567. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR implements configurable rule exclusions for aria-hidden validation, enabling specific elements matching Gutenberg block selectors to bypass the check. The change flows from backend rule definition through frontend data wiring to the validation check itself. ChangesRule Exclusions for Aria-Hidden Validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to dynamically exclude specific elements from accessibility checks by passing rule-specific exclusion selectors from PHP to the frontend scanner. Feedback focuses on improving robustness: wrapping the node.matches call in a try...catch block to prevent crashes from malformed selectors, using the spread operator when assigning scanOptions to avoid overwriting existing properties, and adding defensive checks in PHP to ensure rule slugs and exclusions are properly defined and typed.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc27d9e6db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pageScanner/checks/aria-hidden-valid-usage.js`:
- Around line 24-30: The exclusions handling in the aria-hidden-valid-usage
check should defensively normalize options?.exclusions to an array and guard
node.matches(selector) against malformed selectors; update the exclusions
initialization in aria-hidden-valid-usage.js (the exclusions variable and the
for (const selector of exclusions) loop) to coerce non-array inputs into an
array (e.g., via Array.isArray or [].concat) and wrap the node.matches(selector)
call in a try/catch that skips selectors which throw so the scan continues
safely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 33bc4500-d888-4b82-bf04-47f82e62be01
📒 Files selected for processing (5)
admin/class-enqueue-admin.phpincludes/classes/Rules/Rule/AriaHiddenRule.phpsrc/editorApp/checkPage.jssrc/pageScanner/checks/aria-hidden-valid-usage.jssrc/pageScanner/config/rules.js
SteveJonesDev
changed the title
- aria-hidden-valid-usage.js: use Array.isArray() guard and wrap node.matches() in try/catch so malformed selectors can't crash the scan - checkPage.js: spread existing scanOptions rather than replacing the object wholesale, preserving any properties set by other code - class-enqueue-admin.php: add is_array() defensive check in get_rule_exclusions(), make the method public so it can be reused by the frontend enqueue class - class-enqueue-frontend.php: pass ruleExclusions to edacFrontendHighlighterApp so frontend highlighter scans receive the same exclusions as editor iframe scans - frontendHighlighterApp/index.js: populate window.scanOptions.ruleExclusions from the localized data before the scan runs, fixing a regression where removing the hardcoded wp-block-spacer check would have caused false positives on frontend rescans Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The ariaHiddenValid test registered the check without options, so the wp-block-spacer test case started failing once the hardcoded class check was removed. Update beforeAll to pass the WordPress exclusions via check options, mirroring exactly what config/rules.js does at runtime via window.scanOptions. Also adds test cases for the two new WP core overlay selectors introduced in this POC (cover block background dim and post featured image overlay). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Addressed all inline review feedback in follow-up commits:
Frontend scan regression ( Jest failure: The |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# List coderabbitai's review comments on PR `#1723`
gh pr view 1723 --repo equalizedigital/accessibility-checker --json reviews,comments
# List review thread comments to find coderabbitai's comments to resolve
gh api graphql -f query='
{
repository(owner: "equalizedigital", name: "accessibility-checker") {
pullRequest(number: 1723) {
reviewThreads(first: 50) {
nodes {
id
isResolved
comments(first: 5) {
nodes {
author { login }
body
id
}
}
}
}
}
}
}'Length of output: 27549 🏁 Script executed: #!/bin/bash
# Resolve coderabbitai's review thread PRRT_kwDOEu6fTc6FiBex
gh api graphql -f query='
mutation {
resolveReviewThread(input: { threadId: "PRRT_kwDOEu6fTc6FiBex" }) {
thread {
id
isResolved
}
}
}'Length of output: 368
|
Summary
Testing
|
1 participant
Summary
exclusionskey to PHP rule configs, allowing WordPress-specific CSS selectors to be declared at the rule level rather than hardcoded in generic JS check logicscanOptionspipeline (PHP →wp_localize_script→ iframewindow.scanOptions) so the JS check can apply them vianode.matches()wp-block-spacercheck fromaria-hidden-valid-usage.jsand replaces it with the new config-driven approach; adds the two new WP core block overlay patterns from issue Additional core block exclusions on aria-hidden #567 at the same timeMotivation
The JS scanner rules are used as a standalone package outside of WordPress. Hardcoding WordPress class names (like
wp-block-spacer) directly in the JS checks pollutes generic accessibility logic with framework-specific knowledge. This approach keeps the JS checks clean and moves all WordPress-specific exclusion knowledge into the PHP rule config where it belongs.How it works
PHP rule config (
AriaHiddenRule.php):Collected at enqueue time (
class-enqueue-admin.php) and passed asruleExclusionsinedac_editor_app.Forwarded to iframe (
checkPage.js) viawindow.scanOptions.ruleExclusions.Applied in JS check (
aria-hidden-valid-usage.js) usingnode.matches(selector)— no WordPress knowledge in the check itself.Test plan
wp-block-spacerelements still pass (now via config rather than hardcoded)Closes #567
Fixes PRO-475
🤖 Generated with Claude Code
Summary by CodeRabbit