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

Update math.getOffset algorithm to better align with spec #4119

Closed
straker opened this issue Aug 7, 2023 · 1 comment · Fixed by #4125
Closed

Update math.getOffset algorithm to better align with spec #4119

straker opened this issue Aug 7, 2023 · 1 comment · Fixed by #4125
Labels
pr A pr has been created for the issue tech debt Technical debt related tasks
Milestone

Comments

@straker
Copy link
Contributor

straker commented Aug 7, 2023

As part of #4117 we determined that the math.getOffset calculation is not correct and needs to be updated. It didn't produce any false-positives so we decided to update the code in another pr in order to get 4.8 out the door.

The algorithm should be as follows:

For targets with target size >= 24x24px, we should make sure we skip the (potentially expensive) offset calculation since the spacing exception isn't relevant. Target size === largest unobscured rect from math.splitRects. E.g. the result of calling dom.getTargetSize.

Otherwise, for undersized targets:

  1. determine bounding rect of target and get the center (we'll call centerTarget). Bounding rect === the result of calling math.getBoundingRect for each rect in splitRects. (should be a new function)
  2. determine the target's "neighbors" using grid (any other target that might be within 24px of any part of target's bounding rect is a neighbor)
  3. for each neighbor:
    1. determine neighborSplitRects using math.splitRects(neighbor)
    2. for each neighborSplitRect:
      1. calculate distance from centerTarget to the nearest point in neighborSplitRect (may be 0 if centerTarget lies within neighborSplitRect)
    3. let neighborSpacing be the minimum neighborSplitRect distance
    4. if the neighbor is also undersized (target size < 24x24px):
      1. calculate centerNeighbor similarly to centerTarget using the neighbor's bounding box
      2. if the distance from centerTarget to centerNeighbor is < neighborSpacing, use that as the new neighborSpacing
  4. return the minimum neighborSpacing from among all neighbors
@straker straker added the tech debt Technical debt related tasks label Aug 7, 2023
@straker straker changed the title Target-offset check algorithm Update math.getOffset algorithm to better align with spec Aug 7, 2023
@straker straker added the pr A pr has been created for the issue label Aug 9, 2023
@straker straker added this to the Axe-core 4.8 milestone Aug 9, 2023
@padmavemulapati
Copy link

padmavemulapati commented Sep 22, 2023

Validated with the latest axe-core develop branch code base,
on running await axe.run({runOnly:'target-size'}); at the console for latest axe.js script

seeing the target-size failure when the element -

<button style=\"width: 10px; height: 10px; margin: 0; padding: 0; position: absolute; top: 0; left: 0\">&nbsp;</button>"

and also failure summary is more precised now:

failureSummary:
"Fix any of the following:\n  Target has insufficient size (10px by 10px, should be at least 24px by 24px)\n  Target has insufficient space to its closest neighbors. Safe clickable space has a diameter of 0px instead of at least 24px."

Image

Environment:

Label Value
Product axe-core
Version 4.8.2
OS-Details _MAC- Intel Core i7 - 11.6.8 _
BrowserDetails Chrome Version 117.0.5938.88 (Official Build) (64-bit)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr A pr has been created for the issue tech debt Technical debt related tasks
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants