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: avoid memory issues by doing better cleanup #4059

Merged
merged 5 commits into from
Jun 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion build/tasks/esbuild.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ module.exports = function (grunt) {
bundle: true
})
.then(done)
.catch(done);
.catch(e => {
grunt.fail.fatal(e);
done();
});
});
});
}
Expand Down
2 changes: 1 addition & 1 deletion lib/checks/navigation/region-evaluate.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import * as standards from '../../commons/standards';
import matches from '../../commons/matches';
import cache from '../../core/base/cache';

const landmarkRoles = standards.getAriaRolesByType('landmark');
const implicitAriaLiveRoles = ['alert', 'log', 'status'];

export default function regionEvaluate(node, options, virtualNode) {
Expand Down Expand Up @@ -92,6 +91,7 @@ function isRegion(virtualNode, options) {
const node = virtualNode.actualNode;
const role = getRole(virtualNode);
const ariaLive = (node.getAttribute('aria-live') || '').toLowerCase().trim();
const landmarkRoles = standards.getAriaRolesByType('landmark');

// Ignore content inside of aria-live
if (
Expand Down
11 changes: 8 additions & 3 deletions lib/commons/color/incomplete-data.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import cache from '../../core/base/cache';

const cacheKey = 'color.incompleteData';

/**
* API for handling incomplete color data
* @namespace axe.commons.color.incompleteData
* @inner
*/
let data = {};
const incompleteData = {
/**
* Store incomplete data by key with a string value
Expand All @@ -17,6 +20,7 @@ const incompleteData = {
if (typeof key !== 'string') {
throw new Error('Incomplete data: key must be a string');
}
const data = cache.get(cacheKey, () => ({}));
if (reason) {
data[key] = reason;
}
Expand All @@ -31,7 +35,8 @@ const incompleteData = {
* @return {String} String for reason we couldn't tell
*/
get: function (key) {
return data[key];
const data = cache.get(cacheKey);
return data?.[key];
},
/**
* Clear incomplete data on demand
Expand All @@ -40,7 +45,7 @@ const incompleteData = {
* @instance
*/
clear: function () {
data = {};
cache.set(cacheKey, {});
}
};

Expand Down
20 changes: 13 additions & 7 deletions lib/commons/standards/implicit-html-roles.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,19 @@ import isRowHeader from '../table/is-row-header';
import sanitize from '../text/sanitize';
import isFocusable from '../dom/is-focusable';
import { closest } from '../../core/utils';
import cache from '../../core/base/cache';
import getExplicitRole from '../aria/get-explicit-role';

const sectioningElementSelector =
getElementsByContentType('sectioning')
.map(nodeName => `${nodeName}:not([role])`)
.join(', ') +
' , main:not([role]), [role=article], [role=complementary], [role=main], [role=navigation], [role=region]';
const getSectioningElementSelector = () => {
return cache.get('sectioningElementSelector', () => {
return (
getElementsByContentType('sectioning')
.map(nodeName => `${nodeName}:not([role])`)
.join(', ') +
' , main:not([role]), [role=article], [role=complementary], [role=main], [role=navigation], [role=region]'
);
});
};

// sectioning elements only have an accessible name if the
// aria-label, aria-labelledby, or title attribute has valid
Expand Down Expand Up @@ -64,7 +70,7 @@ const implicitHtmlRoles = {
fieldset: 'group',
figure: 'figure',
footer: vNode => {
const sectioningElement = closest(vNode, sectioningElementSelector);
const sectioningElement = closest(vNode, getSectioningElementSelector());

return !sectioningElement ? 'contentinfo' : null;
},
Expand All @@ -78,7 +84,7 @@ const implicitHtmlRoles = {
h5: 'heading',
h6: 'heading',
header: vNode => {
const sectioningElement = closest(vNode, sectioningElementSelector);
const sectioningElement = closest(vNode, getSectioningElementSelector());

return !sectioningElement ? 'banner' : null;
},
Expand Down
7 changes: 1 addition & 6 deletions lib/core/base/virtual-node/virtual-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { isXHTML, validInputTypes } from '../../utils';
import { isFocusable, getTabbableElements } from '../../../commons/dom';
import cache from '../cache';

let isXHTMLGlobal;
let nodeIndex = 0;

class VirtualNode extends AbstractVirtualNode {
Expand All @@ -27,11 +26,7 @@ class VirtualNode extends AbstractVirtualNode {

this._isHidden = null; // will be populated by axe.utils.isHidden
this._cache = {};

if (typeof isXHTMLGlobal === 'undefined') {
isXHTMLGlobal = isXHTML(node.ownerDocument);
}
this._isXHTML = isXHTMLGlobal;
this._isXHTML = isXHTML(node.ownerDocument);

// we will normalize the type prop for inputs by looking strictly
// at the attribute and not what the browser resolves the type
Expand Down
1 change: 1 addition & 0 deletions lib/core/utils/frame-messenger/message-id.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { v4 as createUuid } from '../uuid';

// No cache, so that this can persist across axe.run calls
const messageIds = [];

export function createMessageId() {
Expand Down
7 changes: 3 additions & 4 deletions lib/core/utils/frame-messenger/post-message.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ import { createMessageId } from './message-id';
* @return {Boolean} true if the message was sent
*/
export function postMessage(win, data, sendToParent, replyHandler) {
if (typeof replyHandler === 'function') {
storeReplyHandler(data.channelId, replyHandler, sendToParent);
}

// Prevent messaging to an inappropriate window
sendToParent ? assertIsParentWindow(win) : assertIsFrameWindow(win);
if (data.message instanceof Error && !sendToParent) {
Expand All @@ -37,6 +33,9 @@ export function postMessage(win, data, sendToParent, replyHandler) {
return false;
}

if (typeof replyHandler === 'function') {
storeReplyHandler(data.channelId, replyHandler, sendToParent);
}
// There is no way to know the origin of `win`, so we'll try them all.
allowedOrigins.forEach(origin => {
try {
Expand Down
5 changes: 1 addition & 4 deletions lib/core/utils/get-selector.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import matchesSelector from './element-matches';
import isXHTML from './is-xhtml';
import getShadowSelector from './get-shadow-selector';

let xhtml;
const ignoredAttributes = [
'class',
'style',
Expand Down Expand Up @@ -238,9 +237,7 @@ function getElmId(elm) {
* @return {String|Array<String>} Base CSS selector for the node
*/
function getBaseSelector(elm) {
if (typeof xhtml === 'undefined') {
xhtml = isXHTML(document);
}
const xhtml = isXHTML(document);
return escapeSelector(xhtml ? elm.localName : elm.nodeName.toLowerCase());
}

Expand Down
8 changes: 5 additions & 3 deletions lib/core/utils/is-xhtml.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
import memoize from './memoize';

/**
* Determines if a document node is XHTML
* @method isXHTML
* @memberof axe.utils
* @param {Node} doc a document node
* @return {Boolean}
*/
function isXHTML(doc) {
if (!doc.createElement) {
const isXHTML = memoize(doc => {
if (!doc?.createElement) {
return false;
}
return doc.createElement('A').localName === 'A';
}
});

export default isXHTML;
24 changes: 14 additions & 10 deletions lib/core/utils/query-selector-all-filter.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import cache from '../base/cache';
import { matchesExpression, convertSelector } from './matches';
import { getNodesMatchingExpression } from './selector-cache';

Expand All @@ -19,17 +20,20 @@ function createLocalVariables(
return retVal;
}

/**
* Allocating new objects in createLocalVariables is quite expensive given
* that matchExpressions is in the hot path.
*
* Keep track of previously allocated objects to avoid useless allocations
* and garbage collection. This is intentionally shared between calls of
* matchExpressions.
*/
const recycledLocalVariables = [];

function matchExpressions(domTree, expressions, filter) {
/**
* Allocating new objects in createLocalVariables is quite expensive given
* that matchExpressions is in the hot path.
*
* Keep track of previously allocated objects to avoid useless allocations
* and garbage collection. This is intentionally shared between calls of
* matchExpressions.
*/
const recycledLocalVariables = cache.get(
'qsa.recycledLocalVariables',
() => []
);

const stack = [];
const vNodes = Array.isArray(domTree) ? domTree : [domTree];
let currentLevel = createLocalVariables(
Expand Down
2 changes: 1 addition & 1 deletion lib/standards/html-elms.js
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ const htmlElms = {
'menuitem',
'menuitemcheckbox',
'menuitemradio',
'meter',
'meter',
'option',
'progressbar',
'radio',
Expand Down
18 changes: 18 additions & 0 deletions test/checks/navigation/region.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ describe('region', function () {
afterEach(function () {
fixture.innerHTML = '';
checkContext.reset();
axe.reset();
});

it('should return true when content is inside the region', function () {
Expand All @@ -28,6 +29,23 @@ describe('region', function () {
assert.isTrue(checkEvaluate.apply(checkContext, checkArgs));
});

it('should return true when a region role is added to standards', () => {
axe.configure({
standards: {
ariaRoles: {
feed: {
type: 'landmark'
}
}
}
});
var checkArgs = checkSetup(
'<div role="feed" id="target">This is random content.</div>' +
'<div role="main"><h1 id="mainheader">Introduction</h1></div>'
);
assert.isTrue(checkEvaluate.apply(checkContext, checkArgs));
});

it('should return false when img content is outside the region', function () {
var checkArgs = checkSetup(
'<img id="target" src="data:image/gif;base64,R0lGODlhEAAQAMQAAORHHOVSKudfOulrSOp3WOyDZu6QdvCchPGolfO0o/XBs/fNwfjZ0frl3/zy7////wAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACH5BAkAABAALAAAAAAQABAAAAVVICSOZGlCQAosJ6mu7fiyZeKqNKToQGDsM8hBADgUXoGAiqhSvp5QAnQKGIgUhwFUYLCVDFCrKUE1lBavAViFIDlTImbKC5Gm2hB0SlBCBMQiB0UjIQA7"><div role="main"><h1 id="mainheader" tabindex="0">Introduction</h1></div>'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,15 @@ <h1 id="pass-h1-valid-role" role="none"></h1>
<div id="fail-dpub-4" role="doc-noteref">ok</div>
<!-- images -->
<div id="fail-dpub-5" role="doc-cover">ok</div>
<img src="#" role="meter" aria-valuenow="0" aria-valuemin="0" aria-valuemax="100" alt="test" id="pass-img-valid-role-meter">
<img
src="#"
role="meter"
aria-valuenow="0"
aria-valuemin="0"
aria-valuemax="100"
alt="test"
id="pass-img-valid-role-meter"
/>
<img role="doc-cover" aria-label="foo" id="pass-img-valid-role-aria-label" />
<img role="menuitem" title="bar" id="pass-img-valid-role-title" />
<div id="image-baz">hazaar</div>
Expand Down