Allow editable to accept aria attributes

[BoardJames]

Description

Previously the TinyMCE component in Editable could only have the aria-label attribute but it is apparent that an accessible implementation of auto-completion and probably other things will require setting additional aria attributes (see #2896 (comment) ) so I have created this small PR to enable passing through any ARIA attribute in the same way that styles are passed.

How Has This Been Tested?

I manually changed autocomplete to pass through the aria-activedescendant which was set to the current autocomplete value and it updated the property on tinymce as the autocomplete selection changed as expected. Tested on Firefox and Chrome.
With the current headless test framework I can't think of a way to automate testing of this but that could be added at a later date when in-browser automated testing becomes available.

Types of changes

New feature.

Checklist:

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

[codecov]

Codecov Report

Merging #3106 into master will increase coverage by 0.12%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #3106      +/-   ##
==========================================
+ Coverage   31.64%   31.76%   +0.12%     
==========================================
  Files         217      218       +1     
  Lines        6282     6302      +20     
  Branches     1116     1116              
==========================================
+ Hits         1988     2002      +14     
- Misses       3610     3616       +6     
  Partials      684      684

Impacted Files	Coverage Δ
blocks/editable/tinymce.js	0% <0%> (ø)	⬆️
blocks/editable/aria.js	100% <100%> (ø)
blocks/editable/index.js	11.02% <100%> (+0.32%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa13720...5764640. Read the comment docs.

[aduth]

See also #2789, which seems to achieve the same goal with a slightly different implementation (supporting other attributes).

[BoardJames]

Ok, I have looked at it and it might be a good solution for non-aria attributes. I'm not sure that I agree that putting aria attributes on the wrong element is a good idea even if you also put aria-hidden; for example quoting from https://www.w3.org/TR/wai-aria/states_and_properties#aria-owns :

Authors MUST ensure that an element's ID is not specified in more than one other element's aria-owns attribute at any time.

One of these two methods needs to be merged or I can not proceed with making auto-completion accessible.

[ephox-mogran]

@afercia what are your thoughts on this? Can there be multiple DOM nodes with aria-owns pointing to the same id as long as one of those nodes has aria-hidden?

[afercia]

Can there be multiple DOM nodes with aria-owns pointing to the same id as long as one of those nodes has aria-hidden?

Hm good question, I'd stick to the specification and not try to do fancy things.

Authors must ensure that an element's ID is not specified in more than one other element's aria-owns attribute at any time. In other words, an element can have only one explicit owner.

Note that in the W3C specifications, "MUST" has a very mandatory meaning.

Software is designed based on specifications. I don't see why screen readers should be so smart to check aria-hidden before establishing an aria-owns relationship. It may or may not work, but a similar behavior is not described anywhere in the specification and it's just an assumption.

Screen readers are not browsers, they don't read the DOM directly (that would be too expensive), they just get an ID and point to the targeted element. For example, aria-describedby works even if the targeted element is hidden with display: none. I guess the same would happen with aria-owns, getting multiple owners for the same element.

[mcsf]

@EphoxJames, based on @aduth's feedback in #2789:

† I think ideally someone who renders an Editable could pass these props as the attributes they would expect to be applied, aria-label etc. instead of a custom grouping object, particularly if we're seeking to apply a broad range of attributes.

I've pushed ac1d5f7cd0c8fde2032d816a28a9111a1320dead. Quoting from the commit message, "one benefit we now get for free is that the linter is able to warn the author if they pass an invalid ARIA attribute."

Feel free to argue with the change. :)

[BoardJames]

The changes seem sensible in general.
My only query is why did you calculate the updated keys using an O(n^2) difference algorithm instead of the O(n) filter that I used originally?

[mcsf]

I wanted to see what this method could look like if we stuck to a single collection interface (difference/differenceWith) and whether it made it any clearer, but you're quite right: it's wasteful and doesn't do much for the reader. Amended and pushed.

[mcsf]

The hand-rolled prop diffing deserves some tests.

[BoardJames]

Good idea - I will move the prop diffing into aria.js and add some tests.

[BoardJames]

Ok, tests added. Do you think it is good to merge?

[mcsf]

Ok, tests added. Do you think it is good to merge?

Looks great!