Skip to content

Commit 656d749

Browse files
authored
Merge pull request #18201 from ckeditor/ck/16923-drop-role
Fix (table): Removed `role: textbox` from `td` / `th` editables. Windows Narrator no longer reads table dimensions as 0 by 0. Closes #16923
2 parents 2a447ec + cf7d6f3 commit 656d749

File tree

8 files changed

+74
-49
lines changed

8 files changed

+74
-49
lines changed

packages/ckeditor5-list/tests/todolist/todolistediting.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ describe( 'TodoListEditing', () => {
648648
'<tbody>' +
649649
'<tr>' +
650650
'<td class="ck-editor__editable ck-editor__nested-editable" ' +
651-
'contenteditable="true" role="textbox" tabindex="-1">' +
651+
'contenteditable="true" tabindex="-1">' +
652652
'<span class="ck-table-bogus-paragraph">foo</span>' +
653653
'</td>' +
654654
'</tr>' +

packages/ckeditor5-restricted-editing/tests/restrictededitingmodeediting.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ describe( 'RestrictedEditingModeEditing', () => {
354354
).to.equalMarkup(
355355
'<figure class="ck-widget ck-widget_with-selection-handle table" contenteditable="false">' +
356356
'<div class="ck ck-widget__selection-handle"></div>' +
357-
'<table><tbody><tr><td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true" role="textbox" ' +
357+
'<table><tbody><tr><td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true" ' +
358358
'tabindex="-1">' +
359359
'<span class="ck-table-bogus-paragraph"><span class="restricted-editing-exception"><b>foo bar baz</b></span></span>' +
360360
'</td></tr></tbody></table>' +

packages/ckeditor5-table/src/converters/downcast.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ export function downcastCell( options: { asWidget?: boolean } = {} ): ElementCre
112112
const cellElementName = isHeading ? 'th' : 'td';
113113

114114
result = options.asWidget ?
115-
toWidgetEditable( writer.createEditableElement( cellElementName ), writer ) :
115+
toWidgetEditable( writer.createEditableElement( cellElementName ), writer, { withAriaRole: false } ) :
116116
writer.createContainerElement( cellElementName );
117117
break;
118118
}

packages/ckeditor5-table/tests/_utils/utils.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,6 @@ function makeRows( tableData, options ) {
379379
if ( asWidget ) {
380380
attributes.class = getClassToSet( attributes );
381381
attributes.contenteditable = 'true';
382-
attributes.role = 'textbox';
383382
attributes.tabindex = '-1';
384383
}
385384

packages/ckeditor5-table/tests/converters/downcast.js

Lines changed: 41 additions & 43 deletions
Large diffs are not rendered by default.

packages/ckeditor5-table/tests/tablecaption/tablecaptionediting.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ describe( 'TableCaptionEditing', () => {
309309
'<table>' +
310310
'<tbody>' +
311311
'<tr>' +
312-
'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true" role="textbox"' +
312+
'<td class="ck-editor__editable ck-editor__nested-editable" contenteditable="true"' +
313313
' tabindex="-1">' +
314314
'<span class="ck-table-bogus-paragraph">xyz</span>' +
315315
'</td>' +

packages/ckeditor5-widget/src/utils.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ export function getLabel( element: ViewElement ): string {
243243
* * adds the `ck-editor__editable` and `ck-editor__nested-editable` CSS classes,
244244
* * adds the `ck-editor__nested-editable_focused` CSS class when the editable is focused and removes it when it is blurred.
245245
* * implements the {@link ~setHighlightHandling view highlight on widget's editable}.
246+
* * sets the `role` attribute to `textbox` for accessibility purposes.
246247
*
247248
* Similarly to {@link ~toWidget `toWidget()`} this function should be used in `editingDowncast` only and it is usually
248249
* used together with {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToElement `elementToElement()`}.
@@ -275,18 +276,24 @@ export function getLabel( element: ViewElement ): string {
275276
*
276277
* @param options Additional options.
277278
* @param options.label Editable's label used by assistive technologies (e.g. screen readers).
279+
* @param options.withAriaRole Whether to add the role="textbox" attribute on the editable. Defaults to `true`.
278280
* @returns Returns the same element that was provided in the `editable` parameter
279281
*/
280282
export function toWidgetEditable(
281283
editable: ViewEditableElement,
282284
writer: DowncastWriter,
283285
options: {
284286
label?: string;
287+
withAriaRole?: boolean;
285288
} = {}
286289
): ViewEditableElement {
287290
writer.addClass( [ 'ck-editor__editable', 'ck-editor__nested-editable' ], editable );
288291

289-
writer.setAttribute( 'role', 'textbox', editable );
292+
// Set role="textbox" only if explicitly requested (defaults to true for backward compatibility).
293+
if ( options.withAriaRole !== false ) {
294+
writer.setAttribute( 'role', 'textbox', editable );
295+
}
296+
290297
writer.setAttribute( 'tabindex', '-1', editable );
291298

292299
if ( options.label ) {

packages/ckeditor5-widget/tests/utils.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,27 @@ describe( 'widget utils', () => {
240240
expect( element.getAttribute( 'tabindex' ) ).to.equal( '-1' );
241241
} );
242242

243+
it( 'should add role attribute by default for backward compatibility', () => {
244+
const element = new ViewEditableElement( viewDocument, 'div' );
245+
toWidgetEditable( element, writer );
246+
247+
expect( element.getAttribute( 'role' ) ).to.equal( 'textbox' );
248+
} );
249+
250+
it( 'should add role attribute when withAriaRole is set to true', () => {
251+
const element = new ViewEditableElement( viewDocument, 'div' );
252+
toWidgetEditable( element, writer, { withAriaRole: true } );
253+
254+
expect( element.getAttribute( 'role' ) ).to.equal( 'textbox' );
255+
} );
256+
257+
it( 'should not add role attribute when withAriaRole is set to false', () => {
258+
const element = new ViewEditableElement( viewDocument, 'div' );
259+
toWidgetEditable( element, writer, { withAriaRole: false } );
260+
261+
expect( element.hasAttribute( 'role' ) ).to.be.false;
262+
} );
263+
243264
it( 'should add label if it was passed through options', () => {
244265
toWidgetEditable( element, writer, { label: 'foo' } );
245266
expect( element.getAttribute( 'aria-label' ) ).to.equal( 'foo' );

0 commit comments

Comments
 (0)