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.

Sign up for GitHub

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

Jump to bottom

feat(checks/label-content-name-mismatch): deprecate occuranceThreshold option in favor of occurrenceThreshold to fix typo #3782

Merged
merged 7 commits into from
Nov 30, 2022
Merged

feat(checks/label-content-name-mismatch): deprecate occuranceThreshold option in favor of occurrenceThreshold to fix typo #3782

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f5fd5dd
Correct typos of "occurrence" across axe-core.
nolandanley Nov 15, 2022
33f2d99
Correct Markdown table spacing.
nolandanley Nov 15, 2022
22d9b95
Add fix to accomodate previous typos to avoid breaking the API per Wi…
nolandanley Nov 16, 2022
aa7e466
Account for old spelling in subtreeText and minor formatting.
nolandanley Nov 25, 2022
f1c97f4
run tests
straker Nov 29, 2022
d2f9dec
Add null-aware chaining where necessary.
nolandanley Nov 30, 2022
cfcf1b5
run tests
straker Nov 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions doc/check-options.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,10 +341,10 @@ th</code></pre>

### label-content-name-mismatch

| Option | Default | Description |
| -------------------- | :------ | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `pixelThreshold` | `0.1` | Percent of difference in pixel data or pixel width required to determine if a font is a ligature font. Ligature fonts are ignored when comparing the label to the content |
| `occuranceThreshold` | `3` | Number of times the font is encountered before auto-assigning the font as a ligature or not |
| Option | Default | Description |
| --------------------- | :------ | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `pixelThreshold` | `0.1` | Percent of difference in pixel data or pixel width required to determine if a font is a ligature font. Ligature fonts are ignored when comparing the label to the content |
| `occurrenceThreshold` | `3` | Number of times the font is encountered before auto-assigning the font as a ligature or not |

### has-lang

Expand Down
4 changes: 2 additions & 2 deletions lib/checks/label/label-content-name-mismatch-evaluate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function curateString(str) {
}

function labelContentNameMismatchEvaluate(node, options, virtualNode) {
const { pixelThreshold, occuranceThreshold } = options || {};
const { pixelThreshold, occurrenceThreshold } = options || {};
nolandanley marked this conversation as resolved.
Show resolved Hide resolved

const accText = accessibleText(node).toLowerCase();
if (isHumanInterpretable(accText) < 1) {
Expand All @@ -50,7 +50,7 @@ function labelContentNameMismatchEvaluate(node, options, virtualNode) {
subtreeDescendant: true,
ignoreIconLigature: true,
pixelThreshold,
occuranceThreshold
occurrenceThreshold
})
).toLowerCase();
if (!visibleText) {
Expand Down
2 changes: 1 addition & 1 deletion lib/checks/label/label-content-name-mismatch.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"evaluate": "label-content-name-mismatch-evaluate",
"options": {
"pixelThreshold": 0.1,
"occuranceThreshold": 3
"occurrenceThreshold": 3
nolandanley marked this conversation as resolved.
Show resolved Hide resolved
},
"metadata": {
"impact": "serious",
Expand Down
2 changes: 1 addition & 1 deletion lib/checks/mobile/css-orientation-lock-evaluate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ function cssOrientationLockEvaluate(node, options, virtualNode, context) {
}

/**
* get last match/occurence of a transformation function that can affect rotation along Z axis
* get last match/occurrence of a transformation function that can affect rotation along Z axis
*/
const matches = transformStyle.match(
/(rotate|rotateZ|rotate3d|matrix|matrix3d)\(([^)]+)\)(?!.*(rotate|rotateZ|rotate3d|matrix|matrix3d))/
Expand Down
4 changes: 2 additions & 2 deletions lib/commons/text/accessible-text-virtual.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,11 @@ function shouldIgnoreHidden(virtualNode, context) {
* @return {Boolean}
*/
function shouldIgnoreIconLigature(virtualNode, context) {
const { ignoreIconLigature, pixelThreshold, occuranceThreshold } = context;
const { ignoreIconLigature, pixelThreshold, occurrenceThreshold } = context;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should avoid a break in the API.

Suggested change
const { ignoreIconLigature, pixelThreshold, occurrenceThreshold } = context;
const { ignoreIconLigature, pixelThreshold } = context;
const occurrenceThreshold = context.occurrenceThreshold ?? context.occurenceThreshold

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for this! I think you mean "occuranceThreshold" since that was the only version of the typo used in code. "Occurence" was only used in a comment. Added this modified fix in 22d9b95a1d851fae3c850f0a5f125a4ba8bf0438.

if (virtualNode.props.nodeType !== 3 || !ignoreIconLigature) {
return false;
}
return isIconLigature(virtualNode, pixelThreshold, occuranceThreshold);
return isIconLigature(virtualNode, pixelThreshold, occurrenceThreshold);
}

/**
Expand Down
12 changes: 6 additions & 6 deletions lib/commons/text/is-icon-ligature.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ import cache from '../../core/base/cache';
* @memberof axe.commons.text
* @instance
* @param {VirtualNode} textVNode The virtual text node
* @param {Number} occuranceThreshold Number of times the font is encountered before auto-assigning the font as a ligature or not
* @param {Number} occurrenceThreshold Number of times the font is encountered before auto-assigning the font as a ligature or not
* @param {Number} differenceThreshold Percent of differences in pixel data or pixel width needed to determine if a font is a ligature font
* @return {Boolean}
*/
function isIconLigature(
textVNode,
differenceThreshold = 0.15,
occuranceThreshold = 3
occurrenceThreshold = 3
) {
/**
* Determine if the visible text is a ligature by comparing the
Expand Down Expand Up @@ -101,17 +101,17 @@ function isIconLigature(

if (!fonts[fontFamily]) {
fonts[fontFamily] = {
occurances: 0,
occurrences: 0,
numLigatures: 0
};
}
const font = fonts[fontFamily];

// improve the performance by only comparing the image data of a font
// a certain number of times
if (font.occurances >= occuranceThreshold) {
if (font.occurrences >= occurrenceThreshold) {
// if the font has always been a ligature assume it's a ligature font
if (font.numLigatures / font.occurances === 1) {
if (font.numLigatures / font.occurrences === 1) {
return true;
}
// inversely, if it's never been a ligature assume it's not a ligature font
Expand All @@ -124,7 +124,7 @@ function isIconLigature(
// other times. in these cases we would need to always check the text
// to know if it's an icon or not
}
font.occurances++;
font.occurrences++;

// 30px was chosen to account for common ligatures in normal fonts
// such as fi, ff, ffi. If a ligature would add a single column of
Expand Down
2 changes: 1 addition & 1 deletion test/commons/text/is-icon-ligature.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ describe('text.isIconLigature', function () {
);
});

describe('occuranceThreshold', function () {
describe('occurrenceThreshold', function () {
(fontApiSupport ? it : it.skip)(
'should change the number of times a font is seen before returning',
function () {
Expand Down