FIX: Use a `role="textbox"` for all the Editable elements #4074

tg-ephox · 2017-12-19T04:59:38Z

Description

Whichever node TinyMCE wraps now has role="textbox".

aria-multiline="true" is set for these node types 'p', 'ol', 'ul', 'table', 'pre', 'cite'

I'm not 100% sure this is correct for the Paragraph block as Enter or Return will add a new Paragraph, as described here.

How Has This Been Tested?

Visually, inspecting elements for different Blocks

Screenshots (jpeg or gifs if applicable):

Types of changes

Bug fix

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows has proper inline documentation.

aduth

What do you think about my comment at #3412 (comment) where I suggested maybe TinyMCE ought to be applying these attribute(s) automatically?

aduth · 2017-12-19T18:32:17Z

blocks/editable/index.js

@@ -825,7 +827,7 @@ export default class Editable extends Component {
 			formatters,
 		} = this.props;

-		const ariaProps = pickAriaProps( this.props );
+		const ariaProps = { ...pickAriaProps( this.props ), 'aria-multiline': MULTI_LINE_TAGS.indexOf( Tagname ) > -1 };


Should aria-multiline be dictated by this.props.multiline ?

this.props.multiline is undefined for a ParagraphBlock and is set to li for a ListBlock. I looked at this prop first but it wasn't suitable for this purpose...

Okay. I suspect future maintainers may have a similar reaction to mine in not understanding the difference between how this.props.multiline and aria-multiline are handled. Perhaps we could do for a code comment or documentation clarifying this.

I think the best approach would to re-name the multiline prop to something more appropriate, maybe multilineTag as this is how its used...

tg-ephox · 2017-12-20T04:30:35Z

@aduth we had some discussion internally believe that it is the responsibility of the tag author to supply its attributes.

aduth

The aria-multiline attribute is a bit confusing to me. Based on the implementation here, it would make me question why it's not up to the browser or accessible device to infer multiline from a tag name. But then, for how we treat a paragraph block, is it indeed "multiline" ? It can have multiple lines via Shift-Enter, but based on my reading of the aria-multiline specification, it would seem to be most relevant for describing the plain "Enter" behavior, in which case it's not accurate to say that the paragraph block is multiline, since pressing Enter creates a new separate role="textbox" element.

aduth · 2018-01-02T15:57:28Z

blocks/editable/index.js

@@ -825,7 +827,7 @@ export default class Editable extends Component {
 			formatters,
 		} = this.props;

-		const ariaProps = pickAriaProps( this.props );
+		const ariaProps = { ...pickAriaProps( this.props ), 'aria-multiline': MULTI_LINE_TAGS.indexOf( Tagname ) > -1 };


Okay. I suspect future maintainers may have a similar reaction to mine in not understanding the difference between how this.props.multiline and aria-multiline are handled. Perhaps we could do for a code comment or documentation clarifying this.

tg-ephox · 2018-01-04T01:07:05Z

But then, for how we treat a paragraph block, is it indeed "multiline" ? It can have multiple lines via Shift-Enter, but based on my reading of the aria-multiline specification, it would seem to be most relevant for describing the plain "Enter" behavior, in which case it's not accurate to say that the paragraph block is multiline, since pressing Enter creates a new separate role="textbox" element.

@aduth that was my concern after reading the spec.
@afercia should the Paragraph block have the aria-multiline attribute given that pressing Enter will add a new block?

afercia · 2018-01-04T15:33:27Z

@tg-ephox to me aria-multiline is meant to distinguish an element that emulates an input field from an element that emulates a textarea:
<div contenteditable role="textbox" ... --> input field
<div contenteditable role="textbox" aria-multiline-"true" ... --> textarea

VoiceOver, for example, announces:
edif text (pause) you are currently on a text field
edif text (pause) you are currently on a text area

Test: https://codepen.io/afercia/full/goGPPN/

As per the Enter behavior, that's not standard in a textarea and is a bit controversial... since default behaviors should not be altered, ever.

afercia · 2018-03-07T15:31:00Z

Any news on this? 🙂

afercia · 2018-03-07T15:40:43Z

Worth noting since 8ae2e80 the paragraph blocks have aria-* attributes typically used for autocompleters and thus they really need a role attribute too.

Even the W3C validator throws an error because of the missing role:

Error: Element p is missing required attribute role.

youknowriad · 2018-03-26T14:43:32Z

This issues is required for the merge proposal, thus, I went ahead and updated the PR. Let me know how it looks now @afercia Thanks

afercia · 2018-03-27T15:38:12Z

@youknowriad thanks. I've tested extensively, also merging this with the navigation/edit mode #5709 and the role application #5807 to get an idea of how this change improves the screen readers behavior.

It's definitely an improvement for all the blocks that can be considered similar to input fields or textareas, e.g.:

heading
paragraph
preformatted
pullquote
quote
verse

