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

fix(target-size): ignore descendant elements in shadow dom #4410

Merged
merged 3 commits into from
Apr 23, 2024
Merged

Conversation

straker
Copy link
Contributor

@straker straker commented Apr 15, 2024

This also adds to the .eslintrc to error if node.contains() or vNode.actualNode.contains() or used. (also upgraded the node.attributes error to account for vNode.actualNode.attributes as I noticed it was missing).

Closes: #4194

@straker straker requested a review from a team as a code owner April 15, 2024 21:59
@@ -2,6 +2,7 @@ import findNearbyElms from './find-nearby-elms';
import isInTabOrder from './is-in-tab-order';
import { splitRects, hasVisualOverlap } from '../math';
import memoize from '../../core/utils/memoize';
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 a separate import on purpose as memoize when destructred from the utils import path causes problems for esbuild and it doesn't hoist the memoize function high enough in the build for functions to use it. We've gotten around that by importing it directly.

test/commons/dom/get-target-rects.js Outdated Show resolved Hide resolved
test/integration/full/target-size/shadow-dom.html Outdated Show resolved Hide resolved
test/integration/full/target-size/shadow-dom.js Outdated Show resolved Hide resolved
lib/checks/mobile/target-size-evaluate.js Show resolved Hide resolved
straker and others added 2 commits April 17, 2024 08:07
Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com>
@@ -411,4 +410,19 @@ describe('target-size tests', function () {
'#obscurer2'
]);
});

it('ignores descendants of the target that are in shadow dom', () => {
fixture.innerHTML =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

shadowCheckSetup assumes that the target is either the shadow root node or inside the shadow content. This isn't true for the DOM I was testing and I didn't want to try to update the function in this PR to account for content which uses both.

@straker straker merged commit 6091367 into develop Apr 23, 2024
21 checks passed
@straker straker deleted the contains branch April 23, 2024 14:11
WilcoFiers added a commit that referenced this pull request May 6, 2024
###
[4.9.1](v4.9.0...v4.9.1)
(2024-05-06)

### Bug Fixes

- Prevent errors when loading axe in a page with prototype.js
- **aria-allowed-attr:** allow meter role allowed aria-\* attributes on
meter element
([#4435](#4435))
([7ac6392](7ac6392))
- **aria-allowed-role:** add gridcell, separator, slider and treeitem to
allowed roles of button element
([#4398](#4398))
([4788bf8](4788bf8))
- **aria-roles:** correct abstract roles (types) for
aria-roles([#4421](#4421))
- **aria-valid-attr-value:** aria-controls & aria-haspopup incomplete
([#4418](#4418))
- fix building axe-core translation files with region locales
([#4396](#4396))
([5c318f3](5c318f3)),
closes [#4388](#4388)
- **invalidrole:** allow upper and mixed case role names
([#4358](#4358))
([105016c](105016c)),
closes [#2695](#2695)
- **isVisibleOnScreen:** account for position: absolute elements inside
overflow container
([#4405](#4405))
([2940f6e](2940f6e)),
closes [#4016](#4016)
- **label-content-name-mismatch:** better dismiss and wysiwyg symbolic
text characters
([#4402](#4402))
- **region:** Decorative images ignored by region rule
([#4412](#4412))
- **target-size:** ignore descendant elements in shadow dom
([#4410](#4410))
([6091367](6091367))
- **target-size:** pass for element that has nearby elements that are
obscured ([#4422](#4422))
([3a90bb7](3a90bb7)),
closes [#4387](#4387)


This PR was opened by a robot 🤖 🎉 (And updated by @WilcoFiers
)
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.

target-offset check: targets containing shadow children cause false positives
2 participants