Make editor's application labels unique

[Comandeer]

📝 Provide detailed reproduction steps (if any)

1. Open https://ckeditor.com/docs/ckeditor5/latest/features/lists/lists.html
2. Open VoiceOver.
3. Open rotor and switch to landmarks list.

✔️ Expected result

Every editor has a unique name on the list displayed by the rotor.

❌ Actual result

Every editor has the same name, "Rich Text Editor".

❓ Possible solution

Add the editor name after the static part of the label: "Rich Text Editor, <editor name>".

📃 Other details

• Browser: all (tested on Chrome)
• OS: all
• First affected CKEditor version: all
• Installed CKEditor plugins: N/A

The analogous issue is present in other screen readers.

---

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

[mlewand]

While working on this issue we can look how we solve this problem in CKE4.

We could add a config property that would allow for providing a human readable title / label. CKE4 has it for instance here: https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_config.html#cfg-applicationTitle

If no human-readable title is provided we should do the same thing we did in CKE4, so create a label like "editor1", "editor2", "editorn...".

[oleq]

Notes from the recent meeting:

• Let's use editor.config.ui.name for configuration.
• For the time being, no properties such as Editor#name.
  • If a feature wants to know the name of the editor, it should use editor.config.get( 'ui.name' ) instead.
  • To provide the incremental EditorN label for editors without editor.config.ui.name config I think we should use editor.config.define() instead of generator proposed in the PR. We're doing this already.
• If there's just a single root (e.g. classic editor), we ditch its name in the label (Editor editing area instead of Editor editing area: main) because it only creates noise.
• Multi root editors
  • editor.config.ui.name should be used for the entire root constellation
  • As for naming individual roots, let's leave it as it is (Rich Text Editor. Editing area: footerleft) for now.
• Editor context should not pass editor.config.ui.name to the editor (we need to exclude it to avoid misleading duplication)
• Comments editor
  • All editors should be using the same name because the (a11y) context will be set on the parent of the thread in the future.
• Let's ignore CKEditorError integration because accessible name is not an instance name.
• Let's ignore CKEditorInspector integration because accessible name is not an instance name

[oleq]

To provide the incremental EditorN label for editors without editor.config.ui.name config I think we should use editor.config.define() instead of generator proposed in the PR. We're doing this already.

I just realized this won't work. See how the classic editor is initialized:

https://github.com/ckeditor/ckeditor5/blob/b0a183e5ddf131625d6c9c26fc575269e55890a1/packages/ckeditor5-editor-classic/src/classiceditor.js#L75-L90

• I think EditorUI (in this case ClassicEditorUI) should use editor.config.define( 'ui.name' ) because the Editor class has nothing to do with the UI. 
• But... to allow EditorUIView (in this case ClassicEditorUIView) propagate the value of the config in DOM, it needs to be known before EditorUI gets created. 
• I don't remember why we decided to create a view first and then pass it to the UI. I don't see any elegant solution to work around this one.

[scofalik]

Maybe I don't see something but... why not set it as one of options when creating ClassicEditorUIView instance?

const view = new ClassicEditorUIView( this.locale, this.editing.view, { 
     shouldToolbarGroupWhenFull,
     label: this.config.get( 'ui.name' )
} );

Or just pass whole config.

Or, cannot this be a dynamic value (I mean either bound to template or just observable)? It could be set afterwards. Why not?

[oleq]

label: this.config.get( 'ui.name' )

Because there must be a place that takes care of unnamed editors and increments "EditorN". Preferably by setting editor.config.define( 'ui.name', Editor${ ++i } ). And the right place for this thing is definitely EditorUI. And that does not exist yet at this point 😛

[oleq]

An update: After a F2F with PK we figured that we're gonna change the API for our editors' UIs ClassicEditorUI, InlineEditorUI etc. and implement ClassicEditorUI#setView( view: EditorUIView )

This will be a minor change because it's unlikely that any of these UIs is used as a base class by integrators and, at the same time, EditorUI#constructor( editor: Editor ) accepts editor instance only, so we're not changing the API of the base class. 

Also, we figured we should probably use editor.config.ui.editorName but let's figure it out during code review when we can see everything coming together.

To sum things up:

• Let's use editor.config.ui.name for configuration.
• For the time being, no properties such as Editor#name.
  • If a feature wants to know the name of the editor, it should use editor.config.get( 'ui.name' ) instead.
  • To provide the incremental Rich Text Editor N label for editors without editor.config.ui.name config I think we should use editor.config.define() instead of generator proposed in the PR. We're doing this already.
    • To make it work, we should ditch the view argument in *EditorUI#constructor() and use *EditorUI#setView( view: EditorUIView ) instead (minor BC)
    • I drafted a PoC and it seems to be working.
• If there's just a single root (e.g. classic editor), we ditch its name in the label (Editor editing area instead of Editor editing area: main) because it only creates noise.
• Multi root editors
  • editor.config.ui.name should be used for the entire root constellation
  • As for naming individual roots, let's leave it as it is (Rich Text Editor. Editing area: footerleft) for now.
• Editor context should not pass editor.config.ui.name to the editor (we need to exclude it to avoid misleading duplication)
• Comments editor
  • All editors should be using the same name because the (a11y) context will be set on the parent of the thread in the future.
• Let's ignore CKEditorError integration because accessible name is not an instance name.
• Let's ignore CKEditorInspector integration because accessible name is not an instance name

[scofalik]

Does this have any impact on multi root editor implementations that people have? I mean changing how the view is set?

[oleq]

Does this have any impact on multi root editor implementations that people have? I mean changing how the view is set?

It should not because we're not changing the api of the base EditorUI class.

[FilipTokarski]

Related - #9731