because they're really similar to inputs. It is also useful for blocks that are input sunder the hood, for example:

button
text columns

But, we should not set the role="textbox" on blocks that are completely different from inputs or textareas:

list: loses it snative semantics: number of items etc. are no longer announced
table: same, the number of rows and columns are no longer announced, as well as the current position in the table

We should definitely exclude the list and table blocks, wondering if there's an elegant way to do that. Also wondering about blocks added b plugins, how would they declare if the block editable part needs a role="textbox" or not?

youknowriad · 2018-03-27T15:40:55Z

We should definitely exclude the list and table blocks, wondering if there's an elegant way to do that. Also wondering about blocks added b plugins, how would they declare if the block editable part needs a role="textbox" or not?

If it's dependent on the TagName, I can add a rule saying that if the tagName is table, ul, ol don't add the role, does that work?

afercia · 2018-03-27T17:07:09Z

Yes I've seen the tagName thing :) was just wondering if there's a more solid way. Alternatively, this should really be well documented for plugin authors. I'd say the tagName is OK for now? and maybe we can iterate later, if need be.

For completeness, also the various captions are OK (cover image, image, gallery, embed, etc.)

afercia · 2018-03-28T09:05:14Z

🎉

tg-ephox changed the title ~~Add role='textbox' aria-multiline='true' to Editable where appropriate~~ FIX: Use a role="textbox" for all the Editable elements Dec 19, 2017

aduth reviewed Dec 19, 2017

View reviewed changes

aduth reviewed Jan 2, 2018

View reviewed changes

youknowriad force-pushed the fix/3412-role-textbox-editable branch from 809daed to 9421278 Compare March 26, 2018 14:42

youknowriad self-assigned this Mar 26, 2018

Add role='textbox' aria-multiline='true' to Editable where appropriate

89df64d

youknowriad force-pushed the fix/3412-role-textbox-editable branch from 9421278 to 89df64d Compare March 26, 2018 14:50

afercia approved these changes Mar 27, 2018

View reviewed changes

Disable the textbox role for tables and lists

9eef0ec

youknowriad merged commit d9a77d2 into master Mar 28, 2018

youknowriad deleted the fix/3412-role-textbox-editable branch March 28, 2018 09:03

afercia mentioned this pull request Mar 28, 2018

Give editable fields better labels #1659

Closed

afercia mentioned this pull request Apr 4, 2018

List block and screen readers: greatly inconsistent behavior #5983

Closed

afercia mentioned this pull request Dec 3, 2018

Table block: the RichText within the table cells shouldn't have role=textbox and aria-multiline=true #12525

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FIX: Use a `role="textbox"` for all the Editable elements #4074

FIX: Use a `role="textbox"` for all the Editable elements #4074

tg-ephox commented Dec 19, 2017

aduth left a comment

aduth Dec 19, 2017

tg-ephox Dec 20, 2017

aduth Jan 2, 2018

tg-ephox Jan 4, 2018

tg-ephox commented Dec 20, 2017

aduth left a comment

aduth Jan 2, 2018

tg-ephox commented Jan 4, 2018 •

edited

Loading

afercia commented Jan 4, 2018

afercia commented Mar 7, 2018

afercia commented Mar 7, 2018

youknowriad commented Mar 26, 2018

afercia commented Mar 27, 2018

youknowriad commented Mar 27, 2018 •

edited

Loading

afercia commented Mar 27, 2018

afercia commented Mar 28, 2018

FIX: Use a role="textbox" for all the Editable elements #4074

FIX: Use a role="textbox" for all the Editable elements #4074

Conversation

tg-ephox commented Dec 19, 2017

Description

How Has This Been Tested?

Screenshots (jpeg or gifs if applicable):

Types of changes

Checklist:

aduth left a comment

Choose a reason for hiding this comment

aduth Dec 19, 2017

Choose a reason for hiding this comment

tg-ephox Dec 20, 2017

Choose a reason for hiding this comment

aduth Jan 2, 2018

Choose a reason for hiding this comment

tg-ephox Jan 4, 2018

Choose a reason for hiding this comment

tg-ephox commented Dec 20, 2017

aduth left a comment

Choose a reason for hiding this comment

aduth Jan 2, 2018

Choose a reason for hiding this comment

tg-ephox commented Jan 4, 2018 • edited Loading

afercia commented Jan 4, 2018

afercia commented Mar 7, 2018

afercia commented Mar 7, 2018

youknowriad commented Mar 26, 2018

afercia commented Mar 27, 2018

youknowriad commented Mar 27, 2018 • edited Loading

afercia commented Mar 27, 2018

afercia commented Mar 28, 2018

FIX: Use a `role="textbox"` for all the Editable elements #4074

FIX: Use a `role="textbox"` for all the Editable elements #4074

tg-ephox commented Jan 4, 2018 •

edited

Loading

youknowriad commented Mar 27, 2018 •

edited

Loading