From 1009801734e0c14ad856a5b3c2322da2c1ced091 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 1 Sep 2021 22:08:43 +0200 Subject: [PATCH] fix(cdk/a11y): allow for multiple browser-generated description containers Currently the `AriaDescriber` is set up to clear all description containers whenever it is instantiated, in order to avoid duplicates coming in from the server. The problem is that there are legitimate use cases where we could have multiple containers (e.g. multiple CDK instances in a micro frontend architecture). These changes rework the internal setup of the `AriaDescriber` so that it only clears containers from server and it doesn't touch containers coming from other describer instances. Fixes #23499. --- .../aria-describer/aria-describer.spec.ts | 27 +++- src/cdk/a11y/aria-describer/aria-describer.ts | 130 +++++++++++------- tools/public_api_guard/cdk/a11y.md | 9 +- 3 files changed, 107 insertions(+), 59 deletions(-) diff --git a/src/cdk/a11y/aria-describer/aria-describer.spec.ts b/src/cdk/a11y/aria-describer/aria-describer.spec.ts index 8ee8270d16fc..8234657e95c7 100644 --- a/src/cdk/a11y/aria-describer/aria-describer.spec.ts +++ b/src/cdk/a11y/aria-describer/aria-describer.spec.ts @@ -1,5 +1,5 @@ import {A11yModule, CDK_DESCRIBEDBY_HOST_ATTRIBUTE} from '../index'; -import {AriaDescriber, MESSAGES_CONTAINER_ID} from './aria-describer'; +import {AriaDescriber} from './aria-describer'; import {ComponentFixture, TestBed} from '@angular/core/testing'; import {Component, ElementRef, ViewChild, Provider} from '@angular/core'; @@ -204,16 +204,31 @@ describe('AriaDescriber', () => { expect(() => ariaDescriber.describe(node, 'This looks like an element')).not.toThrow(); }); - it('should clear any pre-existing containers', () => { + it('should clear any pre-existing containers coming in from the server', () => { createFixture(); const extraContainer = document.createElement('div'); - extraContainer.id = MESSAGES_CONTAINER_ID; + extraContainer.classList.add('cdk-describedby-message-container'); + extraContainer.setAttribute('platform', 'server'); document.body.appendChild(extraContainer); ariaDescriber.describe(component.element1, 'Hello'); - // Use `querySelectorAll` with an attribute since `getElementById` will stop at the first match. - expect(document.querySelectorAll(`[id='${MESSAGES_CONTAINER_ID}']`).length).toBe(1); + expect(document.querySelectorAll('.cdk-describedby-message-container').length).toBe(1); + + if (extraContainer.parentNode) { + extraContainer.parentNode.removeChild(extraContainer); + } + }); + + it('should not clear any pre-existing containers coming from the browser', () => { + createFixture(); + const extraContainer = document.createElement('div'); + extraContainer.classList.add('cdk-describedby-message-container'); + document.body.appendChild(extraContainer); + + ariaDescriber.describe(component.element1, 'Hello'); + + expect(document.querySelectorAll('.cdk-describedby-message-container').length).toBe(2); if (extraContainer.parentNode) { extraContainer.parentNode.removeChild(extraContainer); @@ -334,7 +349,7 @@ describe('AriaDescriber', () => { }); function getMessagesContainer() { - return document.querySelector(`#${MESSAGES_CONTAINER_ID}`)!; + return document.querySelector('.cdk-describedby-message-container')!; } function getMessageElements(): Element[] | null { diff --git a/src/cdk/a11y/aria-describer/aria-describer.ts b/src/cdk/a11y/aria-describer/aria-describer.ts index cc0fc37a5743..c54c9b6adc70 100644 --- a/src/cdk/a11y/aria-describer/aria-describer.ts +++ b/src/cdk/a11y/aria-describer/aria-describer.ts @@ -8,6 +8,7 @@ import {DOCUMENT} from '@angular/common'; import {Inject, Injectable, OnDestroy} from '@angular/core'; +import {Platform} from '@angular/cdk/platform'; import {addAriaReferencedId, getAriaReferenceIds, removeAriaReferencedId} from './aria-reference'; @@ -23,24 +24,30 @@ export interface RegisteredMessage { referenceCount: number; } -/** ID used for the body container where all messages are appended. */ +/** + * ID used for the body container where all messages are appended. + * @deprecated No longer being used. To be removed. + * @breaking-change 14.0.0 + */ export const MESSAGES_CONTAINER_ID = 'cdk-describedby-message-container'; -/** ID prefix used for each created message element. */ +/** + * ID prefix used for each created message element. + * @deprecated To be turned into a private variable. + * @breaking-change 14.0.0 + */ export const CDK_DESCRIBEDBY_ID_PREFIX = 'cdk-describedby-message'; -/** Attribute given to each host element that is described by a message element. */ +/** + * Attribute given to each host element that is described by a message element. + * @deprecated To be turned into a private variable. + * @breaking-change 14.0.0 + */ export const CDK_DESCRIBEDBY_HOST_ATTRIBUTE = 'cdk-describedby-host'; /** Global incremental identifier for each registered message element. */ let nextId = 0; -/** Global map of all registered message elements that have been placed into the document. */ -const messageRegistry = new Map(); - -/** Container for all registered messages. */ -let messagesContainer: HTMLElement | null = null; - /** * Utility that creates visually hidden elements with a message content. Useful for elements that * want to use aria-describedby to further describe themselves without adding additional visual @@ -50,8 +57,22 @@ let messagesContainer: HTMLElement | null = null; export class AriaDescriber implements OnDestroy { private _document: Document; + /** Map of all registered message elements that have been placed into the document. */ + private _messageRegistry = new Map(); + + /** Container for all registered messages. */ + private _messagesContainer: HTMLElement | null = null; + + /** Unique ID for the service. */ + private readonly _id = `${nextId++}`; + constructor( - @Inject(DOCUMENT) _document: any) { + @Inject(DOCUMENT) _document: any, + /** + * @deprecated To be turned into a required parameter. + * @breaking-change 14.0.0 + */ + private _platform?: Platform) { this._document = _document; } @@ -77,8 +98,8 @@ export class AriaDescriber implements OnDestroy { if (typeof message !== 'string') { // We need to ensure that the element has an ID. setMessageId(message); - messageRegistry.set(key, {messageElement: message, referenceCount: 0}); - } else if (!messageRegistry.has(key)) { + this._messageRegistry.set(key, {messageElement: message, referenceCount: 0}); + } else if (!this._messageRegistry.has(key)) { this._createMessageElement(message, role); } @@ -107,13 +128,13 @@ export class AriaDescriber implements OnDestroy { // If the message is a string, it means that it's one that we created for the // consumer so we can remove it safely, otherwise we should leave it in place. if (typeof message === 'string') { - const registeredMessage = messageRegistry.get(key); + const registeredMessage = this._messageRegistry.get(key); if (registeredMessage && registeredMessage.referenceCount === 0) { this._deleteMessageElement(key); } } - if (messagesContainer && messagesContainer.childNodes.length === 0) { + if (this._messagesContainer?.childNodes.length === 0) { this._deleteMessagesContainer(); } } @@ -121,18 +142,15 @@ export class AriaDescriber implements OnDestroy { /** Unregisters all created message elements and removes the message container. */ ngOnDestroy() { const describedElements = - this._document.querySelectorAll(`[${CDK_DESCRIBEDBY_HOST_ATTRIBUTE}]`); + this._document.querySelectorAll(`[${CDK_DESCRIBEDBY_HOST_ATTRIBUTE}="${this._id}"]`); for (let i = 0; i < describedElements.length; i++) { this._removeCdkDescribedByReferenceIds(describedElements[i]); describedElements[i].removeAttribute(CDK_DESCRIBEDBY_HOST_ATTRIBUTE); } - if (messagesContainer) { - this._deleteMessagesContainer(); - } - - messageRegistry.clear(); + this._deleteMessagesContainer(); + this._messageRegistry.clear(); } /** @@ -149,53 +167,67 @@ export class AriaDescriber implements OnDestroy { } this._createMessagesContainer(); - messagesContainer!.appendChild(messageElement); - messageRegistry.set(getKey(message, role), {messageElement, referenceCount: 0}); + this._messagesContainer!.appendChild(messageElement); + this._messageRegistry.set(getKey(message, role), {messageElement, referenceCount: 0}); } /** Deletes the message element from the global messages container. */ private _deleteMessageElement(key: string|Element) { - const registeredMessage = messageRegistry.get(key); + const registeredMessage = this._messageRegistry.get(key); const messageElement = registeredMessage && registeredMessage.messageElement; - if (messagesContainer && messageElement) { - messagesContainer.removeChild(messageElement); + if (this._messagesContainer && messageElement) { + this._messagesContainer.removeChild(messageElement); } - messageRegistry.delete(key); + this._messageRegistry.delete(key); } /** Creates the global container for all aria-describedby messages. */ private _createMessagesContainer() { - if (!messagesContainer) { - const preExistingContainer = this._document.getElementById(MESSAGES_CONTAINER_ID); + if (this._messagesContainer) { + return; + } + + const containerClassName = 'cdk-describedby-message-container'; + const serverContainers = + this._document.querySelectorAll(`.${containerClassName}[platform="server"]`); + for (let i = 0; i < serverContainers.length; i++) { // When going from the server to the client, we may end up in a situation where there's // already a container on the page, but we don't have a reference to it. Clear the // old container so we don't get duplicates. Doing this, instead of emptying the previous // container, should be slightly faster. - if (preExistingContainer && preExistingContainer.parentNode) { - preExistingContainer.parentNode.removeChild(preExistingContainer); + const serverContainer = serverContainers[i]; + if (serverContainer.parentNode) { + serverContainer.parentNode.removeChild(serverContainer); } + } - messagesContainer = this._document.createElement('div'); - messagesContainer.id = MESSAGES_CONTAINER_ID; - // We add `visibility: hidden` in order to prevent text in this container from - // being searchable by the browser's Ctrl + F functionality. - // Screen-readers will still read the description for elements with aria-describedby even - // when the description element is not visible. - messagesContainer.style.visibility = 'hidden'; - // Even though we use `visibility: hidden`, we still apply `cdk-visually-hidden` so that - // the description element doesn't impact page layout. - messagesContainer.classList.add('cdk-visually-hidden'); - - this._document.body.appendChild(messagesContainer); + const messagesContainer = this._document.createElement('div'); + + // We add `visibility: hidden` in order to prevent text in this container from + // being searchable by the browser's Ctrl + F functionality. + // Screen-readers will still read the description for elements with aria-describedby even + // when the description element is not visible. + messagesContainer.style.visibility = 'hidden'; + // Even though we use `visibility: hidden`, we still apply `cdk-visually-hidden` so that + // the description element doesn't impact page layout. + messagesContainer.classList.add(containerClassName); + messagesContainer.classList.add('cdk-visually-hidden'); + + // @breaking-change 14.0.0 Remove null check for `_platform`. + if (this._platform && !this._platform.isBrowser) { + messagesContainer.setAttribute('platform', 'server'); } + + this._document.body.appendChild(messagesContainer); + this._messagesContainer = messagesContainer; } /** Deletes the global messages container. */ private _deleteMessagesContainer() { - if (messagesContainer && messagesContainer.parentNode) { - messagesContainer.parentNode.removeChild(messagesContainer); - messagesContainer = null; + if (this._messagesContainer && this._messagesContainer.parentNode) { + this._messagesContainer.parentNode.removeChild(this._messagesContainer); + this._messagesContainer = null; } } @@ -212,12 +244,12 @@ export class AriaDescriber implements OnDestroy { * message's reference count. */ private _addMessageReference(element: Element, key: string|Element) { - const registeredMessage = messageRegistry.get(key)!; + const registeredMessage = this._messageRegistry.get(key)!; // Add the aria-describedby reference and set the // describedby_host attribute to mark the element. addAriaReferencedId(element, 'aria-describedby', registeredMessage.messageElement.id); - element.setAttribute(CDK_DESCRIBEDBY_HOST_ATTRIBUTE, ''); + element.setAttribute(CDK_DESCRIBEDBY_HOST_ATTRIBUTE, this._id); registeredMessage.referenceCount++; } @@ -226,7 +258,7 @@ export class AriaDescriber implements OnDestroy { * and decrements the registered message's reference count. */ private _removeMessageReference(element: Element, key: string|Element) { - const registeredMessage = messageRegistry.get(key)!; + const registeredMessage = this._messageRegistry.get(key)!; registeredMessage.referenceCount--; removeAriaReferencedId(element, 'aria-describedby', registeredMessage.messageElement.id); @@ -236,7 +268,7 @@ export class AriaDescriber implements OnDestroy { /** Returns true if the element has been described by the provided message ID. */ private _isElementDescribedByMessage(element: Element, key: string|Element): boolean { const referenceIds = getAriaReferenceIds(element, 'aria-describedby'); - const registeredMessage = messageRegistry.get(key); + const registeredMessage = this._messageRegistry.get(key); const messageId = registeredMessage && registeredMessage.messageElement.id; return !!messageId && referenceIds.indexOf(messageId) != -1; diff --git a/tools/public_api_guard/cdk/a11y.md b/tools/public_api_guard/cdk/a11y.md index 4e3ef6b329a4..6a27d4c57988 100644 --- a/tools/public_api_guard/cdk/a11y.md +++ b/tools/public_api_guard/cdk/a11y.md @@ -43,7 +43,8 @@ export class ActiveDescendantKeyManager extends ListKeyManager