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.

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

feat: update WPT usage #1032

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jlp-craigmorten
Copy link
Contributor

@jlp-craigmorten jlp-craigmorten commented Mar 23, 2024

Resolves #1045

Updates the WPT submodule and tests.

jsdom

For the jsdom suite, i've included a (non-exhaustive) set of specs that are relevant to this package in their scope regarding accessible name, description, and role calculation:

  • accname
  • dpub-aam
  • graphics-aria
  • html-aam
  • wai-aria

The current status of the WPT coverage is:

Passing Failing Skipped
177 127 297

Where "Skipped" includes a combination of both relevant and irrelevant tests, but for a practical test runtime it is pragmatic to skip instead of having an expected failure.

I have opted to equate a null return from this package (e.g. from getRole()) to be the equivalent of an empty role "" which eliminates a number of failures (for expected generic or presentational roles). If this normalization is misrepresentative I can remove.

browser

For the Cypress based suite I've made the necessary changes so that the tests run, but have not extended the suite to cover the additional accname (or other spec) tests that have been introduced since the last refresh for this setup.

Copy link

changeset-bot bot commented Mar 23, 2024

⚠️ No Changeset found

Latest commit: eb37052

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

tests/wpt-jsdom/utils.js Fixed Show fixed Hide fixed
// We need rejectUnauthorized support so we can't use built-in fetch(), sadly.
exports.doHeadRequestWithNoCertChecking = (url) => {
const agent = url.startsWith("https")
? new https.Agent({ rejectUnauthorized: false })

Check failure

Code scanning / CodeQL

Disabling certificate validation High test

Disabling certificate validation is strongly discouraged.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is taken directly from the implementation in https://github.com/jsdom/jsdom/blob/2f8a730/test/. Let me know if feel strong that the tests need to prevent MITM attacks.

Copy link
Contributor Author

@jlp-craigmorten jlp-craigmorten Mar 24, 2024

Choose a reason for hiding this comment

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

Looks like an equivalent is already in use with strictSSL disabled so is an existing vulnerability.

See https://github.com/eps1lon/dom-accessibility-api/blob/main/tests/wpt-jsdom/start-wpt-server.js#L85

.gitmodules Outdated Show resolved Hide resolved
@jlp-craigmorten jlp-craigmorten force-pushed the feat-update-wpt-tests branch 2 times, most recently from 4c82554 to d06ebe2 Compare March 31, 2024 14:09
Co-authored-by: Craig Morten <[email protected]>
Comment on lines -3 to -4
description_1.0_combobox-focusable-manual.html:
[fail, title already used for name]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: I think this might actually be passing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update WPT suite
2 participants