Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add checks for clickable elements missing from Accessibility Tree #408

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

feat: add checks for clickable elements missing from Accessibility Tree #408

wants to merge 25 commits into from

Conversation

shioju
Copy link
Collaborator

@shioju shioju commented Oct 11, 2024 •
edited
Loading

This PR adds...

  • I've kept this PR as small as possible (~500 lines) by splitting it into PRs with manageable chunks of code
  • I've requested reviews from 1 reviewer
  • I've tested existing features (website scan, sitemap, custom flow) in both node index and cli
  • I've synced this fork with GovTechSG repo
  • I've added/updated unit tests
  • I've added/updated any necessary dependencies in package[-lock].json npm audit, portable installation on GitHub Actions

@shioju shioju marked this pull request as ready for review October 20, 2024 00:42
@shioju shioju requested a review from younglim October 20, 2024 00:42
@shioju shioju force-pushed the feat/check-non-aria-clickable branch from f58a9d5 to 272239b Compare October 26, 2024 02:02
@shioju shioju force-pushed the feat/check-non-aria-clickable branch from 4561757 to 6b6264a Compare October 31, 2024 21:32
@younglim
Copy link
Collaborator

younglim commented Nov 9, 2024

Fails to scan CNA with no error log:

npm run cli -- -c 2 -u https://www.channelnewsasia.com/ -p 2 -k "A11y Team:[email protected]" -h no

younglim
younglim reviewed Nov 9, 2024
View reviewed changes
Copy link
Collaborator

@younglim younglim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments above

@shioju
feat: directly add oobee-accessible-label violations to results
29cd109
@shioju
chore: group oobee-accessible-label findings as single violation
6e36700 
- filter for "block-mc*" for now, as some others are causing failures
@shioju
fix: skip screenshots but still include violations above maxScreenshots
6937595
@shioju
fix: escape css id selectors that start with a digit
910a9dd 
- add more error handling when running in headless browser
@shioju
chore: remove unused custom axe config
14e8879
@shioju
chore: define custom result interface with screenshot
7cbacad
@shioju
chore: add type annotations
08747d3
@shioju
feat: add cli flag to customise ruleset
341c7f6
@shioju
feat: pass rulset into crawlDomain
e086586 
- change crawlDomain to accept named params
@shioju
feat: pass ruleset into runAxeScript
153ea8d 
- change runAxeScript to take in named params
@shioju
feat: disable oobee rules if --rulset without-oobee flag is set
9bf72af
@shioju
fix: npm audit fix for cross-spawn
335879f 
- https://github.com/GovTechSG/oobee/security/dependabot/37
@shioju shioju mentioned this pull request Nov 19, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@younglim younglim younglim left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@shioju @younglim