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.

test/integration/full/target-size/fixed-scroll.js

test/commons/dom/is-offscreen.js

lib/commons/dom/is-offscreen.js

lib/commons/dom/is-fixed-position.js

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

lib/commons/dom/is-fixed-position.js

+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

test/integration/full/target-size/fixed-scroll.js

+          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

lib/commons/dom/is-offscreen.js

-    //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

lib/commons/dom/is-offscreen.js

+    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

lib/commons/dom/is-offscreen.js

  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 <WilcoFiers@users.noreply.github.com> Co-authored-by: Wilco Fiers <wilco.fiers@deque.com>

### 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)

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

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