Skip to content

fix(utils): Add null check to parseCrossOriginStylesheet, closes #5074 - #5075

Merged
straker merged 5 commits into
dequelabs:developfrom
dcbroad3:develop
May 27, 2026
Merged

fix(utils): Add null check to parseCrossOriginStylesheet, closes #5074#5075
straker merged 5 commits into
dequelabs:developfrom
dcbroad3:develop

Conversation

@dcbroad3

Copy link
Copy Markdown
Contributor

If a null href is passed to parseCrossOriginStylesheet, the function will currently convert that to the string "null" and attempt to load that href. This PR adds a null check so that the function instead returns early if a nullish href is passed, similar to how parseSameOriginStylesheet returns early if a sheet includes no rules.

Closes:
#5074

@dcbroad3
dcbroad3 requested a review from a team as a code owner April 14, 2026 17:29
Copilot AI review requested due to automatic review settings April 14, 2026 17:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses axe-core issue #5074 by preventing parseCrossOriginStylesheet from attempting to fetch a stylesheet when the provided href/URL is nullish, avoiding erroneous network requests like "null".

Changes:

  • Add a nullish check in parseCrossOriginStylesheet to return early when url == null.
  • Add a unit test intended to cover the nullish-URL case.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
lib/core/utils/parse-crossorigin-stylesheet.js Adds early return when the cross-origin stylesheet URL is nullish.
test/core/utils/parse-crossorigin-stylesheet.js Adds coverage for the nullish-URL behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

isCrossOrigin
) {
if (url == null) {
return Promise.resolve();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am following the pattern of the early return in parseSameOriginStylesheet if rules is falsy, but let me know if a different result should be returned (or if the function should do an early reject instead?)

Comment thread test/core/utils/parse-crossorigin-stylesheet.js Outdated
Comment thread test/core/utils/parse-crossorigin-stylesheet.js Outdated
importedUrls,
isCrossOrigin
) {
if (url == null) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually thinking about this I wonder if we should reject the promise here instead of returning. If the cross origin stylesheet somehow has no href but has styles that are applying to the page, we should reject to signify we don't have the full styles. Let me ask my team about what they think.

straker
straker previously approved these changes May 6, 2026

@straker straker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We determined that this will probably only happen with extensions adding styles to the page, so we are fine resolving the promise since we want to test the page itself not the styles added by the extension.

importedUrls,
isCrossOrigin
) {
if (url == null) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Linting is failing as this should use === instead of ==

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You guys don't allow nullish checks? That makes me sad. Will update.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@dcbroad3 Any updates here? Were going to feature freeze for axe-core 4.12 today. If you can get this addressed we can get it in.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, apologies, this slipped my mind. I'll try to get this addressed this morning.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@straker @WilcoFiers Updated.. I hope this is what you meant. (x == null is functionally equivalent to x === null || x === undefined)

@straker straker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the pr!

Reviewed for security.

@straker
straker merged commit f12ef32 into dequelabs:develop May 27, 2026
23 of 24 checks passed
WilcoFiers added a commit that referenced this pull request Jun 1, 2026
##
[4.12.0](v4.11.4...v4.12.0)
(2026-06-01)

### Features

- add gather-internals.js external script
([#5099](#5099))
([c61d58b](c61d58b)),
closes [#5080](#5080)
- **aria-allowed/prohibited-attr, aria-required-parent/children:**
partially support element internals role
([#5080](#5080))
([417b48a](417b48a)),
closes [#5039](#5039)
[#4259](#4259)
- **axe.externalAPIs:** add public api for setting elementInternal data
([#5105](#5105))
([63bab8f](63bab8f))
- **core:** expose normalizeRunOptions
([#4998](#4998))
([b8e6a59](b8e6a59))
- expose axe.resetLocale() to restore the default locale
([#5108](#5108))
([c2b5292](c2b5292)),
closes [#5107](#5107)
- **getRules:** include rule enabled state in returned objects
([#5118](#5118))
([75bf772](75bf772)),
closes [#5116](#5116)
- **list,listitem:** support element internals role
([#5119](#5119))
([7d9d696](7d9d696))
- **new-rule:** check that aria-tab have an accessible name
([#5001](#5001))
([0d4e4e7](0d4e4e7)),
closes [#4842](#4842)
- **rules:** deprecate landmark-complementary-is-top-level rules
([#4992](#4992))
([9e09139](9e09139)),
closes [#4950](#4950)
- **utils:** add `getElementInternals` function
([#5077](#5077))
([1c15f82](1c15f82))

### Bug Fixes

- **aria-allowed-attr:** restrict br and wbr elements to aria-hidden
only ([#4974](#4974))
([c6245e7](c6245e7))
- **aria-conditional-attr:** add support for radio
([#5100](#5100))
([8223c98](8223c98))
- **aria-valid-attr-value:** handle multiple aria-errormessage IDs
([#4973](#4973))
([0489e30](0489e30))
- **aria:** prevent getOwnedVirtual from returning duplicate nodes
([#4987](#4987))
([48ca955](48ca955)),
closes [#4840](#4840)
- **commons/text:** exclude natively hidden elements from
aria-labelledby accessible name
([#5076](#5076))
([ea7202c](ea7202c)),
closes [#4704](#4704)
- **DqElement:** avoid calling constructors with cloneNode
([#5013](#5013))
([0281fa1](0281fa1))
- **existing-rule:** aria-busy now shows an error message for a use with
unallowed children
([#5017](#5017))
([2067b87](2067b87))
- **helpUrl:** ensure axe.configure always updates the help URLs
([#5114](#5114))
([c4f60ff](c4f60ff))
- **label-content-name-mismatch:** match visible text with aria-label
and exclude invisible text
([#5096](#5096))
([3a012a1](3a012a1))
- **locale:** ensure all subtags are correctly set
([#5112](#5112))
([13005ed](13005ed))
- **scrollable-region-focusable:** clarify the issue is in safari
([#4995](#4995))
([4ec5211](4ec5211)),
closes
[WebKit#190870](https://github.com/dequelabs/WebKit/issues/190870)
[WebKit#277290](https://github.com/dequelabs/WebKit/issues/277290)
- **scrollable-region-focusable:** do not fail scroll areas when all
content is visible without scrolling
([#4993](#4993))
([838707a](838707a))
- **target-size:** determine offset using clientRects if target is
display:inline
([#5012](#5012))
([a4b8091](a4b8091))
- **target-size:** ignore position: fixed elements that are offscreen
when page is scrolled
([#5066](#5066))
([1229a6e](1229a6e)),
closes [#5065](#5065)
- **target-size:** ignore widgets that are inline with other inline
elements ([#5000](#5000))
([a8dd81b](a8dd81b))
- **utils/getAncestry:** escape node name
([#5079](#5079))
([d1fabaa](d1fabaa)),
closes [#5078](#5078)
- **utils:** Add null check to parseCrossOriginStylesheet, closes
[#5074](#5074)
([#5075](#5075))
([f12ef32](f12ef32))
- **utils:** update isShadowRoot to use spec-compliant custom element
regex ([#5059](#5059))
([edc6ce2](edc6ce2)),
closes [#5030](#5030)

This PR was opened by a robot 🤖 🎉
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.

4 participants