fix(target-size): ignore position: fixed elements that are offscreen when page is scrolled by straker · Pull Request #5066 · dequelabs/axe-core

straker · 2026-04-09T22:40:14Z

…when page is scrolled

Copilot

Pull request overview

This PR fixes a target-size false-positive when the page is scrolled: position: fixed elements that are outside the viewport (e.g., above the top edge) should remain ignored rather than being treated as on-screen due to scroll offsets.

Changes:

Update offscreen detection to treat fixed-position subtrees differently (use viewport-relative coordinates).
Introduce a new dom.isFixedPosition helper and export it from axe.commons.dom.
Add/adjust unit + integration tests to cover fixed-position + scroll scenarios.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
test/integration/full/target-size/fixed-scroll.html	New integration fixture page with fixed elements and large scroll height.
test/integration/full/target-size/fixed-scroll.js	New integration test that scrolls and asserts `target-size` yields no violations.
test/commons/dom/is-visible.js	Updates tests to set up the flat tree before calling `dom.isVisible`.
test/commons/dom/is-offscreen.js	Refactors tests and adds fixed-position scroll coverage for `dom.isOffscreen`.
test/commons/dom/is-fixed-position.js	Adds unit tests for the new `dom.isFixedPosition` helper.
lib/commons/dom/is-offscreen.js	Adjusts `isOffscreen` to use `getBoundingClientRect()` for fixed-position subtrees and adds a fixed-bottom check.
lib/commons/dom/is-fixed-position.js	Adds the new helper to detect whether a node is in a `position: fixed` subtree.
lib/commons/dom/index.js	Exports `isFixedPosition` from the dom namespace.

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

Copilot · 2026-04-10T08:48:35Z

+export default function isFixedPosition(node, { skipAncestors } = {}) {
+  const { vNode } = nodeLookup(node);
+
+  // detached element
+  if (!vNode) {
+    return false;
+  }


isFixedPosition returns false when nodeLookup can’t find a vNode. For an Element input this can happen simply because axe.setup()/flatTreeSetup() hasn’t been run yet (even though the element is connected and actually position: fixed). Since this is exported as VirtualNode|Element, consider adding a DOM-only fallback (e.g., check getComputedStyle(node).position and traverse composed parents) or documenting/enforcing the requirement that a virtual tree must exist.

Not necessary. We can assume setup was run in commons.

Garbee · 2026-04-10T12:01:30Z

+          return done(err);
+        }
+        results = r;
+        console.log(results);


Should we leave this log in? It is only a test, but still adds cruft to the output data.

Definitely not. 🤨 I saw that and didn't even question it.

Looks like Steve copied it. We have it in a couple more places. I'll just clean those up as well.

Garbee · 2026-04-10T12:02:44Z

-    //This is an edge case, an empty (zero-width) element that isn't positioned 'off screen'.
-    return false;
+  if (isFixed && coords.top >= window.innerHeight) {
+    return true; // Fixed above the viewport


Is coords.top >= window.innerHeight not determining if the element is below the Viewport? Is the comment wrong, the code, or my understanding of it?

Yeah, comment is wrong.

Garbee · 2026-04-10T12:12:26Z

+    return true; // Fixed above the viewport
+  }
+
+  if (isFixed && coords.left >= window.innerWidth) {


Was RTL mode considered here? This feels like in that context things might slip through.

I had, twice. But thinking about it a third time there is a little edge case here we missed. Cheers!

chutchins25

One question for you. Everything else was already fixed or found by Garbee.

chutchins25 · 2026-04-10T13:15:02Z

  if (
-    coords.bottom < 0 &&
+    coords.bottom <= 0 &&
    (noParentScrolled(domNode, coords.bottom) || styl.position === 'absolute')


Do we need to bypass this for fixed elements?

What situation would propose that we should?

Yeah this code is buggy. It's not a new problem. I looked at fixing it while I was in here but it's too complex. I don't want to rush into it so I opened an issue for it instead #5069

chutchins25

Looks good to me.

…when page is scrolled (#5066) closes #5065 --------- Co-authored-by: Wilco Fiers <[email protected]> Co-authored-by: Wilco Fiers <[email protected]>

### Bug Fixes - **aria-allowed-attr:** restrict br and wbr elements to aria-hidden only ([#4974](#4974)) ([1d80163](1d80163)) - **target-size:** ignore position: fixed elements that are offscreen when page is scrolled ([#5066](#5066)) ([5906273](5906273)), closes [#5065](#5065)

## [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 🤖 🎉

fix(target-size): ignore position: fixed elements that are offscreen …

c990f12

…when page is scrolled

straker requested a review from a team as a code owner April 9, 2026 22:40

fix

d9533f1

WilcoFiers requested a review from Copilot April 10, 2026 08:42

Merge branch 'develop' into target-size-fixed

2685586

Copilot started reviewing on behalf of WilcoFiers April 10, 2026 08:43 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

WilcoFiers added 3 commits April 10, 2026 11:37

tweak

a975fc1

Couple more fixes

e469362

more tweaks

29135e3

Garbee reviewed Apr 10, 2026

View reviewed changes

chutchins25 reviewed Apr 10, 2026

View reviewed changes

More cleanup

f87b6b9

chutchins25 self-requested a review April 10, 2026 14:42

chutchins25 approved these changes Apr 10, 2026

View reviewed changes

WilcoFiers approved these changes Apr 10, 2026

View reviewed changes

WilcoFiers merged commit 1229a6e into develop Apr 10, 2026
23 checks passed

WilcoFiers deleted the target-size-fixed branch April 10, 2026 14:46

straker mentioned this pull request Apr 13, 2026

chore(release): v4.11.3 #5070

Merged

attest-team-ci mentioned this pull request Jun 1, 2026

chore(release): v4.12.0 #5122

Merged

WestonThayer mentioned this pull request Jun 18, 2026

color-contrast, target-size, more only reported for nodes within the current viewport as of 4.11.3 #5183

Open

1 task

Uh oh!

Conversation

straker commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chutchins25 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chutchins25 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants