Opened 7 years ago

Closed 6 years ago

#10042 closed New Feature (fixed)

Allow setting meaningful title for inline editable element

Reported by: Piotrek Koszuliński Owned by: Olek Nowodziński
Priority: Normal Milestone: CKEditor 4.2
Component: Accessibility Version: 4.0
Keywords: Cc: chris.ingham@…

Description

Reported on SO:

Currently editor sets the title to "Rich Text Editor, <editor.name>" which in many cases looks awful because of the meaningless name.

It should be possible to modify this behaviour or at least editor's name shouldn't be used. Another solution could be to introduce editor's name for humans and use it in such cases.

Change History (28)

comment:1 Changed 7 years ago by Jakub Ś

Status: newconfirmed
Type: BugNew Feature

This is more of a feature then a bug IHMO.

comment:2 Changed 7 years ago by Chris Ingham

Cc: chris.ingham@… added

To my users a tooltip containing irrelevant data is a bug :(

In the absence of a better recommendation I'm looking at dynamically removing the title attribute as suggested in the referenced SO links.

Last edited 7 years ago by Chris Ingham (previous) (diff)

comment:3 Changed 7 years ago by Chris Ingham

Cc: chris.ingham@… removed

comment:4 Changed 7 years ago by Chris Ingham

Cc: chris.ingham@… added

comment:5 Changed 7 years ago by Jakub Ś

To my users a tooltip containing irrelevant data

@inghamc I think we both agree that this is just a matter of looking at this problem. Personally to me message "Rich Text Editor, editor one explains exactly were user is. You can't mistake it with input filed ;)

comment:6 Changed 7 years ago by Anna Tomanek

comment:7 Changed 7 years ago by Wiktor Walc

Just for the reference:

This is the way how the element looks like before CKEditor attaches itself:

<div id="editable" contenteditable="true">

This is the way how the element looks like after CKEditor is loaded)

<div id="editable" class="cke_editable cke_editable_inline cke_contents_ltr cke_show_borders" 
contenteditable="true" tabindex="0" spellcheck="false" style="position: relative;" 
role="textbox" aria-label="Rich Text Editor, editable" 
title="Rich Text Editor, editable" aria-describedby="cke_85">

1) Note that even when CKEditor toolbar is not shown, when hovering over the editable element users see a pretty confusing tooltip (-> what "Rich Text Editor, ctl00_ctl00_MainContent_MemberMain_..."? I don't see any editor?).

I think the tooltip should be improved for the inline element. The title is there because basically we want to inform user that this particular element can be edited using rich text editor. So maybe something that actually really does inform user nicely about it would be reasonable?

2) Being able to change the editor "human friendly name" (config.label)? or decide whether the "editor name part" should be displayed at all sounds like a good addition as well.

comment:8 Changed 7 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.2

We should introduce config.name setting. Note: we need to validate whether this name isn't yet taken. There might not be such a test (in dev and tests) since now we generate safe name.

comment:9 Changed 7 years ago by Jakub Ś

Just a thought - if we introduce new config option and someone will use it in config.js such name will be used for all editors. If someone has few editors on page, everyone will be called e.g. "my forum post editor".
If this gets validated then except first no other editor will receive custom name.

While there is probably no better option for it, perhaps it should be documented at least that this config option should be instance specific and not used in config.js.

comment:10 Changed 7 years ago by Piotrek Koszuliński

Yep, exactly. If we won't have a better idea, then this needs to be well documented. Or we can take the "safe way" and if defined name is already taken append an incremented value like we now.

comment:11 Changed 7 years ago by Alfonso Martínez de Lizarrondo

Please, don't call it "name".

People will confuse it with the instance name used in CKEDITOR.instances, and this feature shouldn't be related to the instance name (yes, the default value will be generated from that, but when the user changes it, then the instance name should remain the same)

I would call it just "title" and add also a method "setTitle" so it can be called to change it after the instance has been created (in a perfect world it would be possible to use a property setter, but I'm not sure that if that will work correctly in all the browsers)

comment:12 Changed 7 years ago by Piotrek Koszuliński

This may be a good idea. We can have both - editor.name and editor.title. Name being what it is now, so an id of an editor. Title on the other hand would be a "human readable name". And title will be used in all these aria labels, titles and so on.

PS. Nope, setters are not available in IE8 :D http://kangax.github.com/es5-compat-table/#Setter%20in%20property%20initializer

Last edited 7 years ago by Piotrek Koszuliński (previous) (diff)

comment:13 Changed 7 years ago by Olek Nowodziński

Owner: set to Olek Nowodziński
Status: confirmedassigned

comment:14 Changed 7 years ago by Olek Nowodziński

Status: assignedreview

Created branch t/10042 (+tests) that introduces editor.title, config.title and editor.title uniqueness check.

comment:15 Changed 7 years ago by Alfonso Martínez de Lizarrondo

I haven't tested the code but just looking at it I would say that it doesn't fix the problem that the people have:

I don't understand why you want to make the title Unique among all the editors (if the user has set some value for the config then I think that they want exactly that, not a modification), and if they try to set editor.config.title to an empty string it will use [ editor.lang.editor, editor.name ].join( ', ' ) instead.

comment:16 Changed 7 years ago by Piotrek Koszuliński

Yep, I agree that we shouldn't make editor "too smart" if that limits user. We expose config.title to allow user overwrite default behaviour - then it's up to him how this option will be used.

Although, I haven't checked the code yet, so it's not R-.

Last edited 7 years ago by Piotrek Koszuliński (previous) (diff)

comment:17 Changed 7 years ago by Olek Nowodziński

Status: reviewassigned

comment:18 Changed 6 years ago by Daniel Kirsch

For my application I don't want to have any change to the title attribute at all and want to turn if off completely as it's important to me to let the user only see what he created - including title attributes.

Workaround for me so far is: delete a.changeAttr("title",c) (line 283 in the compressed code)

I really wish to see as little changes to my HTML element as possible. I would prefer if the editor would use data- attributes and use them for styling and everything else that is needed. In other cases, I have to parse the created HTML and remove all unwanted additions which is a pain and would be easier if data-attributes where used.

comment:19 Changed 6 years ago by Piotrek Koszuliński

What else, beside editable title, do you think that editor should not change or add?

comment:20 Changed 6 years ago by Daniel Kirsch

Especially don't change the class attribute or add additional styles to the style attribute. These are common attributes used by most applications. They can change the behavior of the edited element and might be used by the application which contains the editable elements (eg. I removed the position:relative definition in the CKEditor code because it changed the position of absolute positioned elements inside the editable element and I don't see why this definition is necessary).

When destroying the editor instance, most changes will be set back, however eg. the style definition position:relative remains.

A defined aria attribute will be overwritten by the editor which IMHO is really bad. It should be at least reset when destroying the editor instance. I love the aria support, however I wish to be able to deactivate it or at least doesn't touch my own settings.

Cheers and thanks for reading.

comment:21 Changed 6 years ago by Jakub Ś

Replying to alfonsoml

I don't understand why you want to make the title Unique among all the editors (if the user >has set some value for the config then I think that they want exactly that, not a >modification)

The initial request was talking about screen readers. If we don't set unique name how will visually impaired user know in which editor he is?
Perhaps this is just a matter of documentation - we should explain users: if you set empty title there will be no title (or default message without name), if you set it in config.js it will be the same among instances which can confuse visually impaired users, if you set it in instance configuration it will be specific to this instance only.
Perhaps it would be best to implement it that way (in what you set is what you get style) and document this well.

comment:22 Changed 6 years ago by Piotrek Koszuliński

@j.swiderski - yep, if we want to give developers full control, then we have to solve this problem using documentation. Although, I'm pretty sure that significant percentage of editors will be incorrectly configured.

@kurschkern - we'll investigate what happens to the editable. I agree that it would be better to leave editable as it is, but there may be reasons for what we do. Worth checking.

Regarding position:relative (and nullified top & left) - iirc we need them for magicline. We had serious troubles with positioning it - it has to be inside editable, but calculating its position according to unknown positioned ancestor would be a killer. But of course this change should be reverted when destroying editor - no doubts about this.

comment:23 Changed 6 years ago by Olek Nowodziński

Status: assignedreview

Updated t/10042 (+tests).

  • Non-unique editor.title are allowed.
  • Disable title attribute if config.title = false.
  • Extended docs with additional information regarding possible accessibility risks.

comment:24 Changed 6 years ago by Piotrek Koszuliński

I think that you should clarify what do you mean by "disable title". It is confusing because when title is already set and config.title=false that original title should not be modified. I'm not sure if "disable" is a good word for this.

comment:25 Changed 6 years ago by Olek Nowodziński

Docs have been clarified. Also added a new tc and slightly refactored tests code.

comment:26 Changed 6 years ago by Mike Burgh

As an aside, the config.title=false makes a big difference in IE.

If the tooltip shows up in IE in the inline mode, and the user clicks on it the editor will close, they have to be quick to click it, but it's easy enough to do.

comment:27 Changed 6 years ago by Piotrek Koszuliński

Status: reviewreview_passed

I pushed both branches rebased and with minor additions (docs fixes, one more test case).

R+.

comment:28 Changed 6 years ago by Olek Nowodziński

Resolution: fixed
Status: review_passedclosed

Fixed in major:

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy