Tweaks for representation of the editor in the accessibility tree

[Comandeer]

What is the purpose of this pull request?

Bug fix

Does your PR contain necessary tests?

All patches that change the editor code must include tests. You can always read more
on PR testing,
how to set the testing environment and
how to create tests
in the official CKEditor documentation.

This PR contains

• Unit tests
• Manual tests

Did you follow the CKEditor 4 code style guide?

Your code should follow the guidelines from the CKEditor 4 code style guide which helps keep the entire codebase consistent.

• PR is consistent with the code style guide

What is the proposed changelog entry for this pull request?

Fixed Issues:

* [#4052](https://github.com/ckeditor/ckeditor4/issues/4052): Fixed: Editor labels are read incorrectly by screen readers due to invalid editor control type for [Iframe Editing Area](https://ckeditor.com/cke4/addon/wysiwygarea) editors.
* [#1904](https://github.com/ckeditor/ckeditor4/issues/1904): Fixed: Screen readers are not announcing read-only editor state due to missing proper aria-labels.
* [#2445](https://github.com/ckeditor/ckeditor4/issues/2445): Fixed: The same editor's label is read several times by screen readers when focusing an editor.

API Changes:

* [#2445](https://github.com/ckeditor/ckeditor4/issues/2445): Added [`config.applicationTitle`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_config.html#cfg-applicationTitle) alongside [`CKEDITOR.editor#applicationTitle`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_editor.html#property-title) to allow customizing editor's application region label.

What changes did you make?

I've added missing [role=textbox] and [aria-multiline] attributes for wysiwygarea editors. I've also added [aria-readonly] attribute to editors to indicate the status of the "readonliness" of the editor. However, for iframe-based editors it required adding also the [tabindex=0] attribute to be actually read by screen readers. This change was implemented for all browsers except Firefox which has a bug connected with this attribute and iframes. It also means that marking read-only iframes does not work in Firefox.

I didn't touch #2445 because I have no idea how to differentiate labels for applications and editables. But the changes introduced for read-only editors indicate such a need (NVDA now reads the same label thrice – probably due to it being the label of the application, the label of the editor, and the title of the editor's iframe). Probably it's something that we need to discuss further.

Proposed changes should also fix #2499 but it needs further verification.

Which issues does your PR resolve?

Closes #4052.
Closes #1904.
Closes #2445.

[github-actions]

It's been a while since we last heard from you. We are marking this pull request as stale due to inactivity. Please provide the requested feedback or the pull request will be closed after next 7 days.

[jacekbogdanski]

NVDA

First thing is that I'm not actually able to reproduce #4052

Control type is not informational (I believe it is announced as 'group')

I've downloaded the latest version of NVDA, and it seems to properly read the editor labels (putting aside fact that the editor labels seem to be read a bit redundantly). So it's hard to tell me how these changes impacted reading labels as I don't see that many differences, except of course read-only support. @Comandeer did you manage to reproduce the original issue?

Currently, NVDA will read focused editor as:

Rich Text Editor, [editor name]
Rich Text Editor, [editor name] Frame
Rich Text Editor, [editor name] Document
Edit Multi Line [text]

In the case of a11yhelp plugin:

Rich Text Editor, [editor name]
Rich Text Editor, [editor name] Frame, press Alt + 0 for help
Rich Text Editor, [editor name] Document
Edit Multi Line [text]

I'm wondering if that much information is really needed. I'm not an accessibility expert, but wouldn't be more helpful to just read something like:

Rich Text Editor, [editor name]
Editing Area
[text]

In the case of a11yhelp plugin:
Rich Text Editor, [editor name], press Alt + 0 for help
Editing Area
[text]

It seems more helpful to skip that much information for me but maybe missing some important cases here. @Comandeer could you help me to understand why we are giving so much information here and duplicating the message?

I suppose that it may be related to switching between another focusable element (like toolbar) in the editor, but in that case, reading Editing Area would still give the user feedback on what's happening.

VoiceOver

I didn't notice any difference except for different reading for read-only (which I suppose is good). It seems that VoiceOver doesn't care about reading Iframe titles at all.

JAWS

Running JAWS resulted in freezing my VM, so I'm going to get back to it soon.

[Comandeer]

@Comandeer did you manage to reproduce the original issue?

Well, yes but actually no. I wasn't able to reproduce it on NVDA but I reproduced it on VO with Safari on desktop macOS.

@Comandeer could you help me to understand why we are giving so much information here and duplicating the message?

Because there are three (or even four) labels:

• for the whole editor (div[role=application] containing the toolbar and the editable),
• for the editable itself,
• for the iframe via its [title] attribute,
• for the document inside the iframe via the title element inside that document (this label is ignored in most cases).

Unfortunately, we reuse the same label for all of these places, which produces the effect you described. And that's the part I haven't fixed yet. There are two possible solutions here:

• come up with different labels for each of these places,
• remove some of the labels.

The second solution is far easier but could hinder the accessibility, e.g. removing the label for the application would make using features like VoiceOver's rotor more difficult (application regions wouldn't have names which makes their identification cumbersome or even impossible), and removing the iframe[title] attribute would cause some screen readers to read iframe's URL instead.

So the first solution is preferred… yet, I thought really long about the labels and I didn't come up with anything sensible 😞. And that's what blocking me from finishing the work on this PR.

[jacekbogdanski]

To sum up our discussion with @Comandeer F2F, we will try to come up with a simplified version of these labels:

Rich Text Editor, [editor name]
Editor [editor name] Frame, press Alt + 0 for help
Editing Area / Text Area (let's check if this label can be skipped without making the editor less accessible)
[text]

It should reduce repetitive text while keeping a good level of accessibility for the different parts of the editor. Note that labels may be slightly different depending on results from another code iteration.

[Comandeer]

Labels were far more difficult than I expected. I had to introduce editor#applicationTitle property alongside CKEDITOR.config#applicationTitle variable (as counterparts to editor#title and CKEDITOR.config#title). Additionally, I had to… add yet another label ([aria-label] attribute to the iframe's body for VoiceOver to correctly read the editor).

So there are two distinctive labels:

1. application one ("Rich Text Editor, <name>"),
2. editor one ("Editor, <name>").

The second one is used on editables and iframes. Unfortunately, this label is repeated twice in NVDA and JAWS (once for the frame itself and once for the document inside it) but it's probably the best approach.

[jacekbogdanski]

I'm often getting duplicated information about the help button with NVDA and JAWS with the Iframe editor. It reads something like:

1. Rich Text Editor, [editor name]
2. Editor [editor name] Frame
3. Press alt + 0 for help.
4. Editor [editor name] document
5. Press alt + 0 for help.

I'm not sure if it's valid from the accessibility point of view, but maybe it makes sense to read it only once?

I've also run into a bug where NVDA is reading labels literally twice - but I'm not sure if it's caused by a screen reader or the editor. Did you notice that issue during your tests?

I'm not fully confident that we should read so much information. I understand that technically it may make the editor less accessible by reading only some part of the editor (e.g. only frame) but it's a lot of information for a use case when the focus goes straight to editable. Anyway, I don't feel enough confident about how often there is a case when labeling all parts of the editor is important (and what % of our users it covers), so I'm leaving that up to @Comandeer.

[Comandeer]

I've also run into a bug where NVDA is reading labels literally twice - but I'm not sure if it's caused by a screen reader or the editor. Did you notice that issue during your tests?

The issue was not present in Firefox and in Chrome it's present only after refocusing the editor therefore I didn't catch it :/ It seems to be a screen reader error as the HTML code is correct and it works correctly in Firefox.

I'm not sure if it's valid from the accessibility point of view, but maybe it makes sense to read it only once?

That's another thing that does not seem to be fixable… This info is added only once, as the [aria-describedby] attribute on the iframe, however, Firefox exposes the same description on both the iframe itself and the document inside the frame.

[Comandeer]

I've updated tests and API docs for config.applicationTitle. Unfortunately, other raised issues don't seem to be fixable on our side.

[jacekbogdanski]

Looks very well, and the explanation given in #2445 (comment) should make it clear why we have chosen that approach.

Could you add a manual test with:

• disabled and changed config.applicationTitle property
• disabled and changed config.title property?

[Comandeer]

I've added manual tests for labels, six in total:

• for disabled labels:
  • themeduidisabledapplicationlabels,
  • themeduidisablededitorlabels,
  • inlinedisabledapplicationlabels,
  • inlinedisablededitorlabels,
• for custom labels:
  • themeduicustomlabels,
  • inlinecustomlabels.

I've also found a bug when an inline editable got the [aria-label] with the false string as a value instead of not getting this attribute at all. Fixed it and added unit tests for disabled labels as well.

[jacekbogdanski]

Good job @Comandeer, these changes give much better control over editor accessibility compliance for integrators.